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='
\n",
+ parser=Parser(),
+ settings_overrides={
+ "warning_stream": stream,
+ "myst_enable_extensions": ["html_image"],
+ },
+ )
+ assert "