Expand directive format validation - #14184
Conversation
| # Check if any directives are present | ||
| if re.search(r"\.\.\s*[a-zA-Z]+::", source_concat) is None: | ||
| # Check if text resembling directives are present | ||
| if re.search(r"\.\.\s*[a-zA-Z]+\s*:", source_concat) is None: |
There was a problem hiding this comment.
Made the initial check for whether a document should be inspected more closely more lenient, to allow for some more malformed instances.
| for idx, line in enumerate(source): | ||
| # Check for missing space after '..' | ||
| missing = re.search(r"\.\.[a-zA-Z]+::", line) | ||
| missing = re.search(r"\.\.[a-zA-Z]+\s*:", line) |
There was a problem hiding this comment.
Spaces can exist after the directive name but before the colons.
| dir_pattern = r"^\s*\.\. \w+::" # line might start with whitespace | ||
| dir_pattern = r"^\s*\.\.\s*\w+:*" # line might start with whitespace |
There was a problem hiding this comment.
Also check a preceding blank line is present for directives without a space between .. and the directive name (that is a problem, but it's nice if we can catch multiple issues with a given directive at once) or with >1 spaces (fine).
|
Local testing shows these other changes not related to the bad number of colons case still catches the errors we initially looked for. |
|
A number of cases of malformed directives which need fixing. |
| # Strip out name | ||
| directive_name = re.sub( | ||
| r"\.\.\s*([a-zA-Z\-]+)\s*(?<!:)(:{3,}|:)(?!:)", r"\1", line | ||
| ) |
There was a problem hiding this comment.
I neglected the fact before that single colons are acceptable for things like page links in examples/tutorials. There is now an additional check that the name of the potential bad directive matches a known sphinx directive, and only then gives a warning.
There was a problem hiding this comment.
Sounds good! We shouldn't have any links named like directives and if we do we should change them :)
|
I want to double check there isn't an instance of something that isn't being caught before merging |
2 participants
Fixes #14177