fix(catalog): fix PYTHON_FORMAT regex false positives for percentage text

[goodluck-ry]

Fixes #1268
Supersedes #1321

Hi @akx,

I've reverted the changes in frontend.py and refactored the PR to focus entirely on the regex rule in catalog.py.

The root cause of the false positive (such as '10% of' breaking builds) was that line 72 of catalog.py matched the space following '%' as a Python format flag in PYTHON_FORMAT. Removing the space flag from the regex prevents strings like '10% of' from being recognized as placeholders in the first place.

The original issue suggested downgrading errors from ERROR to WARNING. However, applying a downgrade in frontend.py would silently swallow real format mismatches. Fixing the regex at the source ensures that true syntax errors are still safely caught and exit with status 1, while the false positives are eliminated without touching error severity.

Trade-off note: Since explicit space-padded specifiers (like '% d') and text like '10% der' are textually identical at the static regex level, removing the space flag means '% d' won't be flagged. However, space-padded specifiers are extremely rare in translations compared to percentage text, making this a clean and pragmatically minimal fix.

I have updated the branch with corresponding tests for this case. Let me know what you think!

[Mukller]

Thanks for tackling this — the \100 % done\ false positives are real and annoying.

I verified the change empirically against both regex versions:

input	master	this PR
\100 % done\ (prose)	matched (FP)	no match
\50 % off\ (prose)	matched (FP)	no match
\% d\ (valid placeholder)	matched	no match
\% 5d, \% f, \% e\	matched	no match

The problem: the space character is a legitimate printf flag ('% d' % 3 == ' 3', Python docs), so dropping \\ \\ from the flags class turns current false positives into false negatives for translations that legitimately use spaced placeholders.

A middle ground that keeps both cases working: allow the space flag only when explicit width/precision digits follow (bare % \ is almost always prose):

\\python
PYTHON_FORMAT = re.compile(
r'''
\%
(?:\(([\w])\))?
(
(?:[-#0+]?(?:\|[\d]+)?(?:\.(?:\*|\[\d]+))?)
|
(?:\ +[\d]+(?:\.[\d]+)?)
)
[hlL]?
([diouxXeEfFgGcrs%])
''',
re.VERBOSE,
)
\\

With this variant: % done\ / % off\ don't match, while % 5d, % f-style placeholders still do. Related open issue about PYTHON_FORMAT quirks: #865. Could you add test cases for both directions to the PR? Happy to re-review.

[Mukller]

(Reposting with fixed formatting — the review above got mangled by my tooling, apologies.)

Thanks for tackling this — the 100 % done false positives are real and annoying.

I verified the change empirically against both regex versions:

input	master	this PR
100 % done (prose)	matched (FP)	no match ✅
50 % off (prose)	matched (FP)	no match ✅
% d (valid placeholder)	matched	no match ❌
% 5d, % f, % e	matched	no match ❌

The problem: the space character is a legitimate printf flag ('% d' % 3 == ' 3', see Python docs), so dropping \ from the flags class turns current false positives into false negatives for translations that legitimately use spaced placeholders.

A middle ground that keeps both cases working — allow the space flag only when explicit width digits follow (a bare % <letter> is almost always prose):

PYTHON_FORMAT = re.compile(
    r'''
    \%
        (?:\(([\w]*)\))?
        (
            (?:[-#0+]?(?:\*|[\d]+)?(?:\.(?:\*|[\d]+))?)
            |
            (?:\ +[\d]+(?:\.[\d]+)?)
        )
        [hlL]?
        ([diouxXeEfFgGcrs%])
''',
    re.VERBOSE,
)

With this variant: % done / % off don't match, while % 5d-style placeholders still do.

Related open issue about PYTHON_FORMAT quirks: #865.

Could you add test cases covering both directions (prose false positives and spaced valid placeholders) to the PR? Happy to re-review.

[Mukller]

Self-correction before anyone wastes time on my earlier suggestion: I verified my proposed pattern empirically and it had a gap — the second branch required digits after the space flag, so % .2f (space + precision, no width) would not match, even though '% .2f' % 1.5 == ' 1.50' is valid Python.

Corrected alternative — space flag is allowed only when followed by width digits or a precision dot:

(
    (?:[-#0+]?(?:\*|[\d]+)?(?:\.(?:\*|[\d]+))?)   # current form, minus space flag
    |
    (?:\ +(?:\.[\d]+|[\d]+(?:\.[\d]+)?))          # space flag requires explicit width/precision
)

Verified against: 100 % done ✗ / 50 % off ✗ (prose stays out), % 5d ✓ / % .2f ✓ / % 5.2f ✓ / %s, %5d ✓.

Remaining known limitation, which I think is fundamental rather than fixable: bare % f / % e (space flag + conversion, no width/precision) stay unmatched — they're indistinguishable from prose like % done at the regex level. Master matches them today, so this trade-off should be an explicit maintainer decision; if losing bare % f is unacceptable, the PR's current approach (drop the space flag entirely) vs. this middle ground vs. status quo are the three real options.

[goodluck-ry]

Self-correction before anyone wastes time on my earlier suggestion: I verified my proposed pattern empirically and it had a gap — the second branch required digits after the space flag, so % .2f (space + precision, no width) would not match, even though '% .2f' % 1.5 == ' 1.50' is valid Python.

Corrected alternative — space flag is allowed only when followed by width digits or a precision dot:

(
    (?:[-#0+]?(?:\*|[\d]+)?(?:\.(?:\*|[\d]+))?)   # current form, minus space flag
    |
    (?:\ +(?:\.[\d]+|[\d]+(?:\.[\d]+)?))          # space flag requires explicit width/precision
)

Verified against: 100 % done ✗ / 50 % off ✗ (prose stays out), % 5d ✓ / % .2f ✓ / % 5.2f ✓ / %s, %5d ✓.

Remaining known limitation, which I think is fundamental rather than fixable: bare % f / % e (space flag + conversion, no width/precision) stay unmatched — they're indistinguishable from prose like % done at the regex level. Master matches them today, so this trade-off should be an explicit maintainer decision; if losing bare % f is unacceptable, the PR's current approach (drop the space flag entirely) vs. this middle ground vs. status quo are the three real options.

Thanks so much for the detailed analysis, empirical validation, and the refined regex logic!

I have updated catalog.py using the proposed middle-ground pattern (requiring explicit width/precision for space flags) and added comprehensive test coverage:

1. Code & Test Changes

catalog.py: Updated PYTHON_FORMAT to handle space-flagged specifiers only when followed by explicit width digits or a precision dot.

test_catalog.py: Added test_message_python_format_prose_percent to ensure prose like 100 % done, 50 % off, 10% of, and 10% der are ignored, while space-padded specifiers with explicit width/precision continue to match.

test_checkers.py: Added test cases ensuring translations with these prose strings pass validation cleanly.

2. Trade-offs & Edge Cases Observed
   As discussed, static regex has fundamental limits with text homographs. For transparency, here are the remaining trade-offs with this pattern:

Bare space flags: As noted, bare % f or % d (without width/precision) remain unmatched to avoid catching prose.

Prose adjacent to digits: Phrases like 100 % 5off or offer % 5x will still match as % 5o / % 5x because 5 triggers the width rule followed by a valid format specifier character.

Dynamic width (% *d): Space-padded specifiers using * for dynamic width (e.g., % *d) are not matched by the explicit digit branch.

All local tests are passing cleanly. Thanks again for guiding this to a much cleaner solution! Ready for re-review whenever you have time.

[Mukller]

Verified your updated branch locally — the middle-ground pattern behaves exactly as specified:

input	expected	on branch
100 % done / 50 % off / 10% of / 10% der	no match	✓
% 5d / % .2f / % 5.2f / %s / %5d	match	✓
Name: %s, %d years (mixed)	match	✓
bare % f (documented limitation)	no match	✓

pytest tests/messages/test_catalog.py -k python_format — 3/3 green, including the new prose-regression test.

I also re-checked the checker-side tests you mentioned: prose strings pass validation cleanly while real format-specifier mismatches still raise. The trade-off table you wrote (bare % f/% e staying unmatched) is accurately stated — that's indeed the irreducible ambiguity at regex level, and leaving it as an explicit maintainer decision is the right call.

Nothing further from my side; the implementation matches everything we discussed.