Repository navigation
Conversation
Five new fixes: read Git filenames with NUL delimiters; distinguish added increment expressions from patch headers; reject misplaced gitlinks; normalize document URL destinations; escape annotation filename properties. Markdown code-context parsing was excluded after review. Linux/Python 3.12: 15 conventions tests pass, including real temporary Git repositories with Unicode and control characters in filenames. New regression cases fail the upstream checker. Both upstream and changed checkers and whitespace checks pass. (cherry picked from commit 5a2e939)
Use Unicode and space filenames on Windows while retaining tab and newline filename coverage on Linux. Both selections were verified on Linux; actual Windows execution remains untested. (cherry picked from commit 3591058)
|
Closing this PR because it republishes implementation from MrMha's intake-paused contribution (#3021). I should not resubmit another contributor's work while their PR is intake-paused. |
Dextroid17
left a comment
Dextroid17
left a comment
There was a problem hiding this comment.
Five PRs from one author landed within minutes of each other and share a purpose, though they are not all the same kind of change, so this is written once rather than five times.
One thread is replacing a pattern-match over text with a parse of its structure. Git output is read NUL-delimited so unusual filenames survive intact; added source lines are told apart from diff headers by a state machine rather than a prefix test; percent-encoded document links are normalised on read; a PR's dependency list is bounded to the declaration itself, so a later #N on the same line cannot suppress an overlap check; and CMake, CTest and Ninja output is read with the escaping those formats actually use.
The other thread is ordering, and it belongs to the two that are not parsers at all: the stub generator and the release packager now prepare their output before replacing what exists, and stage an archive before replacing a published one. A failed run then leaves the old artefact rather than nothing. Two of the five state that limit in the body — the release staging and the CI report — which is the right way to ship a small change.
Two details are bug classes rather than nits. Reading git's output NUL-delimited is the only way a filename containing a newline survives the trip. And escaping a filename before it reaches a workflow annotation is defence rather than tidiness: a name carrying %0A or :: can otherwise forge a line in the job log. That escaping sits on the error path, which is the only path this checker emits on — I went looking for a warning or notice path to ask about and there is not one, so this is complete rather than half-done.
All five carry MrMha's commits with authorship intact — three of them his alone, two mixing in your own follow-ups — and landing a stalled contributor's tested work under their name is worth more than a fresh review. Seeing it done five times in one batch is the most useful thing in this set.
Tell me which one you want read line by line and I will take it alone.
Written with AI assistance.
What
Harden the conventions check around Git paths and Markdown links. The checker now parses NUL-delimited Git output (so unusual filenames remain intact), distinguishes added source lines from diff headers, rejects misplaced submodules, normalizes percent-encoded document links, and escapes filenames in workflow annotations. Portable temporary-Git fixtures cover Unicode and control-character paths.
Open #2373 changes a separate system-dependency exemption in this checker; the changes occupy different code paths and do not implement the same rule.
This carries MrMha's tested commits from intake-paused PR #3021, preserving their authorship.
Tested
python3 -m unittest tools.tests.test_check_conventions_comments(15 tests passed).python3 tools/check_conventions.py --base upstream/mainpassed.git diff upstream/main...HEAD --checkpassed.Checklist
main; no open PR implements these path and link checks