Skip to content

feat(xml): read an xml file as its source, not as one very long line - #679

Merged
andiwand merged 3 commits into
mainfrom
feat/xml-source-view
Aug 10, 2026
Merged

feat(xml): read an xml file as its source, not as one very long line#679
andiwand merged 3 commits into
mainfrom
feat/xml-source-view

Conversation

@andiwand

@andiwand andiwand commented Aug 9, 2026

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

An xml file opened as text_file and rendered through the numbered line list. For the files anyone opens on purpose — content.xml, document.xml, anything a writer emitted rather than a human — that is one line several megabytes wide.

It now opens as xml::XmlFile and renders as a source view: reindented, highlighted, and foldable through <details>/<summary> with no script and no external resources.

Stages 1 and 2 of src/odr/internal/xml/PLAN.md, which is updated to say what landed and what did not.

What is in here

  • internal/xml/xml_file.{hpp,cpp}XmlFile, an abstract::TextFile over a text::TextFile mirroring JsonFile, plus parse_source holding the parse flags (parse_full | parse_ws_pcdata_single) both callers need.
  • internal/html/xml_file.{hpp,cpp}create_xml_service and the writer. xml_css and write_xml_style in frontend.cpp.
  • util::xml::read_declared_encoding — the declaration sniff, in the shared xml helper rather than a second one with the same name.
  • open_file_as gains an xml branch; open_file's unknown-type path tries it after svg and before the text fallthrough, so anything with a more specific reading keeps it. The table row flips to {.open = true, .translate_html = true}.
  • html::translate(const DecodedFile &) sends FileType::xml to the new service, next to the csv branch and for the same reason. Not the TextFile overload: a caller who asks to translate a text file is asking for the text rendering, and still gets it.

Decisions worth a second look

  • Encoding comes from the declaration. pugixml resolves UTF-8/16/32 from a BOM or the <?xml byte pattern and nothing else, so encoding="ISO-8859-1" would be read as UTF-8 and yield invalid UTF-8 in the node strings, silently. The declaration is read off the head of the file, mapped through text_encoding_by_name, and the bytes are transcoded before pugixml sees them. An encoding we can name but not decode throws UnsupportedTextEncoding — there is no "let the browser sort it out" once bytes are inside a parser — and the file falls back to the line list.
  • Mixed content is not reindented. An element holding any text child keeps all of its children on one line, untouched. The plan said "any non-whitespace text child"; with parse_ws_pcdata_single the two differ only for <a> </a>, and the looser rule is the one that leaves it alone.
  • No line numbers. They would number our lines, not the file's, which for a reindented minified document is actively misleading. The gutter carries the fold handles instead.
  • A parse failure falls back to text, so a malformed file renders exactly as it does today.
  • No DTD processing, which is pugixml's behaviour and closes XXE and entity expansion by construction.

Behaviour changes to be aware of

  • A .xml that answered text_file / text/plain now answers xml / application/xml. No binding work — the enumerator already existed — but a caller switching on file_type() sees it.
  • A flat-xml ODF, .xhtml, .rels, .plist and rss feeds become source views rather than line lists. Correct for a source viewer; not to be mistaken for flat-ODF support.

Not in here

Stage 3 (a node budget for the tens-of-megabytes case, which wants a measurement first) and stage 4 (routing archive entries through translate). The test-data samples stage 1 asked for are still owed — the tests are inline string literals in test/src/internal/xml/xml_file_test.cpp.

Test plan

  • ./test/odr_test --gtest_filter='Xml*' — 13 new tests: detection, svg still winning over xml, malformed falling back to text, the declared encoding decoding, the declaration sniff itself, translating it as a text file still writing the line list, reindentation, mixed content, whitespace-only content, declaration/doctype/PI/comment/CDATA all surviving, folding, and attribute quoting.
  • Full suite: 848 passed, 0 failed, 8 skipped (the same 8 as on main).

@andiwand
andiwand force-pushed the feat/xml-source-view branch from 0dec71d to 9ad65a5 Compare August 9, 2026 20:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0dec71d5f3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/odr/internal/html/xml_file.cpp Outdated
Comment thread src/odr/internal/html/frontend.cpp
@andiwand
andiwand force-pushed the feat/xml-source-view branch from 9ad65a5 to 675339a Compare August 9, 2026 20:50
An xml file opened as `text_file` and rendered through the numbered line list.
For the files anyone opens on purpose — `content.xml`, `document.xml`, anything
a writer emitted rather than a human — that is one line several megabytes wide.

It now opens as `xml::XmlFile` and renders as a source view: reindented,
highlighted, and foldable through `<details>`/`<summary>` with no script. The
encoding comes from the declaration where the file names one, so a document
that is not UTF-8 decodes instead of arriving as mojibake, and one we can name
but not decode has no tree at all rather than a broken one.

Mixed content is the one non-trivial rule and it is left alone: an element
holding any text keeps its children on the line they came in on, because
nothing short of a schema tells significant whitespace from the other kind.

Detection is unchanged except in where it ends up — xml sits last in the
unknown-type path, after svg, so anything with a more specific reading keeps
it, and a malformed file still falls through to the line list.

Stages 1 and 2 of `src/odr/internal/xml/PLAN.md`; the size budget and the
archive seam are not in here, and neither are the test-data samples.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V1PApAFAK7q2yN7Rd2UUpr
andiwand and others added 2 commits August 10, 2026 07:31
… its spaces

A value holding a double quote switched the delimiter to the single one, which
is wrong the moment the value holds an apostrophe too: the source view wrote
`<a v='can't say "hi"'>` and the boundary landed in the middle of the value. The
delimiter is now the one the value does not carry, and where it carries both,
the double quote goes in as `&quot;`.

The value span also inherited html's whitespace folding, so `"a  b"` was shown
as `"a b"` in a view whose whole point is the source text. It preserves it now,
along with the declaration, the doctype and a processing instruction, which read
back the same way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014ER7HcXMJZ1Q8azxx4wJRS
The parse flags were a free function in the xml header so the html service
could reach them, which meant the service parsed the file a second time —
once to recognise it, once to render it. XmlFile now keeps what its
constructor parsed and lends it out as a const reference, and the service
casts the decoded file back to XmlFile to get at it.

Parsing only the head of the file the way csv's probe does is not on offer:
pugixml is dom-only, and a sniff would give up the contract that whatever
opens as xml renders. Nor is handing out a copy — pugi::xml_document is
non-copyable, and reset(proto) is a deep clone no cheaper than reparsing.

The price is memory: the dom is roughly twice the file, held for as long as
the XmlFile is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012MotjyxkDXsdEnhLrv8b7W
@andiwand
andiwand enabled auto-merge (squash) August 10, 2026 09:33
@andiwand
andiwand merged commit 1daf2f1 into main Aug 10, 2026
36 checks passed
@andiwand
andiwand deleted the feat/xml-source-view branch August 10, 2026 09:45
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