Skip to content

Implement line-ending recognition and normalization #353

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

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

MinusGix
Copy link
Member

@MinusGix MinusGix commented Mar 7, 2024

This is a companion PR to lapce/lapce#3047

This implement basic line-ending support.
Fixes #3029 because I was focusing on that issue and I decided to just implement line-ending changing while I was at it.

The cause of that issue is that x-rope does not consider Carriage Return (\r) as a newline character, but cosmic-text does (and it panics if there is more than one paragraph for our text layouts). CRs in terminals are meant to go to the start of the current line, but old MacOS used it as a line-ending character; as well, other editors like VSCode/GEdit/... treat it as a newline character. We imitate other editors which simply replace a lone \r character with the line-ending of the file.

However, on loading content the editor does not normalize the line-endings, because for large files that could be a lot of replacements. So there is no standardized in-memory newline that we use, which I believe is fine as I don't believe anything cares about that (especially since that was the editor's behavior for the last ~while). The only normalization is with normalize_limited which just gets rid of lone CRs since cosmic-text can't handle it.

The primary alternative solution to replacing all lone CRs would be to modify Rope to consider it as a newline character, but at a glance that appears nontrivial and adds extra work to many common operations. Another alternative would be to treat \r as a whitespace character and modify cosmic-text to treat it as such. That option is inconsistent with other editors treating it as a newline, however, so I dismissed it as an option.

@dzhou121 dzhou121 merged commit 3c41a54 into lapce:main Mar 7, 2024
7 checks passed
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.

[CRASH] Carrige return (followed by string) crashes Lapce
2 participants