fix(checks): Fix false positives in placeholders for Punctuation spacing check#18600
fix(checks): Fix false positives in placeholders for Punctuation spacing check#18600gersona wants to merge 5 commits intoWeblateOrg:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes false positives in French punctuation spacing by ignoring punctuation inside placeholders/placeables, and refactors placeholder/format stripping to use the centralized highlighting mechanism.
Changes:
- Added XLIFF placeholder coverage to punctuation spacing autofix and related checks.
- Introduced
replace_highlighted()+placeholder_replacement()utilities and refactored duplicate/same logic to use them. - Updated changelog to note the reduced false positives for placeholder content.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| weblate/trans/tests/test_autofix.py | Adds regression tests for punctuation spacing autofix with XLIFF placeholders. |
| weblate/trans/autofixes/chars.py | Updates punctuation spacing autofix to skip replacements inside highlighted ranges (placeables). |
| weblate/checks/utils.py | Adds replace_highlighted() and placeholder_replacement() helpers built on highlight_string(). |
| weblate/checks/tests/test_utils.py | Adds unit tests for replace_highlighted() default and callable replacement behavior. |
| weblate/checks/tests/test_same_checks.py | Extends same-check tests to cover XLIFF placeholder regex. |
| weblate/checks/tests/test_duplicate_checks.py | Adds tests ensuring duplicates inside placeholders are ignored. |
| weblate/checks/tests/test_chars_checks.py | Adds tests ensuring punctuation-spacing check ignores punctuation inside XLIFF placeholders. |
| weblate/checks/same.py | Refactors stripping logic to use centralized replace_highlighted() instead of bespoke stripping. |
| weblate/checks/duplicate.py | Refactors duplicate detection to replace highlighted spans with stable placeholders. |
| weblate/checks/chars.py | Updates punctuation spacing check to ignore highlighted ranges and disables “Fix” button when placeables exist. |
| docs/changes.rst | Documents the improvement to the punctuation spacing check regarding placeholders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for start, end, _text in highlights: | ||
| if start < last_end: | ||
| continue | ||
| result.append(source[last_end:start]) | ||
| if callable(replacement): | ||
| result.append(replacement(start)) | ||
| else: | ||
| result.append(replacement) | ||
| last_end = end |
There was a problem hiding this comment.
replace_highlighted() can fail to replace the full intended range when highlights contains partially-overlapping spans (e.g., (0, 5) and (3, 8)). In that case the second span is skipped (start < last_end) and last_end is not extended, leaving a tail of the highlighted content unmodified. Update the overlap handling so that when start < last_end, the function still advances last_end = max(last_end, end) (without appending a second replacement), ensuring the union of overlaps is removed/replaced.
There was a problem hiding this comment.
In practice, is it possible that some multiple placeholders overlap without being nested ? This seems unlikely to me. And none of the tests in weblate/checks/tests/test_utils.py go in that direction
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
fix spacing punctuation fix to ignore placeholders
replace strip_format uses with highlight_string
changelog update
fixes Fixing spacing punctuation in XLIFF placeholder breaks Angular application #17224
Also refactors some utility function by replacing all use of strip_format with highlight_string