-
Notifications
You must be signed in to change notification settings - Fork 245
Fix mkdocs-macros Jinja2 syntax errors in generated block docs #2012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
yeldarby
wants to merge
1
commit into
main
Choose a base branch
from
claude/wonderful-jones
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+204
−0
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| """Validate that generated documentation files do not contain Jinja2 syntax | ||
| errors. | ||
|
|
||
| mkdocs-macros processes all Markdown files through Jinja2 before rendering. | ||
| Patterns like ``{{ $parameters.xxx }}`` cause "unexpected char '$'" errors | ||
| because ``$`` is not valid inside Jinja2 expressions. The doc build pipeline | ||
| (``build_block_docs.py``) escapes these patterns, but this script acts as a | ||
| safety net to catch any regressions. | ||
|
|
||
| Usage: | ||
| python -m development.docs.validate_docs_jinja2 [docs_dir] | ||
|
|
||
| If no directory is given, it defaults to ``docs/``. The script exits with | ||
| code 1 if any Jinja2 parse errors are found. | ||
| """ | ||
|
|
||
| import glob | ||
| import os | ||
| import sys | ||
|
|
||
| from jinja2 import Environment | ||
|
|
||
|
|
||
| def validate_markdown_files(docs_dir: str) -> list[tuple[str, str]]: | ||
| """Parse every ``.md`` file under *docs_dir* as a Jinja2 template. | ||
|
|
||
| Returns a list of ``(relative_path, error_message)`` tuples for files that | ||
| fail to parse. | ||
| """ | ||
| env = Environment() | ||
| errors: list[tuple[str, str]] = [] | ||
|
|
||
| md_files = sorted(glob.glob(os.path.join(docs_dir, "**", "*.md"), recursive=True)) | ||
| for md_file in md_files: | ||
| with open(md_file, encoding="utf-8") as fh: | ||
| content = fh.read() | ||
| try: | ||
| env.parse(content) | ||
| except Exception as exc: | ||
| rel_path = os.path.relpath(md_file, docs_dir) | ||
| errors.append((rel_path, str(exc))) | ||
|
|
||
| return errors | ||
|
|
||
|
|
||
| def main() -> None: | ||
| if len(sys.argv) > 1: | ||
| docs_dir = sys.argv[1] | ||
| else: | ||
| docs_dir = os.path.join(os.path.dirname(__file__), "..", "..", "docs") | ||
|
|
||
| docs_dir = os.path.abspath(docs_dir) | ||
| if not os.path.isdir(docs_dir): | ||
| print(f"Error: {docs_dir} is not a directory", file=sys.stderr) | ||
| sys.exit(2) | ||
|
|
||
| print(f"Validating Jinja2 syntax in {docs_dir} ...") | ||
| errors = validate_markdown_files(docs_dir) | ||
|
|
||
| if errors: | ||
| print(f"\n{len(errors)} file(s) with Jinja2 syntax errors:\n", file=sys.stderr) | ||
| for rel_path, err_msg in errors: | ||
| print(f" {rel_path}: {err_msg}", file=sys.stderr) | ||
| print( | ||
| "\nThese files will cause 'Macro Syntax Error' when rendered by " | ||
| "mkdocs-macros. Ensure that {{ ... }} expressions containing '$' " | ||
| "are escaped (e.g. {{ '{{' }} $parameters.xxx {{ '}}' }}).", | ||
| file=sys.stderr, | ||
| ) | ||
| sys.exit(1) | ||
| else: | ||
| md_count = len( | ||
| glob.glob(os.path.join(docs_dir, "**", "*.md"), recursive=True) | ||
| ) | ||
| print(f"All {md_count} Markdown files pass Jinja2 syntax validation.") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
Empty file.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| """Tests for the documentation build pipeline's Jinja2 escaping logic. | ||
|
|
||
| These tests ensure that generated Markdown docs do not contain raw | ||
| ``{{ $parameters.xxx }}`` expressions that would cause mkdocs-macros | ||
| (Jinja2) parse errors like "unexpected char '$'". | ||
| """ | ||
|
|
||
| import pytest | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unused import
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @claude fix pls |
||
| from jinja2 import Environment | ||
|
|
||
| from development.docs.build_block_docs import ( | ||
| _escape_jinja2_expressions, | ||
| ) | ||
|
|
||
| jinja_env = Environment() | ||
|
|
||
|
|
||
| def _parses_as_jinja2(text: str) -> bool: | ||
| """Return True if *text* can be parsed by Jinja2 without errors.""" | ||
| try: | ||
| jinja_env.parse(text) | ||
| return True | ||
| except Exception: | ||
| return False | ||
|
|
||
|
|
||
| class TestEscapeJinja2Expressions: | ||
| """Tests for ``_escape_jinja2_expressions``.""" | ||
|
|
||
| def test_bare_dollar_parameters_in_braces(self): | ||
| """The main bug: ``{{ $parameters.xxx }}`` must be escaped.""" | ||
| raw = 'replaces placeholders like `{{ $parameters.parameter_name }}`' | ||
| result = _escape_jinja2_expressions(raw) | ||
| assert _parses_as_jinja2(result) | ||
| # The rendered output should still contain the human-readable text | ||
| assert "$parameters.parameter_name" in result | ||
|
|
||
| def test_multiple_dollar_expressions(self): | ||
| raw = ( | ||
| "Detected {{ $parameters.num_objects }} objects. " | ||
| "Classes: {{ $parameters.classes }}." | ||
| ) | ||
| result = _escape_jinja2_expressions(raw) | ||
| assert _parses_as_jinja2(result) | ||
| assert "$parameters.num_objects" in result | ||
| assert "$parameters.classes" in result | ||
|
|
||
| def test_already_escaped_expression_not_double_escaped(self): | ||
| """Expressions that are already escaped must not be broken.""" | ||
| already_escaped = "{{ '{{' }} $parameters.predicted_classes {{ '}}' }}" | ||
| result = _escape_jinja2_expressions(already_escaped) | ||
| assert _parses_as_jinja2(result) | ||
|
|
||
| def test_normal_jinja2_variables_untouched(self): | ||
| """Legitimate Jinja2 variables (no $) must not be modified.""" | ||
| normal = "Version: {{ VERSION }}" | ||
| result = _escape_jinja2_expressions(normal) | ||
| assert result == normal | ||
|
|
||
| def test_no_braces_dollar_untouched(self): | ||
| """Bare $inputs.xxx outside {{ }} must not be modified.""" | ||
| bare = '"smtp_server": "$inputs.smtp_server"' | ||
| result = _escape_jinja2_expressions(bare) | ||
| assert result == bare | ||
| assert _parses_as_jinja2(result) | ||
|
|
||
| def test_dollar_parameters_with_extra_spaces(self): | ||
| raw = "{{ $parameters.foo }}" | ||
| result = _escape_jinja2_expressions(raw) | ||
| assert _parses_as_jinja2(result) | ||
|
|
||
| def test_css_double_braces_untouched(self): | ||
| """CSS rules using ``{{ }}`` for escaping Python .format() must survive.""" | ||
| css = "article > a.md-content__button.md-icon:first-child {{ display: none; }}" | ||
| result = _escape_jinja2_expressions(css) | ||
| # CSS doesn't contain $ so should be untouched | ||
| assert result == css | ||
|
|
||
| def test_real_world_email_notification_description(self): | ||
| """Reproduce the exact text that caused the original bug report.""" | ||
| text = ( | ||
| "3. Formats the email message by processing dynamic parameters " | ||
| "(replaces placeholders like `{{ $parameters.parameter_name }}` " | ||
| "with actual workflow data from `message_parameters`)" | ||
| ) | ||
| assert not _parses_as_jinja2(text) # Confirm it's broken before fix | ||
| result = _escape_jinja2_expressions(text) | ||
| assert _parses_as_jinja2(result) | ||
|
|
||
| def test_real_world_field_description(self): | ||
| """Field description with multiple {{ $parameters }} references.""" | ||
| text = ( | ||
| "SMS message content (plain text). Supports dynamic parameters " | ||
| "using placeholder syntax: {{ $parameters.parameter_name }}. " | ||
| "Example: 'Detected {{ $parameters.num_objects }} objects. " | ||
| "Alert: {{ $parameters.classes }}.'" | ||
| ) | ||
| assert not _parses_as_jinja2(text) | ||
| result = _escape_jinja2_expressions(text) | ||
| assert _parses_as_jinja2(result) | ||
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.
Uh oh!
There was an error while loading. Please reload this page.