Match only ASCII digits in the lexer number patterns - #195
Conversation
In Python 3 \d matches every Unicode decimal digit, so the lexers read full-width digits as numbers. \tempo 4 = ６０ lexed ６０ as an IntegerValue and #(display ４２) lexed ４２ as a Number, though neither is legal LilyPond. Replaced \d with [0-9] in the 12 patterns that match a digit, across ly/lex/scheme.py, ly/lex/lilypond.py and ly/lex/html.py. Six other \d uses are left as they are. They sit inside negated character classes or negative lookaheads, where \d correctly excludes every digit. Rewriting [^\W\d_] as [^\W0-9_] would start accepting ４ as an identifier character, which is the bug this issue reports rather than a fix for it. Fixes #166
This is a good explanation. I'd add a comment in the source files as well so future maintainers will understand why not to change it back. (Something slightly briefer like
This should also be included as a comment. |
Records why \d must not come back, and why the negated classes keep it.
|
Done in e05d8dd. The combined note sits above # digit patterns use [0-9]: \d also matches Unicode digits that are not legal
# LilyPond. \d in a negated class excludes those too, so it staysscheme.py and html.py get a one-liner each at the pattern itself. You did not comment on the html.py change, so checking before you review again: keep it, or would you rather this stayed in the LilyPond and Scheme lexers only? |
|
Looks good to me. @fedelibre and @ksnortum, do you want to double-check anything before we merge?
Let's keep it. I don't see any reason we'd need to handle HTML separately. |
|
Looks good to me. |
|
Cool, then let's do this. |
4 participants
In Python 3
\dmatches every Unicode decimal digit, so the lexers read full-width digits as numbers.\tempo 4 = ６０lexes ６０ as an IntegerValue and#(display ４２)lexes ４２ as a Number, though neither is legal LilyPond. Fixes #166.Replaced with
[0-9]in the 12 patterns that match a digit: 3 in scheme.py, 8 in lilypond.py, 1 in html.py.Six other
\duses are left alone on purpose. They sit inside negated classes or negative lookaheads, where\dcorrectly excludes every digit. Rewriting[^\W\d_]as[^\W0-9_]would start accepting ４ as an identifier character, which is this bug rather than a fix for it.tests/test_lex.pyis new, so this is the first test for the lexers. Mutation testing covers 10 of the 12 patterns. Scaling and TempoSeparator resist it because both need a valid ASCII number earlier in the expression, so full-width input never reaches them at all.The html.py line is the loosest fit, HTML numeric character references being a different language. Happy to drop it if you would rather keep this to the LilyPond and Scheme lexers.