Skip to content

feat(markdown): open a markdown file as a text document - #734

Open
andiwand wants to merge 1 commit into
mainfrom
feat/markdown
Open

feat(markdown): open a markdown file as a text document#734
andiwand wants to merge 1 commit into
mainfrom
feat/markdown

Conversation

@andiwand

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

Implements stages 1 to 3 of the markdown plan that was already in the repo
(src/odr/internal/markdown/PLAN.md). A .md file decodes to a TextRoot, so
the generic html renderer and every binding get it without format-specific code
— rather than a markdown→html renderer next to html/text_file.cpp, which
would produce html only, with no element api and nothing for
JNI/embind/pybind/ObjC.

CommonMark plus the GitHub extensions, parsed with md4c (new dependency,
md4c/0.5.2, MIT): headings, paragraphs, soft and hard breaks, lists (ordered
counting from their start), block quotes, code blocks, emphasis, strong,
strikethrough, inline code, links, GFM tables and task lists.

FileType::markdown moves from FileCategory::text to
FileCategory::document with DocumentType::text. detect_by_content stays
false — markdown has no signature, and a content probe for it is a probe
for "prose with occasional punctuation" — so the only way in is
DecodedFile(file, FileType::markdown), and a .md still opens as a text file
by default. Callers route on the file name, which is what they already have.

Design rationale and the mapping table are in the new
src/odr/internal/markdown/AGENTS.md;
PLAN.md is marked up with what it got wrong.

Three things the plan had wrong, found by rendering a sample

The element-tree tests would not have caught any of these on their own.

  1. A heading needs a bold span, not just a bold paragraph style.
    html::translate_paragraph takes only font family and size from a
    paragraph's text style (translate_block_font_style) — weight, slant and
    decoration are expected on the spans inside. Headings rendered unbolded.
    Only the weight goes on the span: the paragraph's size is already in em,
    and repeating it compounded 2em into 4em. A TH cell has the same
    shape.
  2. A tight list item needs a paragraph opened for it. md4c omits
    MD_BLOCK_P inside a tight list item, and html::translate_list_item
    writes the marker into the item's first paragraph — so without one, every
    bullet was silently dropped.
  3. Table columns cannot be children of the table. The rows are; one sibling
    chain cannot carry both, and the renderer walks table_first_column's
    siblings, so a 3-column table emitted six <col>s — three columns and three
    rows. Now on their own chain via append_column, the shape odf uses.

The plan's fourth claim — that html::translate_list hardcodes <ul> — is
stale; the renderer takes the label from list_item_marker now, so ordered
lists number themselves.

Known gaps, documented rather than dropped quietly

  • Named entities beyond the five predefined XML ones (plus &nbsp;) stay
    literal.
    Numeric references resolve. The plan asked whether the conan
    package exports the md4c-html component whose entity_lookup resolves the
    ~2000 named entities: it does, and the symbol is in the archive, but
    entity.h is not shipped — so using it means declaring md4c's private
    MD_ENTITY layout ourselves, where a mismatch corrupts silently instead of
    failing to link. Left out; worth revisiting before claiming CommonMark
    conformance.
  • Raw html is dropped, block and inline. There is no passthrough element,
    and inventing one means deciding what Text::content() returns for it in
    four bindings.
  • Images are transparent — the alt text flows through as text. Stage 4.
    Note for whoever picks it up: html::translate_image writes the <img> at
    width:100%;height:100% absolutely positioned inside the frame's div, so a
    frame with no dimensions renders nothing at all, and markdown carries none.
  • Horizontal rules have nothing in the model.
  • A hyperlink's href is passed through and only escape_attributed by the
    renderer, exactly as an odt's is — this is a second consumer of the existing
    document-link policy, not a third policy.

Testing

20 gtests in test/src/internal/markdown/markdown_file_test.cpp, all inline
string literals in and element tree out, no fixture files. The rendered html
was eyeballed end to end for a document exercising every construct above.

FileTypeCapabilities.declaration_matches_the_engines and
color_scheme_matches_the_html now exercise the .md already in odr-public,
so nothing needs adding to the test-data repo.

Before merging

  • The reference output needs regenerating. odr-public/README.md was
    skipped by html_output_test while the row declared open = false, and now
    renders.
  • The commit subject is feat, i.e. a minor bump. The category flip is
    api-visible, but the row was classification-only and declared no capabilities,
    so nothing could hold a decoded markdown file to be broken — the plan's own
    reasoning. If you read it as breaking, the subject needs a ! and this
    becomes v7.0.0.

Implements stages 1 to 3 of `src/odr/internal/markdown/PLAN.md`: a `.md`
decodes to a `TextRoot`, so the generic html renderer and every binding
get it without format-specific code.

CommonMark plus the GitHub extensions, parsed with md4c (new dependency):
headings, paragraphs, lists, block quotes, code blocks, emphasis, links,
tables and task lists. Raw html, images and horizontal rules are not
modelled yet.

`FileType::markdown` moves from `FileCategory::text` to
`FileCategory::document` with it. `detect_by_content` stays false —
markdown has no signature, and a content probe for it is a probe for
prose — so the caller routes on the file name and a `.md` still opens as
a text file by default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XeD48ZqWKpgMsQCvkiSRPn
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