Skip to content

feat: generate OLX with single-quoted attributes (WIP) - #35031

Closed
kdmccormick wants to merge 1 commit into
openedx:masterfrom
kdmccormick:kdmccormick/single-quote-xml
Closed

feat: generate OLX with single-quoted attributes (WIP)#35031
kdmccormick wants to merge 1 commit into
openedx:masterfrom
kdmccormick:kdmccormick/single-quote-xml

Conversation

@kdmccormick

Copy link
Copy Markdown
Member

No description provided.

@kdmccormick

Copy link
Copy Markdown
Member Author

Since the latest iteration of the Content libraries upstream sync plan doesn't involve stuffing JSON into an OLX attribute, so I'm closing as this is no longer a priority.

However, if this is ever picked back up, here a summary of feedback from @ormsbee and @bradenmacdonald :

  • This would be good for OLX readability...
  • but it's a shame that there's builtin LXML option to do this for us...
  • although it's good that the logic here is at least isolated to one function in one module.
  • Having single quotes is not worth risking content integrity, so it would need to be airtight.
  • One way to make it failsafe would be to do a round-trip check when serializing, and fallback to the original double-quoted XML if it's not an exact match:
    xml_double_quotes = etree.tostring(root_node, ...)
    xml_single_quotes = "".join(_convert_attrs_to_single_quotes(iter(xml_str)))
    root_node_single_quotes_roundtripped = etree.fromstring(xml_single_quotes)
    if root_node_single_quotes_roundtripped == root_node:
        return xml_single_quotes
    else:
        return xml_double_quotes
  • Performance impacts of the above would need to be considered. Roll out with a waffle flag and monitor?

@kdmccormick kdmccormick closed this Sep 4, 2024
@kdmccormick
kdmccormick deleted the kdmccormick/single-quote-xml branch September 4, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant