From 59480c2cfcf2d62a7654683f2dfa0f017669db36 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Tue, 14 Jul 2026 22:31:13 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20FIX:=20report=20accurate=20sourc?= =?UTF-8?q?e=20lines=20for=20directives=20and=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix three classes of wrong source-line attribution: - Directive option warnings (unknown key, invalid value, bad YAML, comments) now point at the offending option's own source line, for both `---` and `:option:` block styles, instead of the directive's opening line. Unknown option keys are now warned once per key, at its own line. The `---` options block is now split line-based: any text trailing a closing delimiter still becomes body content (as previously), but is kept as its own line, so it and all subsequent body lines map exactly to their source lines. A new `options_to_tokens` API in `myst_parser.parsers.options` exposes the tokenizer's per-key positions, and its `State` now records the lines of `#` comments (`options_to_items` is now a thin wrapper around it). Directives run from synthesized, non-source content (HTML images and admonitions) attribute option warnings to the element's line. - Directives with body content on the opening line (no-argument directives such as `{note}`) reported every body warning one line too low; `body_offset` is now `-1` for the merged first line. - `document.current_line` was set once per directive and never reset, so docutils' `Node.setup_child` stamped the most recent directive's line (or 0) onto every subsequently-created line-less node, e.g. every `Text` node - the root cause of tools such as sphinxcontrib-spelling and gettext reporting 'line 0' or the last directive's line. The renderer now keeps `current_line` in sync as nodes are created. Also: - `block_text` passed to directives is now the full directive text (opening fence, options, body, closing fence), matching what an equivalent rST directive receives, so third-party directives that inspect it see rST-consistent content. - `include` with `:literal:` now sets the literal block's line to `start-line + 1` (docutils parity) instead of a hardcoded 1. Closes #546 --- AGENTS.md | 9 +- docs/configuration.md | 8 + myst_parser/mdit_to_docutils/base.py | 29 +++- myst_parser/mocking.py | 3 +- myst_parser/parsers/directives.py | 112 +++++++++---- myst_parser/parsers/options.py | 49 ++++-- tests/test_docutils.py | 156 ++++++++++++++++++ .../fixtures/directive_options.md | 2 +- .../fixtures/directive_parsing.txt | 18 +- tests/test_renderers/fixtures/myst-config.txt | 4 +- .../fixtures/reporter_warnings.md | 75 ++++++++- tests/test_renderers/test_parse_directives.py | 121 +++++++++++++- .../test_sphinx_builds/test_gettext.pot | 4 +- .../test_gettext_additional_targets.pot | 4 +- 14 files changed, 527 insertions(+), 67 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c7681c5d..b9ce18dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,15 +142,20 @@ def parse_directive_text( first_line: str, content: str, *, + line: int | None = None, validate_options: bool = True, -) -> tuple[list[str], dict[str, Any], list[str], int]: + additional_options: dict[str, str] | None = None, +) -> DirectiveParsingResult: """Parse directive text into its components. :param directive_class: The directive class to parse for. :param first_line: The first line (arguments). :param content: The directive content. + :param line: The 1-based source line of the directive's opening line. :param validate_options: Whether to validate options against the directive spec. - :return: Tuple of (arguments, options, body_lines, body_offset). + :param additional_options: Additional options to add to the directive. + :return: A ``DirectiveParsingResult`` with fields + ``arguments``, ``options``, ``body``, ``body_offset`` and ``warnings``. :raises MarkupError: If the directive text is malformed. """ ... diff --git a/docs/configuration.md b/docs/configuration.md index 6f4d7790..4acdf059 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -211,5 +211,13 @@ suppress_warnings = ["myst.header"] Or use `--myst-suppress-warnings="myst.header"` for the [docutils CLI](myst-docutils). +:::{note} +Two known limitations apply to the source lines reported by warnings: +warnings for inline syntax (such as roles and links) report the first line of the enclosing paragraph, +rather than the exact line of the syntax, +and warnings within the rendered output of a [substitution](syntax/substitutions) may map beyond the substitution's own location, +if that output spans more lines than the source. +::: + ```{myst-warnings} ``` diff --git a/myst_parser/mdit_to_docutils/base.py b/myst_parser/mdit_to_docutils/base.py index 8a29b5b1..164b3ba6 100644 --- a/myst_parser/mdit_to_docutils/base.py +++ b/myst_parser/mdit_to_docutils/base.py @@ -321,7 +321,9 @@ def nested_render_text( if tokens and tokens[0].type == "front_matter": tokens.pop(0) - # update the line numbers + # update the line numbers, + # note the maps stay 0-based here; + # ``_render_tokens`` applies the final 0-based to 1-based conversion for token in tokens: if token.map: token.map = [token.map[0] + lineno, token.map[1] + lineno] @@ -374,6 +376,10 @@ def add_line_and_source_path(self, node, token: SyntaxTreeNode) -> None: """Copy the line number and document source path to the docutils node.""" with suppress(ValueError): node.line = token_line(token) + # keep the document's current_line in sync, so that docutils' + # ``Node.setup_child`` stamps sensible lines onto line-less + # children (such as ``Text`` nodes), rather than a stale value + self.document.current_line = node.line node.source = self.document["source"] def add_line_and_source_path_r( @@ -1783,12 +1789,22 @@ def render_directive( :param arguments: The remaining text on the same line as the directive name. """ position = token_line(token) + # reconstruct the directive's full text, so that ``block_text`` aligns + # with what an equivalent rST directive would receive + # (best-effort: the parser strips fence indentation and normalises + # the info string, so this may not be byte-identical to the source) + content_text = token.content + if content_text and not content_text.endswith("\n"): + # e.g. for an unterminated fence at the end of the document + content_text += "\n" + block_text = f"{token.markup}{token.info}\n{content_text}{token.markup}\n" nodes_list = self.run_directive( name, arguments, token.content, position, additional_options=additional_options, + block_text=block_text, ) self.current_node += nodes_list @@ -1799,6 +1815,8 @@ def run_directive( content: str, position: int, additional_options: dict[str, str] | None = None, + *, + block_text: str | None = None, ) -> list[nodes.Element]: """Run a directive and return the generated nodes. @@ -1809,6 +1827,8 @@ def run_directive( :param position: The line number of the first line :param additional_options: Additional options to add to the directive, above those parsed from the content. + :param block_text: The full unparsed text of the directive, + defaulting to ``content`` if not given. """ self.document.current_line = position @@ -1838,7 +1858,10 @@ def run_directive( directive_class, first_line, content, - line=position, + # when no block_text is given, the content is synthesized + # (e.g. from HTML attributes) and has no source lines, + # so option warnings fall back to the directive's position + line=position if block_text is not None else None, additional_options=additional_options, ) except MarkupError as error: @@ -1883,7 +1906,7 @@ def run_directive( # the line offset of the first line of the content content_offset=parsed.body_offset, # a string containing the entire directive - block_text="\n".join(parsed.body), + block_text=content if block_text is None else block_text, state=state, state_machine=state_machine, ) diff --git a/myst_parser/mocking.py b/myst_parser/mocking.py index f809c6d7..b65888e7 100644 --- a/myst_parser/mocking.py +++ b/myst_parser/mocking.py @@ -451,7 +451,8 @@ def run(self) -> list[nodes.Element]: literal_block = nodes.literal_block( file_content, source=str(path), classes=self.options.get("class", []) ) - literal_block.line = 1 # TODO don;t think this should be 1? + # docutils sets this to the ``start-line`` option (or 0) + 1 + literal_block.line = (self.options.get("start-line") or 0) + 1 self.add_name(literal_block) if "number-lines" in self.options: # note starting in docutils 0.22 this option is now an integer instead of a string, see: https://github.com/live-clones/docutils/commit/f39ac1413e56a330c8fea6e0d080fed0ff2b8483 diff --git a/myst_parser/parsers/directives.py b/myst_parser/parsers/directives.py index 1908d139..61c4d524 100644 --- a/myst_parser/parsers/directives.py +++ b/myst_parser/parsers/directives.py @@ -36,7 +36,6 @@ from __future__ import annotations -import re from collections.abc import Callable from dataclasses import dataclass from textwrap import dedent @@ -50,7 +49,7 @@ from myst_parser.warnings_ import MystWarnings -from .options import TokenizeError, options_to_items +from .options import TokenizeError, options_to_tokens YAML_LOAD_ERRORS = (yaml.YAMLError, ValueError, RecursionError) """Errors that ``yaml.safe_load`` can raise on invalid input: @@ -76,7 +75,12 @@ class DirectiveParsingResult: body: list[str] """The lines of body content""" body_offset: int - """The number of lines to the start of the body content.""" + """The number of lines from the directive's opening line + to the first line of the body content. + + This is ``-1`` when body content starts on the opening line itself + (only possible for directives that take no arguments). + """ warnings: list[ParseWarnings] """List of non-fatal errors encountered during parsing. (message, line_number) @@ -144,7 +148,9 @@ def parse_directive_text( ) ) body_lines.insert(0, first_line) - content_offset = 0 + # the merged first body line sits on the opening line itself, + # one line *before* where body content normally starts + content_offset -= 1 arguments = [] else: arguments = parse_directive_arguments(directive_class, first_line) @@ -181,21 +187,40 @@ def _parse_directive_options( ) -> _DirectiveOptions: """Parse (and validate) the directive option section. - :returns: (content, options, validation_errors) + :param content: All text after the directive's opening line, + possibly starting with an option block + :param directive_class: The directive class to validate options against + :param as_yaml: Whether to parse the options block with the full YAML spec, + rather than validating against the directive's option specification + (used by myst-nb) + :param line: The 1-based line number of the directive's opening line, + or None if unknown + :param additional_options: Additional options for the directive, + which the options block takes priority over """ options_block: None | str = None + options_position: int | None = None + """The 1-based source line of the first line of the options block.""" if content.startswith("---"): - line = None if line is None else line + 1 - content = "\n".join(content.splitlines()[1:]) - match = re.search(r"^-{3,}", content, re.MULTILINE) - if match: - options_block = content[: match.start()] - content = content[match.end() + 1 :] # TODO advance line number + options_position = None if line is None else line + 2 + content_lines = content.splitlines()[1:] + for index, content_line in enumerate(content_lines): + if content_line.startswith("---"): + options_block = "\n".join(content_lines[:index]) + # any text after the closing delimiter's dashes becomes + # the first line of the body content (and, being kept as + # its own line, maps exactly to its source line) + remainder = content_line.lstrip("-") + remainder = remainder.removeprefix(" ") + rest = content_lines[index + 1 :] + content = "\n".join([remainder, *rest] if remainder else rest) + break else: - options_block = content + options_block = "\n".join(content_lines) content = "" options_block = dedent(options_block) elif content.lstrip().startswith(":"): + options_position = None if line is None else line + 1 content_lines = content.splitlines() yaml_lines = [] while content_lines: @@ -214,12 +239,19 @@ def _parse_directive_options( yaml_errors: list[ParseWarnings] = [] try: yaml_options = yaml.safe_load(options_block or "") or {} - except YAML_LOAD_ERRORS: + except YAML_LOAD_ERRORS as exc: yaml_options = {} + yaml_error_line = options_position + if ( + options_position is not None + and isinstance(exc, yaml.MarkedYAMLError) + and exc.problem_mark is not None + ): + yaml_error_line = options_position + exc.problem_mark.line yaml_errors.append( ParseWarnings( "Invalid options format (bad YAML)", - line, + yaml_error_line, MystWarnings.DIRECTIVE_OPTION, ) ) @@ -228,7 +260,7 @@ def _parse_directive_options( yaml_errors.append( ParseWarnings( "Invalid options format (not a dict)", - line, + options_position, MystWarnings.DIRECTIVE_OPTION, ) ) @@ -237,10 +269,13 @@ def _parse_directive_options( validation_errors: list[ParseWarnings] = [] options: dict[str, str] = {} + option_lines: dict[str, int] = {} + """0-based line of each key, within the options block + (both block styles keep a 1:1 line correspondence with the source). + """ if options_block is not None: try: - _options, state = options_to_items(options_block) - options = dict(_options) + option_tokens, state = options_to_tokens(options_block) except TokenizeError as err: return _DirectiveOptions( content, @@ -248,21 +283,39 @@ def _parse_directive_options( [ ParseWarnings( f"Invalid options format: {err.problem}", - line, + None + if options_position is None + else options_position + err.problem_mark.line, MystWarnings.DIRECTIVE_OPTION, ) ], has_options_block, ) + for key_token, value_token in option_tokens: + options[key_token.value] = ( + value_token.value if value_token is not None else "" + ) + option_lines[key_token.value] = key_token.start.line if state.has_comments: validation_errors.append( ParseWarnings( "Directive options has # comments, which may not be supported in future versions.", - line, + None + if options_position is None + else options_position + + (state.comment_lines[0] if state.comment_lines else 0), MystWarnings.DIRECTIVE_OPTION_COMMENTS, ) ) + def _option_line(name: str) -> int | None: + """The 1-based source line of an option key, or None if unknown + (e.g. keys injected via ``additional_options`` have no source line). + """ + if options_position is None or name not in option_lines: + return None + return options_position + option_lines[name] + if issubclass(directive_class, TestDirective): # technically this directive spec only accepts one option ('option') # but since its for testing only we accept all options @@ -274,14 +327,19 @@ def _parse_directive_options( # check options against spec options_spec: dict[str, Callable] = directive_class.option_spec - unknown_options: list[str] = [] new_options: dict[str, Any] = {} value: str | None for name, value in options.items(): try: converter = options_spec[name] except KeyError: - unknown_options.append(name) + validation_errors.append( + ParseWarnings( + f"Unknown option key: {name!r} (allowed: {sorted(options_spec)})", + _option_line(name), + MystWarnings.DIRECTIVE_OPTION, + ) + ) continue if not value: # restructured text parses empty option values as None @@ -296,23 +354,13 @@ def _parse_directive_options( validation_errors.append( ParseWarnings( f"Invalid option value for {name!r}: {value}: {error}", - line, + _option_line(name), MystWarnings.DIRECTIVE_OPTION, ) ) else: new_options[name] = converted_value - if unknown_options: - validation_errors.append( - ParseWarnings( - f"Unknown option keys: {sorted(unknown_options)} " - f"(allowed: {sorted(options_spec)})", - line, - MystWarnings.DIRECTIVE_OPTION, - ) - ) - return _DirectiveOptions(content, new_options, validation_errors, has_options_block) diff --git a/myst_parser/parsers/options.py b/myst_parser/parsers/options.py index 443a4865..c904074f 100644 --- a/myst_parser/parsers/options.py +++ b/myst_parser/parsers/options.py @@ -19,6 +19,7 @@ from collections.abc import Iterable from dataclasses import dataclass, replace +from dataclasses import field as dc_field from typing import ClassVar, Final, Literal, cast @@ -167,6 +168,35 @@ def __str__(self) -> str: @dataclass class State: has_comments: bool = False + comment_lines: list[int] = dc_field(default_factory=list) + """0-based lines (relative to the text) on which comments were found.""" + + def record_comment(self, line: int) -> None: + self.has_comments = True + # a comment may be scanned twice (at the end of a plain scalar, + # then again when skipping to the next token), and scanning is + # forward-only, so checking the last entry suffices to dedupe + if not self.comment_lines or self.comment_lines[-1] != line: + self.comment_lines.append(line) + + +def options_to_tokens( + text: str, line_offset: int = 0, column_offset: int = 0 +) -> tuple[list[tuple[KeyToken, ValueToken | None]], State]: + """Parse a directive option block into (key, value) token pairs. + + Token positions are 0-based and relative to the start of ``text``; + ``line_offset`` and ``column_offset`` apply only to error positions. + + :param text: The directive option text. + :param line_offset: The line offset to apply to the error positions. + :param column_offset: The column offset to apply to the error positions. + + :raises: `TokenizeError` + """ + state = State() + tokens = list(_to_tokens(text, state, line_offset, column_offset)) + return tokens, state def options_to_items( @@ -180,13 +210,10 @@ def options_to_items( :raises: `TokenizeError` """ - output = [] - state = State() - for key_token, value_token in _to_tokens(text, state, line_offset, column_offset): - output.append( - (key_token.value, value_token.value if value_token is not None else "") - ) - return output, state + tokens, state = options_to_tokens(text, line_offset, column_offset) + return [ + (key.value, value.value if value is not None else "") for key, value in tokens + ], state def _to_tokens( @@ -280,7 +307,7 @@ def _scan_to_next_token(stream: StreamBuffer, state: State) -> None: while stream.peek() == " ": stream.forward() if stream.peek() == "#": - state.has_comments = True + state.record_comment(stream.line) while stream.peek() not in _CHARS_END_NEWLINE: stream.forward() if not _scan_line_break(stream): @@ -298,7 +325,7 @@ def _scan_plain_scalar( while True: length = 0 if stream.peek() == "#": - state.has_comments = True + state.record_comment(stream.line) break while True: ch = stream.peek(length) @@ -318,7 +345,7 @@ def _scan_plain_scalar( spaces = _scan_plain_spaces(stream, allow_newline=(not is_key)) if not spaces or stream.peek() == "#" or (stream.column < indent): if stream.peek() == "#": - state.has_comments = True + state.record_comment(stream.line) break return ( @@ -599,7 +626,7 @@ def _scan_block_scalar_ignored_line( while stream.peek() == " ": stream.forward() if stream.peek() == "#": - state.has_comments = True + state.record_comment(stream.line) while stream.peek() not in _CHARS_END_NEWLINE: stream.forward() ch = stream.peek() diff --git a/tests/test_docutils.py b/tests/test_docutils.py index 1e17235c..c27ef692 100644 --- a/tests/test_docutils.py +++ b/tests/test_docutils.py @@ -441,6 +441,162 @@ def test_footnote_duplicate_definition_warns(): assert len(list(doctree.findall(nodes.footnote))) == 1 +def test_directive_block_text_rst_parity(): + """A fenced directive's ``self.block_text`` mirrors the rST full source. + + The block text spans the opening fence, options block, body and closing + fence, while ``self.content``/``self.content_offset`` stay body-relative. + """ + from docutils.parsers.rst import Directive + from docutils.parsers.rst import directives as rst_directives + + captured = {} + + class _BlockTextEcho(Directive): + has_content = True + option_spec = {"class": rst_directives.class_option} + + def run(self): + captured["block_text"] = self.block_text + captured["content"] = list(self.content) + captured["content_offset"] = self.content_offset + return [] + + rst_directives.register_directive("blocktextecho", _BlockTextEcho) + try: + publish_doctree( + source="```{blocktextecho}\n---\nclass: tip\n---\nbody line\n```\n", + parser=Parser(), + settings_overrides={"warning_stream": io.StringIO()}, + ) + finally: + rst_directives._directives.pop("blocktextecho", None) + assert captured["block_text"] == ( + "```{blocktextecho}\n---\nclass: tip\n---\nbody line\n```\n" + ) + assert captured["content"] == ["body line"] + assert captured["content_offset"] == 3 + + +def test_directive_block_text_fallback(): + """Directives run from synthesized (non-source) content, such as HTML + admonitions, receive that content as ``block_text``.""" + from docutils.parsers.rst import Directive + from docutils.parsers.rst import directives as rst_directives + + captured = {} + + class _Echo(Directive): + required_arguments = 1 + final_argument_whitespace = True + has_content = True + option_spec = {"class": rst_directives.class_option} + + def run(self): + captured["block_text"] = self.block_text + return [] + + rst_directives.register_directive("admonition", _Echo) + try: + publish_doctree( + source='
\nTitle text\n\nBody text here\n
\n', + parser=Parser(), + settings_overrides={ + "warning_stream": io.StringIO(), + "myst_enable_extensions": ["html_admonition"], + }, + ) + finally: + rst_directives._directives.pop("admonition", None) + assert captured["block_text"] == ":class: admonition\n\nTitle text\n" + + +def test_html_image_option_warning_line(): + """Option warnings for HTML-synthesized directives point at the element. + + The synthesized ``:key: value`` content has no source lines, so warnings + must fall back to the element's own line, not per-key arithmetic. + """ + stream = io.StringIO() + publish_doctree( + source="para\n\npara two\n\n\n", + parser=Parser(), + settings_overrides={ + "warning_stream": stream, + "myst_enable_extensions": ["html_image"], + }, + ) + assert ":5: (WARNING/2) 'image': Invalid option value for 'width'" in ( + stream.getvalue() + ) + + +def test_colon_fence_option_warning_line(): + """Per-key option warning lines also apply to ``:::`` fence directives.""" + stream = io.StringIO() + publish_doctree( + source="para\n\n:::{note}\n:foo: bar\nbody\n:::\n", + parser=Parser(), + settings_overrides={ + "warning_stream": stream, + "myst_enable_extensions": ["colon_fence"], + }, + ) + assert ":4: (WARNING/2) 'note': Unknown option key: 'foo'" in ( + stream.getvalue() + ) + + +def test_include_literal_line(tmp_path): + """A literal ``include`` block's source line reflects ``start-line``.""" + inc = tmp_path / "inc.txt" + inc.write_text("l1\nl2\nl3\nl4\nl5\n") + source_path = str(tmp_path / "main.md") + + doctree = publish_doctree( + source=f"```{{include}} {inc}\n:literal: true\n:start-line: 2\n```\n", + source_path=source_path, + parser=Parser(), + settings_overrides={"warning_stream": io.StringIO()}, + ) + literal_block = next(doctree.findall(nodes.literal_block)) + assert literal_block.line == 3 + assert literal_block.astext() == "l3\nl4\nl5" + + doctree = publish_doctree( + source=f"```{{include}} {inc}\n:literal: true\n```\n", + source_path=source_path, + parser=Parser(), + settings_overrides={"warning_stream": io.StringIO()}, + ) + literal_block = next(doctree.findall(nodes.literal_block)) + assert literal_block.line == 1 + + +def test_text_node_line_stamping(): + """Text nodes carry their enclosing block's line, not a stale one. + + Regression for a directive's line being stamped + (via ``document.current_line`` and docutils' ``Node.setup_child``) + onto every subsequent ``Text`` node in the document. + """ + source = ( + "para one\n\n```{note}\n:class: tip\n\nnote body\n```\n\npara two\n\n- item\n" + ) + doctree = publish_doctree( + source=source, + parser=Parser(), + settings_overrides={"warning_stream": io.StringIO()}, + ) + lines = {text.astext(): text.line for text in doctree.findall(nodes.Text)} + assert lines["para one"] == 1 + # previously stamped with the note directive's line (3) + assert lines["para two"] == 9 + assert lines["item"] == 11 + # created while detached from the document, so defers to ancestor lookup + assert lines["note body"] is None + + def test_definition_list_orphan_definition(): """A definition with no preceding term errors, but keeps its content. diff --git a/tests/test_renderers/fixtures/directive_options.md b/tests/test_renderers/fixtures/directive_options.md index 7c5b7e86..61b99977 100644 --- a/tests/test_renderers/fixtures/directive_options.md +++ b/tests/test_renderers/fixtures/directive_options.md @@ -131,7 +131,7 @@ foo ``` . - + 'restructuredtext-test-directive': Invalid options format: expected ':' after key [myst.directive_option] diff --git a/tests/test_renderers/fixtures/directive_parsing.txt b/tests/test_renderers/fixtures/directive_parsing.txt index d1480940..e8b60186 100644 --- a/tests/test_renderers/fixtures/directive_parsing.txt +++ b/tests/test_renderers/fixtures/directive_parsing.txt @@ -6,7 +6,7 @@ note: content in first line only arguments: [] body: - a -content_offset: 0 +content_offset: -1 options: {} warnings: [] . @@ -35,7 +35,7 @@ arguments: [] body: - a - b -content_offset: 0 +content_offset: -1 options: {} warnings: [] . @@ -73,7 +73,7 @@ options: - name warnings: - 'ParseWarnings(msg=''Directive options has # comments, which may not be supported - in future versions.'', lineno=0, type=)' + in future versions.'', lineno=1, type=)' . note: content after option with new line @@ -199,8 +199,8 @@ body: [] content_offset: 3 options: {} warnings: -- 'ParseWarnings(msg="Unknown option keys: [''a''] (allowed: [''class'', ''name''])", - lineno=1, type=)' +- 'ParseWarnings(msg="Unknown option key: ''a'' (allowed: [''class'', ''name''])", + lineno=2, type=)' . warning: yaml not a dict @@ -216,7 +216,7 @@ body: [] content_offset: 3 options: {} warnings: -- 'ParseWarnings(msg="Invalid options format: expected '':'' after key", lineno=1, +- 'ParseWarnings(msg="Invalid options format: expected '':'' after key", lineno=2, type=)' . @@ -231,8 +231,8 @@ body: [] content_offset: 1 options: {} warnings: -- 'ParseWarnings(msg="Unknown option keys: [''unknown''] (allowed: [''class'', ''name''])", - lineno=0, type=)' +- 'ParseWarnings(msg="Unknown option key: ''unknown'' (allowed: [''class'', ''name''])", + lineno=1, type=)' . warning: invalid option value @@ -247,7 +247,7 @@ content_offset: 1 options: {} warnings: - 'ParseWarnings(msg=''Invalid option value for \''class\'': 1: cannot make "1" into - a class name'', lineno=0, type=)' + a class name'', lineno=1, type=)' . error: missing argument diff --git a/tests/test_renderers/fixtures/myst-config.txt b/tests/test_renderers/fixtures/myst-config.txt index 62259d96..cd0b5963 100644 --- a/tests/test_renderers/fixtures/myst-config.txt +++ b/tests/test_renderers/fixtures/myst-config.txt @@ -512,7 +512,7 @@ content Unknown directive type: 'unknown' [myst.directive_unknown] - 'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option] + 'admonition': Unknown option key: 'a' (allowed: ['class', 'name']) [myst.directive_option] title @@ -520,7 +520,7 @@ content content <string>:1: (WARNING/2) Unknown directive type: 'unknown' [myst.directive_unknown] -<string>:6: (WARNING/2) 'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option] +<string>:6: (WARNING/2) 'admonition': Unknown option key: 'a' (allowed: ['class', 'name']) [myst.directive_option] . [links-external-new-tab] --myst-links-external-new-tab="true" diff --git a/tests/test_renderers/fixtures/reporter_warnings.md b/tests/test_renderers/fixtures/reporter_warnings.md index 27b0ea83..457d9e34 100644 --- a/tests/test_renderers/fixtures/reporter_warnings.md +++ b/tests/test_renderers/fixtures/reporter_warnings.md @@ -169,7 +169,7 @@ directive bad-option-value :class: [1] ``` . -<string>:1: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_option] +<string>:2: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_option] <string>:1: (ERROR/3) Content block expected for the "note" directive; none found. . @@ -195,3 +195,76 @@ Paragraph . <string>:7: (WARNING/2) Unknown interpreted text role "unknown". [myst.role_unknown] . + +directive option warnings in yaml block +. +para + +```{note} +--- +class: [1] +name: ok +foo: bar +--- +body + +{unknownrole}`x` +``` +. +<string>:5: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_option] +<string>:7: (WARNING/2) 'note': Unknown option key: 'foo' (allowed: ['class', 'name']) [myst.directive_option] +<string>:11: (WARNING/2) Unknown interpreted text role "unknownrole". [myst.role_unknown] +. + +directive option warnings in colon block +. +para + +```{note} +:class: [1] +:foo: bar + +body +``` +. +<string>:4: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_option] +<string>:5: (WARNING/2) 'note': Unknown option key: 'foo' (allowed: ['class', 'name']) [myst.directive_option] +. + +directive option tokenize error line +. +```{note} +--- +class: "unclosed +--- +body +``` +. +<string>:3: (WARNING/2) 'note': Invalid options format: found unexpected end of stream [myst.directive_option] +. + +directive body warning below options block +. +```{note} +--- +class: tip +name: mynote +--- + +{unknownrole}`x` +``` +. +<string>:7: (WARNING/2) Unknown interpreted text role "unknownrole". [myst.role_unknown] +. + +directive first-line content line mapping +. +```{note} first line + +body + +{unknownrole}`x` +``` +. +<string>:5: (WARNING/2) Unknown interpreted text role "unknownrole". [myst.role_unknown] +. diff --git a/tests/test_renderers/test_parse_directives.py b/tests/test_renderers/test_parse_directives.py index 36eba61b..79b1bc9f 100644 --- a/tests/test_renderers/test_parse_directives.py +++ b/tests/test_renderers/test_parse_directives.py @@ -9,7 +9,11 @@ from sphinx.directives.code import CodeBlock from myst_parser.parsers.directives import MarkupError, parse_directive_text -from myst_parser.parsers.options import TokenizeError, options_to_items +from myst_parser.parsers.options import ( + TokenizeError, + options_to_items, + options_to_tokens, +) FIXTURE_PATH = Path(__file__).parent.joinpath("fixtures") @@ -127,3 +131,118 @@ def test_colon_options_stop_at_colon_fence(): result = parse_directive_text(Note, "", ":class: xxx\n::::{other}\ncontent\n::::") assert result.options == {"class": ["xxx"]} assert result.body == ["::::{other}", "content", "::::"] + + +def test_option_warning_lines_yaml_block(): + """Option warnings carry per-key source lines for a ``---`` YAML block.""" + result = parse_directive_text( + Note, "", "---\nclass: [1]\nname: ok\nfoo: bar\n---\nbody", line=3 + ) + assert len(result.warnings) == 2 + # line 3 fence, 4 `---`, 5 `class: [1]` + assert result.warnings[0].msg.startswith("Invalid option value for 'class'") + assert result.warnings[0].lineno == 5 + assert result.warnings[1].msg == ( + "Unknown option key: 'foo' (allowed: ['class', 'name'])" + ) + assert result.warnings[1].lineno == 7 + assert result.body == ["body"] + assert result.body_offset == 5 + + +def test_option_warning_lines_colon_block(): + """Option warnings carry per-key source lines for a ``:opt:`` block.""" + result = parse_directive_text(Note, "", ":class: [1]\n:foo: bar\n\nbody", line=3) + assert len(result.warnings) == 2 + assert result.warnings[0].msg.startswith("Invalid option value for 'class'") + assert result.warnings[0].lineno == 4 + assert result.warnings[1].msg == ( + "Unknown option key: 'foo' (allowed: ['class', 'name'])" + ) + assert result.warnings[1].lineno == 5 + assert result.body == ["body"] + assert result.body_offset == 3 + + +def test_option_warning_lines_none(): + """Without a known directive line, option warnings have no line number.""" + result = parse_directive_text( + Note, "", "---\nclass: [1]\nname: ok\nfoo: bar\n---\nbody" + ) + assert len(result.warnings) == 2 + assert all(w.lineno is None for w in result.warnings) + + +def test_option_tokenize_error_line(): + """A tokenize error carries the source line of the problem mark.""" + result = parse_directive_text(Note, "", '---\nclass: "unclosed\n---\nbody', line=1) + assert len(result.warnings) == 1 + # fence 1, `---` 2, bad line 3 + assert result.warnings[0].msg.startswith("Invalid options format") + assert result.warnings[0].lineno == 3 + assert result.body == ["body"] + + +def test_yaml_block_closing_delimiter_trailing_text(): + """Text trailing a ``---`` closing delimiter becomes the first body line. + + As previously, any line starting with three dashes closes the block, but + the split is now line-based, so the trailing text keeps an exact source + line mapping (it was previously merged into the following line's mapping). + """ + result = parse_directive_text( + Note, "", "---\nclass: tip\n--- foo\n---\nbody", line=1 + ) + assert not result.warnings + assert result.options == {"class": ["tip"]} + # "foo" is kept as its own body line, mapping to source line + # 1 (fence) + 2 (body_offset) + 1 = 4, the `--- foo` line itself + assert result.body == ["foo", "---", "body"] + assert result.body_offset == 2 + # a dashes-only closer with trailing whitespace closes without a leak + result2 = parse_directive_text(Note, "", "---\nclass: tip\n--- \nbody", line=1) + assert not result2.warnings + assert result2.options == {"class": ["tip"]} + assert result2.body == ["body"] + + +def test_option_comment_warning_line(): + """The comments warning points at the (first) comment's own line.""" + result = parse_directive_text( + Note, "", "---\nclass: tip\n# a comment\n---\nbody", line=1 + ) + assert len(result.warnings) == 1 + assert "# comments" in result.warnings[0].msg + assert result.warnings[0].lineno == 4 + + +def test_as_yaml_error_line(): + """The full-YAML (``validate_options=False``) parse errors carry lines.""" + result = parse_directive_text( + Note, "", "---\na: {\n---\nbody", line=1, validate_options=False + ) + assert len(result.warnings) == 1 + assert result.warnings[0].msg == "Invalid options format (bad YAML)" + assert result.warnings[0].lineno == 3 + assert result.body == ["body"] + + +def test_options_to_tokens(): + """``options_to_tokens`` yields key/value token pairs with source lines.""" + text = "a: 1\nb: |\n multi\n line\nc:\n" + tokens, _state = options_to_tokens(text) + assert [key.value for key, _ in tokens] == ["a", "b", "c"] + assert [key.start.line for key, _ in tokens] == [0, 1, 4] + values = [value.value if value is not None else None for _, value in tokens] + assert values == ["1", "multi\nline\n", None] + items, _ = options_to_items(text) + assert items == [ + (key.value, value.value if value is not None else "") for key, value in tokens + ] + + +def test_options_to_tokens_comment_lines(): + """``State.comment_lines`` records each comment's line, exactly once.""" + _, state = options_to_tokens("# first\na: 1 # second\n# third\nb: 2\n") + assert state.has_comments + assert state.comment_lines == [0, 1, 2] diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext.pot b/tests/test_sphinx/test_sphinx_builds/test_gettext.pot index a9d5ec9f..bcbf14fa 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext.pot +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext.pot @@ -48,11 +48,11 @@ msgstr "" msgid "**bold** text 8" msgstr "" -#: ../../index.md:0 +#: ../../index.md:20 msgid "**bold** text 9" msgstr "" -#: ../../index.md:0 +#: ../../index.md:20 msgid "**bold** text 10" msgstr "" diff --git a/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot b/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot index 896ba731..d78d5a05 100644 --- a/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot +++ b/tests/test_sphinx/test_sphinx_builds/test_gettext_additional_targets.pot @@ -48,11 +48,11 @@ msgstr "" msgid "**bold** text 8" msgstr "" -#: ../../index.md:0 +#: ../../index.md:20 msgid "**bold** text 9" msgstr "" -#: ../../index.md:0 +#: ../../index.md:20 msgid "**bold** text 10" msgstr ""