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

Correctly count CRLF newlines when counting line offsets. #1007

Merged
merged 1 commit into from Dec 8, 2018

Conversation

Projects
None yet
3 participants
@akovaski
Copy link
Contributor

akovaski commented Dec 7, 2018

This allows completion coordinates to find the correct position in the file when using Windows style newlines (CRLF). I added two simple tests which are slight tweaks of the coords_to_point_works test in core.rs.

Notably for me, this allows RLS autocomplete to function correctly when working on files with Windows style newlines.

See issue 976 in the rust-lang/rls repo. I believe this behavior was merged to the racer github repo on Jun 24, 2018 with commit 42046fc.

Example image of the autocomplete problem in VSCode on Windows. This is a file with CRLF newlines. Notice that the cursor is on line 7, but the auto-completion is for the code that exists on line 5.
image

Now the autocomplete will autocomplete line 5 at the end of line 5, where the user will want it to. 🎉
image

Correctly count CRLF newlines when couting line offsets.
This allows completion coordinates to find the correct position in the
file when using Windows' newline characters (CRLF).
@Xanewok

This comment has been minimized.

Copy link
Contributor

Xanewok commented Dec 8, 2018

Nice find! This actually fixes Windows CI failure for RLS, which was mismatched auto-completion based on lines https://travis-ci.org/Xanewok/rls/jobs/465194944 (errors because of debug Error: StringError("actually ok") but it's actually fixed now) 🎉

@kngwyu

This comment has been minimized.

Copy link
Collaborator

kngwyu commented Dec 8, 2018

Great, thanks 👍
@Xanewok
Should I publish new version for rls?

@kngwyu kngwyu merged commit eca9dce into racer-rust:master Dec 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Xanewok

This comment has been minimized.

Copy link
Contributor

Xanewok commented Dec 8, 2018

@kngwyu I think that’d be great, yes. Windows users get their autocompletion back so I think that alone meets the point release bar 👍

@kngwyu

This comment has been minimized.

Copy link
Collaborator

kngwyu commented Dec 8, 2018

Published v2.1.15 now

@kngwyu

This comment has been minimized.

Copy link
Collaborator

kngwyu commented Dec 8, 2018

I yanked 2.1.15 because of a really silly miss 4c2d363
Please use 2.1.16 😓

@akovaski akovaski deleted the akovaski:crlf_lines_fix branch Dec 8, 2018

@Xanewok

This comment has been minimized.

Copy link
Contributor

Xanewok commented Dec 8, 2018

Thanks for the release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment