🐛 Accurate directive line mapping and source-attributed nested renders#1175
Open
chrisjsewell wants to merge 7 commits into
Open
🐛 Accurate directive line mapping and source-attributed nested renders#1175chrisjsewell wants to merge 7 commits into
chrisjsewell wants to merge 7 commits into
Conversation
Directives rendered by MyST now receive a document-relative `content_offset`
(the 0-based absolute line of the first content line, == 1-based first
content line minus 1), and `content.items` carrying each line's 0-based
absolute document line -- exactly the convention docutils' own rST parser
provides. Previously `content_offset` was the body-relative `body_offset`
(0, 1, 2... from the fence) and items were relative 0, 1, 2..., so
extensions computing `content_offset + N` (e.g. sphinx-jinja2)
mis-attributed reporter warnings.
This is coupled with `MockState.nested_parse`, whose `input_offset` is now
an absolute document line (dropping the `self._lineno +` shift). For the
standard `nested_parse(self.content, self.content_offset, node)` idiom the
rendered line is `position + body_offset` either way, so every nested node
line is byte-identical -- no rendering fixtures change.
Free line-attribution fixes made correct while here:
- `{epigraph}` attribution `.line` was one too small (latent off-by-one);
it now equals the attribution's true document line, matching docutils'
`parse_attribution` (`lineno = 1 + line_offset`).
- `{parsed-literal}` `.line` (docutils sets `content_offset + 1`) is now the
true absolute content line instead of a hardcoded 1.
Internal docs directives (`_docs.py`) that passed a literal `0` to
`nested_parse` now pass `self.content_offset`, preserving their
render-at-the-directive placement under the new absolute semantics.
Behaviour note for extension authors: `MockState.nested_parse` now
interprets `input_offset` as an absolute document line
(docutils-consistent); callers that passed a literal directive-relative
offset should pass an absolute one (the common `self.content_offset` idiom
is unaffected). `DirectiveParsingResult.body_offset` docstring clarified
(relative to the line after the opening line; `0` = next line, `-1` =
merged); its semantics are unchanged.
Tests: new `tests/test_directive_line_mapping.py` pins the contract across
fence shapes (bare, merged, options, YAML, nested, list-embedded, colon,
argument) at multiple document positions, against docutils rST ground
truth, plus the eval-rst pin, the epigraph/parsed-literal fixes,
nested-line invariance, the `reporter.warning(line=content_offset + N)`
idiom, and a `{role}` smoke test. The existing
`test_directive_block_text_rst_parity` asserted the old body-relative
`content_offset` (3); updated to the document-relative value (4) --
`body line` is on document line 5.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Add a supported mechanism for extensions to render generated content (an included file, a template's output, ...) at the current document position, attributing both the rendered nodes and any warnings to a caller-specified source path. `DocutilsRenderer.nested_render_text` gains a `source` keyword. While it is set, the document/reporter are pointed at that path (so node `source` stamping and docutils reporter warnings attribute there) and the path is pushed onto a new renderer attribution stack consulted by `create_warning` (so the sphinx logger attributes there too). The override is restored on exit -- including after an exception -- and nests re-entrantly, so a source render may itself contain further source renders (e.g. nested includes). The method is now documented as supported API, with its `lineno` line-space semantics and the MyST-not-rST parsing caveat spelled out. `warnings_.create_warning` gains a matching `source` keyword: - sphinx branch: an explicit `node` still wins; otherwise `source` produces a location *string* `"source:line"` (or `"source:"` when there is no line). The colon is required -- sphinx passes a colon-containing string through verbatim, whereas a colon-less string (or a `(source, line)` tuple) is run through `env.doc2path`, which would mangle an explicit source path. The default `(env.docname, line)` path is byte-identical to before. - docutils branch: `source` (with `line`) is passed straight to `reporter.warning`, which docutils honours as an explicit override. The two modes are therefore symmetric: a warning inside generated content is reported against the generating source in both. Consumers wired in-repo: - `MockIncludeDirective.run` now delegates the document/reporter dance to `nested_render_text(..., source=str(path))`, and renders at `startline` rather than `startline + 1`. This fixes the include line off-by-one: a warning on line 3 of an included file was reported at line 4. In sphinx mode it was additionally attributed to the *parent* document (a side effect of the logger switching to `env.docname`); it now cites the included file. The relative-images/relative-docs md_env handling stays in the directive, as it resolves against the containing document's directory. - `MockStateMachine.insert_input(input_lines, source=None)` renders generated MyST at the directive's position with a docutils-compatible signature; with `source` it attributes to `source:1..n`, otherwise it renders in the document's line-space. Content is parsed as MyST (not rST) and rendered immediately into the current node (before any nodes the directive returns -- identical to docutils for the common `return []` pattern). - `MockState.renderer` / `MockStateMachine.renderer` read-only properties bless the existing `_renderer` as the supported way for extensions to reach these APIs from a directive. Known limitation (documented): deferred document-level warnings (e.g. duplicate reference definitions, emitted in `_render_finalise`) escape a render-scoped override and attribute to the containing document; inline tokens carry no per-line maps. Also fixes a stale docs URL in `MockState.__getattr__` (kept with the code). Tests: new `tests/test_renderers/test_nested_render.py` pins include attribution (sphinx via `sphinx_doctree`, docutils via `publish_doctree` + `tmp_path`), the source override in both modes (warnings + node `.source` stamping), `insert_input` (ordering, source attribution, document-space fallback, and the `renderer` properties), and restoration (nested override-in-override attributing innermost then restoring outward, restoration after a directive raises, and restoration when the render itself raises mid-way). It lives under `tests/test_renderers/` so its sphinx builds run after the docutils role fixtures (the same ordering the existing `test_fixtures_docutils`/`test_fixtures_sphinx` split relies on). Fixture regenerated for the off-by-one fix (hand-verified against the source): - `tests/test_renderers/fixtures/mock_include_errors.md`: the unknown-role warning for the single-line `bad.md` moves from `bad.md:2` to `bad.md:1` (its true line); the `bad.md` attribution itself is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Add `docs/develop/extending.md` (in the developer toctree after
`architecture.md`), documenting the supported API for rendering generated
content from a directive:
- reaching the renderer via `self.state.renderer` /
`self.state_machine.renderer`;
- `nested_render_text` (its `lineno` line-space semantics, with a worked
example, and `source=` attribution);
- `insert_input` (the docutils-compatible signature, the MyST-not-rST caveat,
and the immediate-render ordering note);
- what `content_offset` now means (document-relative; the `content_offset + N`
idiom reports true document lines);
- documented limitations (deferred document-level warnings attribute to the
containing document; inline tokens carry no per-line maps).
The page is narrative and cross-references the API objects with `{py:meth}`/
`{py:obj}`/`{py:func}` roles; autodoc2 renders the docstrings, which are the
source of truth.
Add an `## Unreleased` CHANGELOG section (✨ New Features / 🐛 Bug Fixes /
📚 Documentation) covering the document-relative `content_offset` fix (with the
epigraph/parsed-literal corrections and the absolute `nested_parse` offset
note), the include attribution + off-by-one fix, the `source=` nested-render
API, `insert_input`, the `renderer` properties, and this page.
The stale `MockState` docs URL fix ships with the code in the preceding commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
In `MockIncludeDirective.run`, the `:start-after:` handling advanced the
included file's `startline` by `split_index + len(split_on)` -- a *character*
index into the file content -- rather than a line count. Because `startline`
is then used as the 0-based line shift for `nested_render_text`, a warning
inside the retained segment was reported far past the end of the file (e.g. a
directive truly on line 4 of a 5-line included file warned at `inc.md:19`).
Advance by the number of newlines in the consumed prefix instead:
startline += file_content.count("\n", 0, split_index + len(split_on))
The remaining text's first (possibly partial) line still lies ON the marker's
line, so counting the consumed newlines makes `nested_render_text` report the
TRUE file lines -- consistent with the `:start-line:` path. This deliberately
differs from docutils, whose `:start-after:` reports lines relative to the
retained segment; MyST reports true file lines. `:end-before:` only truncates
`file_content` and does not touch `startline`, so it is unaffected.
Tests (`tests/test_renderers/test_nested_render.py`): a `:start-after:` include
whose warning-triggering line sits at a known true file line asserts the true
line is reported and the old character-index-derived line is absent, plus a
combined `:start-after:` + `:end-before:` case.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Follow-ups from the review of the nested-render/attribution work. No runtime
behaviour changes -- tests, docs, docstrings and a changelog note only.
- Tests: make `create_warning`'s `source` kwarg standalone-load-bearing. The
existing extension-style tests always run with the renderer's ambient reporter
override active, which masks the docutils-branch `source` kwarg (dropping it
survives the suite). Add `test_create_warning_source_docutils_arm` (calls
`create_warning` directly on an override-free `new_document`, asserting the
`source`/`source:line` attribution and that the document's own source does not
leak), and `test_create_warning_source_sphinx_arm_no_line` (sphinx build,
`line=None` -> logged `source:` trailing-colon form, not doc2path-mangled).
- Tests: parametrize `test_insert_input_renders_at_position` over `list[str]`
and a docutils `StringList` input, pinning that `insert_input` accepts both.
- Tests: `test_downstream_reporter_warning_line` now also asserts the old,
body-relative `<string>:{body_offset + 1}:` location is absent (guarded to
skip if it would coincide with the true line -- it does not for these cases).
- Changelog: add a migration note for extension authors under the
`content_offset` bullet -- `MockState.nested_parse`'s `input_offset` is now an
absolute document offset; the standard idiom is unchanged, but literal
relative offsets (e.g. `0`) now place docutils-identically (autodoc/autosummary
`offset=0` renders at the document top), and `self.lineno + self.content_offset`
now double-counts (as it always did under reStructuredText).
- Docs/docstring: `insert_input` -- state plainly that docutils requires `source`
positionally while the mock makes it optional; that inserted content lands
before the directive's returned nodes (docutils splices after; identical only
for `return []`); and that a `StringList`'s per-line `(source, offset)` items
are not preserved (joined and attributed uniformly to `source`).
- Docs: fix the `{eval-rst}` cross-reference to the `#`-prefixed label form used
by sibling docs, and note in the limitations that `document["source"]`/the
reporter are repointed at the override during a source-attributed render
(extensions resolving paths against `document["source"]` mid-render see it --
the same behaviour `{include}` has always had).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
- docs/develop/extending.md: add a "Stability" subsection promising the page's
APIs (`nested_render_text`/`source`, `insert_input`, the `renderer` properties,
the document-relative `content_offset` contract) are supported public API.
- docs/develop/extending.md: document `{eval-rst}` semantics (real docutils state
machine; document-absolute `lineno`/`content_offset`; `content_offset + N`
works identically), pinned by `test_eval_rst_content_offset_pinned`.
- CHANGELOG.md (Unreleased): tighten to house-bullet style keeping every fact,
and correct the autodoc/autosummary offset note (rST-consistent offset value,
not byte-identical `StringList` attribution).
- Code polish: `_attribute_to_source` guards empty-string source (`if not
source`); `create_warning` docstring now documents all parameters.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
The assertion in test_include_attribution_sphinx compared against a forward-slash <src>/inc.md path, but on Windows the include path is joined with a backslash (<src>\inc.md), failing both windows-latest CI jobs. The CI logs show the feature itself behaves correctly on Windows (true file, true line); only the assertion was separator-naive. Normalise backslashes in the captured warnings before asserting. The other sphinx-mode tests assert verbatim caller-supplied paths and the docutils-mode tests use prefix-independent substring matches, so this was the only separator-sensitive assertion (all other tests passed on windows-latest). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes three directive-API fidelity gaps characterised while making sphinx-jinja2's warnings line-accurate (myst-parser 4.0.1/5.1.0, re-verified against master post-#1164).
content_offsetis now document-relative (rST convention)Directives run under MyST received
content_offset = body_offset(directive-relative), so the conventionalcontent_offset + Nidiom reported lines near the top of the file wherever the fence sat. Directives now receive the 0-based document line of their first content line, andself.content.itemscarry per-line absolute lines — both exactly what docutils' rST parser provides. Coupled with this,MockState.nested_parsetreatsinput_offsetas an absolute document offset. For the standardnested_parse(self.content, self.content_offset, node)idiom the rendered position is unchanged (the two old offsets summed to the same number), so no rendering fixture changes; the external contract is what's fixed.Free corrections that fell out:
{epigraph}attribution.line(was one too small),{parsed-literal}.line(was hardcoded 1), and myst's own_docs.pydirectives.Source-attributed nested renders (supported API)
DocutilsRenderer.nested_render_text(text, lineno, source=...)attributes rendered nodes and warnings (sphinx logger + docutils reporter, symmetrically) to a caller-specified path, exception-safe and re-entrant. Since #1114 switched warning locations toenv.docname, the document/reporter override dance was inert under sphinx —{include}itself was affected: warnings inside included markdown were logged against the including document, one line off. Includes now cite the included file at the true line (the #1164 "tracked separately" off-by-one), and:start-after:no longer derives line numbers from a character index (previously past-EOF lines likeinc.md:19in a 5-line file).Supported insertion API
MockStateMachine.insert_input(input_lines, source=None)— docutils-compatible signature; parsed as MyST (documented caveat), rendered at the directive's position, attributed tosource:1..n.MockState.renderer/MockStateMachine.rendererproperties bless the access path.docs/develop/extending.mddocuments the API with an explicit stability promise,linenoline-space semantics, eval-rst semantics (already document-absolute; now pinned by test), and limitations (deferred document-level warnings; inline tokens).Behaviour notes for extension authors (also in changelog)
nested_parsewith literal relative offsets (e.g. autodoc/autosummary'soffset=0) now places content at the document top, as under rST.self.lineno + self.content_offsetarithmetic now double-counts — as it always did under rST (sphinx-design's badges/buttons hit this latent pattern).Verification
content_offset + 2now warns atindex.md:11(wasindex.md:3).Downstream (sphinx-jinja2): the
isinstance(state_machine, StateMachine)gate can become a version check, the override dance collapses ontonested_render_text(..., source=template_path), and itstest_myst_render_error_locationcan assert line-accurate expectations.