Repository navigation
Conversation
|
A small line-terminator test, if it's useful. The LF cases pass with this change, but the regexp rule still matches across CR, U+2028 and U+2029, including immediately after a backslash. The test covers all four terminators plus a normal-regexp control. On Windows/Python 3.14.4, six cases still fail on this head; both LF cases and the control pass. The full Babel/CLDR suite wasn't run. Would you consider excluding those other ECMAScript line terminators from both the ordinary and backslash paths too? |
c6e18a1 to
2ef3aef
Compare
|
All four terminators are handled now ( What changed, following your split:
Both of your failing shapes are now red on the old head and green on this one: 6 failed / 10 passed on Tests added, mirroring your test:
Verification: I amended the single commit rather than adding a fixup, so the branch stays one commit; the PR description now covers all four terminators and credits your measurement. |
akx
left a comment
akx
left a comment
There was a problem hiding this comment.
Please rebase and drop the specific jslexer tests. It's enough to test the extraction result.
A regexp literal cannot contain an ECMAScript line terminator, and a backslash cannot escape one either, so the fallback regexp rule must not match across a line break; it previously swallowed the rest of the source and silently dropped every gettext() call it contained (issue python-babel#640). Per review, drop the jslexer-level token assertion tests; the extraction result is what matters.
2ef3aef to
33328b9
Compare
|
Rebased onto current
The production change is unchanged. It is still needed after the rebase: Verified: extraction tests 129 passed, 1 skipped (124 before); the three JS files 57 passed (52 before); full suite 7784 passed, 9 skipped, 2 xfailed; |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1392 +/- ##
=======================================
Coverage 93.56% 93.56%
=======================================
Files 27 27
Lines 4943 4943
=======================================
Hits 4625 4625
Misses 318 318
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Fix jslexer regexp literals spanning lines (issue #640)
The tokenizer matched a JS regexp literal with
re.DOTALL, so a match attempt could run past line terminators and swallow everything up to the next/on a later line. That silently dropped everygettextmessage between such literals — exactly the symptom in #640, where the message between/>/gand/'/gdisappears.A JS regexp literal cannot contain any of the ECMAScript line terminators — U+000A, U+000D, U+2028 or U+2029 — and a backslash does not continue one onto the next line, so dropping
DOTALLand excluding all four terminators from both the ordinary and the escaped character classes bounds a failed match to a single line. With the source from #640, for LF, CR, CRLF, U+2028 and U+2029 alike:['First message', 'Third message'](matches the report's POT output)['First message', 'Second message', 'Third message']Thanks to @carlitogwapito, who measured that CR, U+2028 and U+2029 still matched — in the ordinary path and immediately after a backslash — and supplied failing cases for them.
The new
test_regexp_literal_cannot_span_linesandtest_regexp_literal_cannot_continue_after_a_backslashpin the tokenizer behaviour for all four terminators,test_ordinary_regexp_literals_still_matchkeeps escaped slashes and flags working, andtest_regexp_literals_do_not_swallow_messagescovers both regexp shapes from #640 at the extraction level, parametrised over the terminators.One existing assertion changes meaning:
test_jsx_extraction's source also contains regexp literals, and the buggy lexer dropped messages from it in non-JSX mode, which is what the old!=assertion was locking in. With extraction fixed, non-JSX now yields the same messages as JSX, so the assertion becomes==(kept asif/elseso both parametrised cases stay visible in the report).Prior art: #791 rewrites this pattern to handle
/inside character classes, but it keepsDOTALL(its character classes also match newlines), so it does not address the newline case in #640. The two fixes are independent; mine only changes line-terminator handling.I used an AI assistant (ZCode) to help locate the swallowing behavior in the tokenizer and to draft the tests; I verified the before/after behavior above myself and wrote the fix.