🐛 FIX: attrs: unterminated groups, class accumulation, closing-token merge

[chrisjsewell]

Summary

• Unterminated attribute groups no longer swallow the author's text. parse() treated running off
  the end of the string as success, so `a`{ rendered <p><code>a</code></p> — the brace and
  everything after it were deleted from the token stream. It now raises ParseError, which all three
  call sites already catch and decline. In the same change, a % comment that no second % can close
  is ended by }, so comments closed only by } keep working — they only "work" today by running off
  the end, which is the very path being removed.
• Classes accumulate across adjacent groups on spans and links, as they already do on inline code
  and images. [a]{.x}{.y} gave class="y"; it now gives class="x y".
• Block attributes are no longer merged onto a closing token. > {.a} alone in a blockquote
  rendered </blockquote class="a">; the group is now dropped, exactly as a group with nothing after
  it already is.
• Docs: say what actually happens to a backslash inside a quoted value — it is kept.

Compatibility

Only input that v0.7.0 currently loses characters on is affected. That is not a claim by
assertion: it is measured twice, exhaustively, below under "Evidence".

Unterminated groups. Four shapes:

1. An unterminated group after an eligible element now renders literally instead of being deleted:
   `a`{ -> <code>a</code>{, `a`{.a -> <code>a</code>{.a, [a](u){, [a](u){.x,
   `a`{.a}{, `a`{k="x. `a`{.a\nmore recovers a whole word, because parse() is
   handed the rest of the inline source and the run-off consumed all of it. `a`{ .a b shows the
   related sub-case: attributes scanned out of an unterminated group are no longer applied
   (class="a" used to be applied while { .a b was deleted).

2. [a]{ and [a]{.x no longer produce a <span> at all; the text is left literal. This is a shape
   change, not only recovered characters — and it is on the worst input of the set: at present the
   span is created, {.x is eaten, and the class it asks for is thrown away, because {.x
   compiles to an empty attribute dict.

3. {k="a\} forms — a backslash-escaped } inside a quoted value, which therefore never terminates
   — now render literally instead of being consumed. ({k="a} without the escape is unaffected: it
   already raised and rendered literally. Where such a group had already produced attributes, e.g.
   {.a k="x\}, those attributes are no longer applied — that is shape 1.)

4. % comments. A comment ends at the next %. Only if no % occurs anywhere in the rest of the
   scanned string does it end at the next }, which then also ends the attribute group. That second
   clause is what keeps comments closed only by } working: `a`{%c}, `a`{.a %c},
   [a]{%c}, [a](u){%c}, {%x} and {.a %c} above a paragraph all render exactly as they do
   today, and today they do so only because the scanner runs off the end. Where this changes anything,
   it recovers text v0.7.0 deleted: `a`{%a}b keeps the b, `a`{.a %c} and {more} keeps
   and {more}, `a`{%a}{.b} now applies the second group, and `a`{%c} tail keeps its
   space (at v0.7.0 parse('{%c}') returned (4, {}) because it ran off the end, and the caller's
   state.pos += new_pos + 1 then over-advanced by one character; it now returns (3, {}), the index
   of the }, so the + 1 lands exactly past it).

   The rule is deliberately lazy, and the reason matters. Ending a comment at every } — what
   djot.js does — changes input this plugin accepts today: {% c } % .b} above a paragraph renders
   <p class="b">para</p> at v0.7.0 and would silently lose the class, and because the "was the whole
   line consumed" check in _attr_block_rule is disabled, {% see } below %{x} on its own line —
   a paragraph today — would be consumed and render to nothing. The lazy rule leaves both exactly as
   they are.
   Its cost, stated plainly: a % later in the same paragraph (for an inline group) or the same line (for a block group) keeps a brace-closed comment open, so
   `a`{.a %c} and 100% sure now renders literally as <code>a</code>{.a %c} and 100% sure
   instead of applying class="a" and swallowing the tail. Nothing is lost, but nothing is applied
   either. A fixture row pins that, titled
   comment: a later percent keeps a brace-closed comment open.

Class accumulation. Span and link join classes across groups now: [a]{.x}{.y} ->
class="x y", [a](u){.x}{.y}{.z} -> class="x y z", and [a]{.x #p}{.y #q} ->
id="q" class="x y" (id was already last-wins and stays that way; ordinary keys are still
last-wins). Inline code and images already behaved this way and are untouched. Concatenation is not
de-duplicated: [a]{#a .a}{#b .a .b other=c}{other=d} gives class="a a b". That matches the
existing image behaviour in this repo (the merging attributes fixture expects class="a b x x g")
and djot's insert_attribute, which appends with a space and no de-duplication — so the one existing
fixture row this PR changes, spans: merge attributes, has its expected class="a b" updated to
class="a a b". That is the only existing expectation that moves.

Closing-token merge. Every output this changes is invalid HTML today —
</blockquote class="a">, </li class="a"> — so nothing a user can depend on changes.
> {.a} alone in a blockquote, - {.a}, 1. {.a}, > {.a}\n> {.b} and
{.a}\n{.b}\n> {.c} all drop the trailing group now. Controls are unchanged: > {.a}\n> para still
attaches to the inner paragraph, an attrs block before a fence, heading, hr, list, blockquote or
table still attaches to it, and {.a} with nothing after it was already dropped. Note the drop is
silent — no warning is emitted, consistent with the existing behaviour for a group with nothing
after it.

Docs. No behaviour change.

Evidence for "only input that currently loses characters is affected"

• Exhaustive parse() differential, every string { + up to 7 characters of {}.%a#
  (335,923 strings), v0.7.0 against this branch:
  0 inputs that v0.7.0 parsed to completion changed their position or attributes; 0 inputs
  v0.7.0 rejected are now accepted; 0 inputs v0.7.0 parsed to a closing brace are now rejected. Of the 31,161
  strings v0.7.0 ran off the end of (the buggy path), 15,901 now raise — the group is left literal —
  and 15,260 now terminate with exactly the attributes v0.7.0 produced, with the tail recovered as
  text instead of being swallowed.
• Render fuzz, 80,000 documents (40,000 comment-heavy, 40,000 wide-alphabet):
  0 documents lose text; 5,936 differ, and every one of them preserves strictly more of the
  source's characters than v0.7.0 did. (Metric: the multiset of the document's own characters that
  survive into the rendered text or into an attribute value.)
• Both harnesses were checked for non-vacuity against a variant that ends a comment at every }:
  they report 34 accepted inputs with changed attributes, 3,901 new acceptances, and 194 documents
  losing text. On this branch, zero of each.

Tests

All rows are in tests/fixtures/attrs.md; each fix and its rows are in one commit, and each row was
run RED on the unfixed tree and GREEN after. tests/fixtures/attrs.md is excluded from every
pre-commit hook by the repository's own exclude: pattern (test.*\.md), so it was checked by hand:
a single trailing newline, no trailing whitespace on any added line.

• Unterminated groups — 29 rows, 13 RED before / green after, 16 pins that are green at v0.7.0 and
  after. Examples: unterminated: inline code (`a`{) failed with
  expected <p><code>a</code>{</p>, got <p><code>a</code></p>;
  unterminated: following text is not consumed with
  expected <p><code>a</code>{.a\nmore</p>, got <p><code class="a">a</code></p>;
  spans: unterminated attributes with a class are not a span with
  expected <p>[a]{.x</p>, got <p><span>a</span></p>;
  comment: text after a brace-closed comment is kept with
  expected <p><code class="a">a</code> and {more}</p>, got <p><code class="a">a</code></p>.
  The fixture file had no % rows at all before this PR, so the suite could not have caught a
  comment regression. Eight of the new pins are load-bearing for the comment half — with the raise
  applied but the comment rule removed, these fail: comment: block, comment: block after a class,
  comment: inline code, comment: after a class, comment: link,
  comment: closed by a brace, following text is kept,
  comment: text after a brace-closed comment is kept, spans: comment.
  Seven more guard the laziness: with a comment ended at every } instead, these fail —
  comment: block, attributes after a brace inside the comment ({% c } % .b} would lose its class),
  comment: block, a brace inside the comment does not terminate the group
  ({% see } below %{x} would render to nothing),
  comment: a brace inside a comment closed by a percent,
  comment: attributes after a brace inside the comment,
  comment: an empty comment containing a brace,
  comment: a later percent keeps a brace-closed comment open,
  spans: a brace inside an unterminated comment is not a span.
• Class accumulation — 6 rows RED before, green after; e.g.
  spans: merge classes from two groups failed with
  expected <p><span class="x y">a</span></p>, got <p><span class="y">a</span></p>, and
  links: merge classes from three groups with expected class="x y z", got class="z". Plus the one
  updated expectation on spans: merge attributes (class="a b" -> class="a a b"), which fails
  with the old expectation once the fix is in.
• Closing-token merge — 5 rows RED before, green after; e.g.
  block: attrs last in a blockquote failed with
  expected <blockquote></blockquote>\n<p>para</p>, got <blockquote></blockquote class="a">\n<p>para</p>.
  The rows assert the full rendered string on purpose: a guard that skips the merge but forgets to
  pop the token renders < class="a">, still invalid, and all five rows catch it (verified by
  mutation). Controls: block: attrs followed by a paragraph in a blockquote and the existing
  block fence row.
• Docs — one fixture row, quoted value keeps a backslash escape:
  `a`{k="a\"b"} -> <p><code k="a\&quot;b">a</code></p>, green before and after. It pins the
  behaviour the docstring describes; the docstring prose itself is not covered by any test (the repo
  has no docstring harness), so treat it as reviewed text, not tested text. Note the example uses
  double backticks deliberately: inside single backticks docutils eats the backslash and the example
  would contradict itself.

Gates

• mdit-py-plugins, Python 3.10 (project floor): 553 passed, exit 0. Python 3.13: 553 passed,
  exit 0. (511 at v0.7.0 + 42 new rows.)
• pre-commit run --all-files: exit 0, all hooks pass, nothing rewritten. That includes the mypy
  hook, which type-checks against markdown-it-py~=3.0 as configured.
• MyST-Parser's full suite against this patched plugin (editable install, mdit_py_plugins
  resolving to the patched tree): 1245 passed, 0 failed, 11 skipped, in three chunks
  (23 / 391+8 skipped / 831+3 skipped), with the same 11 skip lines as against released v0.7.0.
  Zero downstream churn.

Changelog lines

- 🐛 FIX: attrs: an unterminated attribute group no longer swallows the text after it
- 🐛 FIX: attrs: classes from adjacent attribute groups are combined on spans and links
- 🐛 FIX: attrs: an attributes block at the end of a container is no longer merged onto the closing token
- 📚 DOCS: attrs: state that a backslash escape is retained in a quoted value

Follow-ups noticed

• djot.js alignment on comments. djot.js ends a % comment at any }
  (attributes.ts, handlers[State.SCANNING_COMMENT]); the Lua implementation this module cites as
  its source ends one only at %. This PR does neither exactly, on purpose: matching djot.js changes
  attributes on input accepted today ({% c } % .b} loses its class) and, while the full-line check
  is disabled, deletes whole block lines ({% see } below %{x}). Aligning would want the full-line
  check enabled first. Receipts: 34 accepted inputs with changed attributes and 194 fuzzed documents
  losing text, measured against that variant.
• A span's own attribute group bypasses allowed=: attrs_plugin(spans=True, allowed=["id"]) still
  renders [a]{.x} as <span class="x">, with no insecure_attrs meta. _span_rule is registered
  without allowed and assigns token.attrs directly instead of going through _add_attrs.
• meta["insecure_attrs"] is overwritten rather than merged across groups: with allowed=["id"],
  [a](u){.x}{.y} leaves only {'class': 'y'}. (Related, and disclosed for completeness: with
  allowed=, a span's insecure_attrs["class"] now carries the accumulated x y rather than the
  last group only. The rendered HTML is identical; this is visible only because of the bypass above.)
• parse() is given state.src[state.pos:] rather than a posMax-bounded slice, so it can read past
  the current inline sub-range. Today every over-read still ends in ParseError — [x `a`{.a](u)
  renders <a href="u">x <code>a</code>{.a</a></p> — and this PR adds a row pinning that.
• Class accumulation does not de-duplicate (class="a a b", class="a b x x g"). djot does not
  either, so this is deliberate, but it may deserve a note in the docs.
• A newline inside a quoted value truncates it to the first line: `a`{k="x\ny"} ->
  <code k="x">.
• Keys that are not valid HTML attribute names are accepted: `a`{-=v} -> <code -="v">,
  `a`{1=v} -> <code 1="v">.
• A span inside a link label breaks the link: [a [b]{.c} d](u) ->
  <p>[a <span class="c">b</span> d](u)</p>.
• ParseError positions are swallowed at all three call sites; surfacing them as warnings would be a
  useful diagnostic, including for the silent drop this PR's third fix produces.
• On backslash escapes, this plugin diverges from djot: djot's scanner also keeps the backslash, but
  ast.lua's insert_attributes then applies :gsub("\\(%p)", "%1") ("resolve backslash escapes"),
  so djot's value for {k="a\"b"} is a"b while this plugin's is a\"b. This PR documents the
  current behaviour rather than changing an accepted input's value; aligning with djot would be a
  behaviour change worth its own discussion.

Out of scope by ruling

• De-duplicating classes — would also change the existing image row class="a b x x g" ->
  "a b x g"; a behaviour change on accepted input.
• A newline inside a quoted value — raising or joining changes accepted input.
• The disabled "full line consumed" check in _attr_block_rule ({.a}{#b} on one line applies
  only the first group). It stays disabled: enabling it turns such a line into a paragraph
  ({.a}{#b}\npara -> <p>{.a}{#b}\npara</p> instead of <p class="a">para</p>, measured), which is
  a behaviour change on accepted input. For the record, the reason it "was not working in some
  instances" is an indexing bug, not the unterminated-group behaviour: the commented-out
  if (maximum - 1) != new_pos compares new_pos, an index into the slice state.src[pos:maximum],
  with maximum - 1, an index into the full source; they agree only when pos == 0. Measured at both
  pins: at v0.7.0 the check as written fails 3 tests and the corrected form (maximum - pos - 1)
  passes the whole suite; on this branch the check as written fails 9 fixture rows plus
  test_attrs_allowed, and the corrected form still passes the whole suite (75 passed). No row added
  by this PR pins the disabled check.
• Keys that are not valid HTML attribute names, a span inside a link label, surfacing
  ParseError positions, and the allowed= bypass on spans — each needs its own discussion.
• The silent drop of a trailing group in a container — this PR keeps the existing silence rather
  than introducing a new warning.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (d11bdaf) to head (3f5d498).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
+ Coverage   92.80%   93.65%   +0.85%     
==========================================
  Files          31       40       +9     
  Lines        1835     2286     +451     
==========================================
+ Hits         1703     2141     +438     
- Misses        132      145      +13     

Flag	Coverage Δ
pytests	93.65% <100.00%> (+0.85%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.