Repository: executablebooks/MyST-Parser
Pull request #893 (2024-03-26 02:21:18)

👌 Improve directive parsing warnings
Minor improvement to the specificity of warnings regarding problems encountered when parsing directives

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) Report
Attention: Patch coverage is `80.00000%` with `4 lines` in your changes are missing coverage. Please review.
> Project coverage is 90.06%. Comparing base [(`978e845`)](https://app.codecov.io/gh/executablebooks/MyST-Parser/commit/978e845543b5bcb7af0ff89cac9f798cb8c16ab3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) to head [(`ee4ae60`)](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks).

| [Files](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) | Patch % | Lines |
|---|---|---|
| [myst\_parser/parsers/directives.py](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks#diff-bXlzdF9wYXJzZXIvcGFyc2Vycy9kaXJlY3RpdmVzLnB5) | 81.25% | [3 Missing :warning: ](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) |
| [myst\_parser/mocking.py](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks#diff-bXlzdF9wYXJzZXIvbW9ja2luZy5weQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   90.04%   90.06%   +0.02%     
==========================================
  Files          24       24              
  Lines        3364     3372       +8     
==========================================
+ Hits         3029     3037       +8     
  Misses        335      335              
```

| [Flag](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) | Coverage Δ | |
|---|---|---|
| [pytests](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks) | `90.06% <80.00%> (+0.02%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks#carryforward-flags-in-the-pull-request-comment) to find out more.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/executablebooks/MyST-Parser/pull/893?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=executablebooks).

chrisjsewell: Minor improvement to the specificity of warnings regarding problems encountered when parsing directives

Patch:
diff --git a/myst_parser/mdit_to_docutils/base.py b/myst_parser/mdit_to_docutils/base.py
index 4a02459..0b3904a 100644
--- a/myst_parser/mdit_to_docutils/base.py
+++ b/myst_parser/mdit_to_docutils/base.py
@@ -1758,11 +1758,11 @@ class DocutilsRenderer(RendererProtocol):
             )
             return [error]
 
-        for warning_msg, warning_line in parsed.warnings:
+        for _warning in parsed.warnings:
             self.create_warning(
-                f"{name!r}: {warning_msg}",
-                MystWarnings.DIRECTIVE_PARSING,
-                line=warning_line if warning_line is not None else position,
+                f"{name!r}: {_warning.msg}",
+                _warning.type,
+                line=_warning.lineno if _warning.lineno is not None else position,
                 append_to=self.current_node,
             )
 
diff --git a/myst_parser/mocking.py b/myst_parser/mocking.py
index a6f6431..f3c21f6 100644
--- a/myst_parser/mocking.py
+++ b/myst_parser/mocking.py
@@ -140,7 +140,7 @@ class MockState:
         # TODO should argument_str always be ""?
         parsed = parse_directive_text(directive, "", "\n".join(content))
         if parsed.warnings:
-            raise MarkupError(",".join(w for w, _ in parsed.warnings))
+            raise MarkupError(",".join(w.msg for w in parsed.warnings))
         return (
             parsed.arguments,
             parsed.options,
diff --git a/myst_parser/parsers/directives.py b/myst_parser/parsers/directives.py
index e49ed42..e4cfee8 100644
--- a/myst_parser/parsers/directives.py
+++ b/myst_parser/parsers/directives.py
@@ -46,10 +46,19 @@ from docutils.parsers.rst.directives import flag
 from docutils.parsers.rst.directives.misc import TestDirective
 from docutils.parsers.rst.states import MarkupError
 
+from myst_parser.warnings_ import MystWarnings
+
 from .options import TokenizeError
 from .options import to_items as options_to_items
 
 
+@dataclass
+class ParseWarnings:
+    msg: str
+    lineno: int | None = None
+    type: MystWarnings = MystWarnings.DIRECTIVE_PARSING
+
+
 @dataclass
 class DirectiveParsingResult:
     arguments: list[str]
@@ -60,7 +69,7 @@ class DirectiveParsingResult:
     """The lines of body content"""
     body_offset: int
     """The number of lines to the start of the body content."""
-    warnings: list[tuple[str, int | None]]
+    warnings: list[ParseWarnings]
     """List of non-fatal errors encountered during parsing.
     (message, line_number)
     """
@@ -88,7 +97,7 @@ def parse_directive_text(
 
     :raises MarkupError: if there is a fatal parsing/validation error
     """
-    parse_errors: list[tuple[str, int | None]]
+    parse_warnings: list[ParseWarnings]
     options: dict[str, Any]
     body_lines: list[str]
     content_offset: int
@@ -104,13 +113,13 @@ def parse_directive_text(
             as_yaml=not validate_options,
             additional_options=additional_options,
         )
-        parse_errors = result.errors
+        parse_warnings = result.warnings
         has_options_block = result.has_options
         options = result.options
         body_lines = result.content.splitlines()
         content_offset = len(content.splitlines()) - len(body_lines)
     else:
-        parse_errors = []
+        parse_warnings = []
         has_options_block = False
         options = {}
         body_lines = content.splitlines()
@@ -120,11 +129,10 @@ def parse_directive_text(
         # If there are no possible arguments, then the body can start on the argument line
         if first_line.strip():
             if has_options_block and any(body_lines):
-                parse_errors.append(
-                    (
-                        "Cannot split content across first line and body, "
-                        "when options block is present (move first line to body)",
-                        None,
+                parse_warnings.append(
+                    ParseWarnings(
+                        "Splitting content across first line and body, "
+                        "when an options block is present, is not recommended"
                     )
                 )
             body_lines.insert(0, first_line)
@@ -141,10 +149,10 @@ def parse_directive_text(
 
     # check for body content
     if body_lines and not directive_class.has_content:
-        parse_errors.append(("Has content, but none permitted", None))
+        parse_warnings.append(ParseWarnings("Has content, but none permitted"))
 
     return DirectiveParsingResult(
-        arguments, options, body_lines, content_offset, parse_errors
+        arguments, options, body_lines, content_offset, parse_warnings
     )
 
 
@@ -152,7 +160,7 @@ def parse_directive_text(
 class _DirectiveOptions:
     content: str
     options: dict[str, Any]
-    errors: list[tuple[str, int | None]]
+    warnings: list[ParseWarnings]
     has_options: bool
 
 
@@ -195,15 +203,27 @@ def _parse_directive_options(
     has_options_block = yaml_block is not None
 
     if as_yaml:
-        yaml_errors: list[tuple[str, int | None]] = []
+        yaml_errors: list[ParseWarnings] = []
         try:
             yaml_options = yaml.safe_load(yaml_block or "") or {}
         except (yaml.parser.ParserError, yaml.scanner.ScannerError):
             yaml_options = {}
-            yaml_errors.append(("Invalid options format (bad YAML)", line))
+            yaml_errors.append(
+                ParseWarnings(
+                    "Invalid options format (bad YAML)",
+                    line,
+                    MystWarnings.DIRECTIVE_OPTION,
+                )
+            )
         if not isinstance(yaml_options, dict):
             yaml_options = {}
-            yaml_errors.append(("Invalid options format (not a dict)", line))
+            yaml_errors.append(
+                ParseWarnings(
+                    "Invalid options format (not a dict)",
+                    line,
+                    MystWarnings.DIRECTIVE_OPTION,
+                )
+            )
         return _DirectiveOptions(content, yaml_options, yaml_errors, has_options_block)
 
     options: dict[str, str] = {}
@@ -214,7 +234,13 @@ def _parse_directive_options(
             return _DirectiveOptions(
                 content,
                 options,
-                [(f"Invalid options format: {err.problem}", line)],
+                [
+                    ParseWarnings(
+                        f"Invalid options format: {err.problem}",
+                        line,
+                        MystWarnings.DIRECTIVE_OPTION,
+                    )
+                ],
                 has_options_block,
             )
 
@@ -231,7 +257,7 @@ def _parse_directive_options(
     options_spec: dict[str, Callable] = directive_class.option_spec
     unknown_options: list[str] = []
     new_options: dict[str, Any] = {}
-    validation_errors: list[tuple[str, int | None]] = []
+    validation_errors: list[ParseWarnings] = []
     value: str | None
     for name, value in options.items():
         try:
@@ -250,17 +276,22 @@ def _parse_directive_options(
             converted_value = convertor(value)
         except (ValueError, TypeError) as error:
             validation_errors.append(
-                (f"Invalid option value for {name!r}: {value}: {error}", line)
+                ParseWarnings(
+                    f"Invalid option value for {name!r}: {value}: {error}",
+                    line,
+                    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,
             )
         )
 
diff --git a/myst_parser/warnings_.py b/myst_parser/warnings_.py
index 7493d89..c101f26 100644
--- a/myst_parser/warnings_.py
+++ b/myst_parser/warnings_.py
@@ -31,6 +31,10 @@ class MystWarnings(Enum):
 
     DIRECTIVE_PARSING = "directive_parse"
     """Issue parsing directive."""
+    DIRECTIVE_OPTION = "directive_option"
+    """Issue parsing directive options."""
+    DIRECTIVE_BODY = "directive_body"
+    """Issue parsing directive body."""
     UNKNOWN_DIRECTIVE = "directive_unknown"
     """Unknown directive."""
     UNKNOWN_ROLE = "role_unknown"


Test patch:
diff --git a/tests/test_renderers/fixtures/directive_options.md b/tests/test_renderers/fixtures/directive_options.md
index 779a794..7c5b7e8 100644
--- a/tests/test_renderers/fixtures/directive_options.md
+++ b/tests/test_renderers/fixtures/directive_options.md
@@ -133,7 +133,7 @@ foo
 <document source="<src>/index.md">
     <system_message level="2" line="1" source="<src>/index.md" type="WARNING">
         <paragraph>
-            'restructuredtext-test-directive': Invalid options format: expected ':' after key [myst.directive_parse]
+            'restructuredtext-test-directive': Invalid options format: expected ':' after key [myst.directive_option]
     <system_message level="1" line="1" source="<src>/index.md" type="INFO">
         <paragraph>
             Directive processed. Type="restructuredtext-test-directive", arguments=[], options={}, content:
diff --git a/tests/test_renderers/fixtures/directive_parsing.txt b/tests/test_renderers/fixtures/directive_parsing.txt
index 25b9b76..8f4f601 100644
--- a/tests/test_renderers/fixtures/directive_parsing.txt
+++ b/tests/test_renderers/fixtures/directive_parsing.txt
@@ -112,9 +112,9 @@ options:
   class:
   - tip
 warnings:
-- - Cannot split content across first line and body, when options block is present
-    (move first line to body)
-  - null
+- 'ParseWarnings(msg=''Splitting content across first line and body, when an options
+  block is present, is not recommended'', lineno=None, type=<MystWarnings.DIRECTIVE_PARSING:
+  ''directive_parse''>)'
 .
 
 admonition: no options, no new line
@@ -180,8 +180,8 @@ body: []
 content_offset: 3
 options: {}
 warnings:
-- - 'Unknown option keys: [''a''] (allowed: [''class'', ''name''])'
-  - 1
+- 'ParseWarnings(msg="Unknown option keys: [''a''] (allowed: [''class'', ''name''])",
+  lineno=1, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: yaml not a dict
@@ -197,8 +197,8 @@ body: []
 content_offset: 3
 options: {}
 warnings:
-- - 'Invalid options format: expected '':'' after key'
-  - 1
+- 'ParseWarnings(msg="Invalid options format: expected '':'' after key", lineno=1,
+  type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: unknown option name
@@ -212,8 +212,8 @@ body: []
 content_offset: 1
 options: {}
 warnings:
-- - 'Unknown option keys: [''unknown''] (allowed: [''class'', ''name''])'
-  - 0
+- 'ParseWarnings(msg="Unknown option keys: [''unknown''] (allowed: [''class'', ''name''])",
+  lineno=0, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 warning: invalid option value
@@ -227,8 +227,8 @@ body: []
 content_offset: 1
 options: {}
 warnings:
-- - 'Invalid option value for ''class'': 1: cannot make "1" into a class name'
-  - 0
+- 'ParseWarnings(msg=''Invalid option value for \''class\'': 1: cannot make "1" into
+  a class name'', lineno=0, type=<MystWarnings.DIRECTIVE_OPTION: ''directive_option''>)'
 .
 
 error: missing argument
diff --git a/tests/test_renderers/fixtures/myst-config.txt b/tests/test_renderers/fixtures/myst-config.txt
index e0e6930..52f5487 100644
--- a/tests/test_renderers/fixtures/myst-config.txt
+++ b/tests/test_renderers/fixtures/myst-config.txt
@@ -497,7 +497,7 @@ content
             Unknown directive type: 'unknown' [myst.directive_unknown]
     <system_message level="2" line="6" source="<string>" type="WARNING">
         <paragraph>
-            'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_parse]
+            'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option]
     <admonition classes="class1" ids="myname" names="myname">
         <title>
             title
@@ -505,5 +505,5 @@ 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_parse]
+<string>:6: (WARNING/2) 'admonition': Unknown option keys: ['a'] (allowed: ['class', 'name']) [myst.directive_option]
 .
diff --git a/tests/test_renderers/fixtures/reporter_warnings.md b/tests/test_renderers/fixtures/reporter_warnings.md
index dd8d70b..4ff162c 100644
--- a/tests/test_renderers/fixtures/reporter_warnings.md
+++ b/tests/test_renderers/fixtures/reporter_warnings.md
@@ -143,13 +143,13 @@ lines
 <string>:12: (ERROR/3) Unknown interpreted text role "unknown".
 .
 
-bad-option-value
+directive bad-option-value
 .
 ```{note}
 :class: [1]
 ```
 .
-<string>:1: (WARNING/2) 'note': Invalid option value for 'class': [1]: cannot make "[1]" into a class name [myst.directive_parse]
+<string>:1: (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.
 .
 
diff --git a/tests/test_renderers/test_parse_directives.py b/tests/test_renderers/test_parse_directives.py
index a8c8fa7..0cb6292 100644
--- a/tests/test_renderers/test_parse_directives.py
+++ b/tests/test_renderers/test_parse_directives.py
@@ -60,7 +60,7 @@ def test_parsing(file_params):
                 "options": result.options,
                 "body": result.body,
                 "content_offset": result.body_offset,
-                "warnings": result.warnings,
+                "warnings": [repr(w) for w in result.warnings],
             },
             sort_keys=True,
         )
@@ -112,4 +112,4 @@ def test_additional_options():
         Note, "", "content", additional_options={"foo": "bar"}
     )
     assert len(result.warnings) == 1
-    assert "Unknown option" in result.warnings[0][0]
+    assert "Unknown option" in result.warnings[0].msg