Repository navigation
Fix pretty() of patterns so the output recompiles to the same regex - #431
Open
binggao1230 wants to merge 1 commit into
Open
binggao1230 wants to merge 1 commit into
binggao1230 wants to merge 1 commit into
Conversation
Pattern._pretty() ran the pattern through trim() and switched to the ?"..." form whenever it held a slash. But pretty() reindents its output line by line, so a regex spanning lines came back with the indentation inside it, and ?"..." holds neither a line break nor a double quote, so those grammars did not recompile at all. Escape the characters str.splitlines() breaks on, keep ?"..." where it is legal, and use the /.../ form with escaped slashes otherwise. Patterns under (?x) keep the existing layout normalisation, where whitespace is not significant. Grammar._pretty() rendered the regex directives through the same branch, escaping nothing.
Contributor
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pretty()output should recompile to the same grammar; for regexes it does not, mostly without an error.Pattern._pretty()trimmed the pattern and switched to the?"..."form whenever it held a/.The printer reindents line by line, so a regex spanning lines comes back with the new indentation inside it — silently a different grammar:
?"..."holds neither a line break nor a double quote (DOUBLEQUOTEDis/"((?:[^"\n]|\\"|\\\\)*?)"/), so the recompile raises outright on/line1\/\nline2/, and on any pattern with both a/and a". Thepat.replace('"', r'\"')in that branch cannot help —\"does not parse inside?"..."either.Auditing the rest of the surface: 19 of the 28 round-trip cases in the new test fail on main. Every character
str.splitlines()breaks on (\v \f \r \x1c-\x1e \x85 \u2028 \u2029), whitespace at either end of the pattern and tabs (both eaten bytrim()), and the same at every nesting depth.Grammar._pretty()sends@@comments/@@eol_comments/@@whitespacethrough the same'/' in valuebranch and escapes nothing, so@@comments :: ?'/[*].*?[*]/|"[^"]*"'prints source that does not parse.So: escape what the printer would rewrite, keep
?"..."where it is legal,/.../with escaped slashes otherwise. Patterns under(?x)keep the existing layout normalisation — whitespace is not significant there, and escaping it would be.This is the case #430's description offered to file separately. Different module, no dependency on it, though both add to
tests/grammar/pretty_test.py.Also ran a
pretty → compile → prettyfixed-point sweep over every grammar in the repo, which stays clean. 501 pass, 6 skipped.