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

The Great Line Ending & Cursor Range Cleanup #376

Merged

Conversation

cessen
Copy link
Contributor

@cessen cessen commented Jun 25, 2021

Addresses #362.

Switch all code over to:

  1. Use gap indexing.
  2. Not assume a file-final line-ending.

All code has been adjusted so that it still presents an on-char indexing model to the user, but internally everything works in terms of gap indexing.

@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch 2 times, most recently from b194d54 to 9493688 Compare June 30, 2021 12:49
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from 51d0168 to e725957 Compare July 1, 2021 21:24
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from a1f9ec8 to b4c59b4 Compare July 8, 2021 23:47
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from 774a8e1 to 954314a Compare July 17, 2021 18:03
@cessen cessen marked this pull request as ready for review July 19, 2021 06:11
@cessen cessen mentioned this pull request Jul 28, 2021
2 tasks
@cessen
Copy link
Contributor Author

cessen commented Jul 28, 2021

This is almost done. There's just one last bug I need to track down, where / searching doesn't seem to work correctly after CJK characters. (My guess is it's a mismatch between byte and char indices, but I haven't tracked it down yet.)

Forgot to convert from char indices to byte indices before passing
to the regex engine.
@cessen
Copy link
Contributor Author

cessen commented Jul 28, 2021

Yup, silly mistake on my part. Fixed!

I think this is ready for a final round of reviews, now.

@cessen cessen requested a review from archseer July 28, 2021 23:05
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, after a bunch of testing I think this is good enough for merge, and we can deal with any residual issues on master. Great work! 🎉

@archseer archseer merged commit 05d20e1 into helix-editor:master Jul 29, 2021
@CBenoit
Copy link
Member

CBenoit commented Jul 29, 2021

Great work!

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.

4 participants