Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Add CR to ignore list for multi-line check #12

Merged
merged 1 commit into from
Oct 9, 2018

Conversation

bryphe
Copy link
Contributor

@bryphe bryphe commented Oct 3, 2018

Issue: When using ppx_expect with Windows-style newlines (CRLF vs LF), the multi-line expectation fails with:

         ppx lib/Util.pp.ml (exit 1)
(cd _build/default && .ppx/ppx_expect+ppx_inline_test+ppxlib/ppx.exe --cookie "library-name=\"lib\"" -o lib/Util.pp.ml --impl lib/Util.ml -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
File "lib/Util.ml", line 7, characters 13-26:
Error: Multi-line expectations must start with an empty line

Defect: The current code considers the CR (/r) a character, and fails the check.

Fix: Ignore the /r as we do for other whitespace.

We hit this in our Windows build of esy: esy/esy#499

Signed-off-by: Bryan Phelps [email protected]

@ghost
Copy link

ghost commented Oct 4, 2018

Thanks, I imported the patch in our internal repository

@xclerc xclerc merged commit 2e386fa into janestreet:master Oct 9, 2018
@bryphe bryphe deleted the bryphe/windows-fix-crlf branch October 9, 2018 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants