Don't normalize CRLF to LF in Junk #193
Conversation
syntax/grammar.mjs
Outdated
| line_end) | ||
| either( | ||
| string("\u000A"), | ||
| string("\u000D\u000A"), |
There was a problem hiding this comment.
I wonder if it would make sense to add other, technically unsupported, line endings here. In particular, I'm thinking of \r, \u2028 and \u2029, which are not matched by . in the JavaScript regex implementation.
We don't need to include all line terminators recognized by Unicode, but adding these extra three would make sure the reference parser can produce meaningful junk for files with these endings. Otherwise, the parser doesn't have any way to skip over the EOL in the junk_line production, and the parsing of the whole resource ends immediately.
There was a problem hiding this comment.
Would regex(/[^\n]*/) be closer to our intent? And not change line_end at all?
There was a problem hiding this comment.
This is a really good idea! Thanks.
I had some trouble making it work when I tested it. I was seeing additional \ns where I was sure there shouldn't be any. And then I realized that I needed to use echo -n for my CLI testing...
|
@Pike This is now ready for another round of review. Thanks! |
Fix #184.
This doesn't change the parsing story for unsupported line endings (
\r,\u2028,\u2029). Right now, that story is largely undefined.