ocamllex: Accept line directives ended with CR/LF. (#12146) #12147
Conversation
scjung
force-pushed
the
ocamllex-crlf-line-directive
branch
from
a001cf1 to
1767878
Compare
nojb
left a comment
nojb
left a comment
There was a problem hiding this comment.
PR looks reasonable to me. I left a suggestion.
|
The change looks good to me. I think you should also open an issue on cppo. I think cppo should just preserve existing line ending |
|
Also, a Changes entry is in order. |
scjung
force-pushed
the
ocamllex-crlf-line-directive
branch
from
1767878 to
17e716d
Compare
scjung
force-pushed
the
ocamllex-crlf-line-directive
branch
2 times, most recently
from
13b7e08 to
ea592c2
Compare
|
CI fails because checking out |
damiendoligez
left a comment
damiendoligez
left a comment
There was a problem hiding this comment.
Looks good. Fix the CI problem and we can merge.
| | "#" [' ' '\t']* (['0'-'9']+ as num) [' ' '\t']* | ||
| ('\"' ([^ '\010' '\013' '\"']* as name) '\"')? | ||
| [^ '\010' '\013']* '\010' | ||
| [^ '\010' '\013']* '\013'* '\010' |
There was a problem hiding this comment.
You could also do it this way:
| [^ '\010' '\013']* '\013'* '\010' | |
| [^ '\010' ]* '\010' |
|
The CI is configured to complain about cr/lf at the end of files, and git does not handle it well either. The simplest route to merge this PR would be to simply remove test. @scjung, could you do this? |
gasche
added
the
submitter-action-needed
Has this been reported on the cppo project ? |
scjung
force-pushed
the
ocamllex-crlf-line-directive
branch
from
ea592c2 to
f891fd9
Compare
Sorry for late reply. I removed the test. |
5 participants
This PR fixes ocamllex to accept line-directives ended with CR/LF.
Use case: I'm building a tokenizer from CPPO-preprocessed source. On Windows, CPPO (v1.6.9) converts all LF to CF/LF. Because of that, ocamllex syntax errors occur on every line-directives generated by CPPO.
Looking at lex/lexer.mll and history, It seems that all CRs are ignored except for those at the end of line-directives.