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

Track document changes in the jumplist #3592

Conversation

the-mikedavis
Copy link
Member

Previously, the jumplist was not updated by changes to the document. This meant that the jumplist entries were prone to being incorrect after some edits and that usages of the jumplist were prone to panics if, for example, some edits truncated the document.

This change applies Transactions (document edits) to the selections stored in any jumplists.

Implementation-wise I used a Rc<RefCell<Selection>> so that the Document could edit the selections when applying transactions and then the View could use the updated selections for jumping. Kakoune does this by waiting to apply transactions until C-o/C-i are used. I would prefer to do all of that work incrementally though since we have the jumplist picker (we would need to update all selections with all changes since the jumps were saved which seems like it could be a lot of work).

Closes #3550
Closes #2489

Previously, the jumplist was not updated by changes to the document.
This meant that the jumplist entries were prone to being incorrect
after some edits and that usages of the jumplist were prone to panics
if, for example, some edits truncated the document.

This change applies `Transaction`s (document edits) to the selections
stored in any jumplists.
@Jummit
Copy link
Contributor

Jummit commented Sep 9, 2022

What's the status on this? I've face this issue and lost a bunch of work, and it was reported by three people, so there are probably lots of affected users. I think it would be cool to see this merged sooner than later.

@the-mikedavis the-mikedavis linked an issue Sep 9, 2022 that may be closed by this pull request
@kirawi kirawi added C-bug Category: This is a bug A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Sep 13, 2022
@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 10, 2022
@the-mikedavis
Copy link
Member Author

This approach works but the Rc<RefCell<Selection>> is kinda weird: it's odd having the selections on the Document and the View. Talking with archseer about this, we think we can apply changes to Views so that they can go against View.jumps directly. I'll open a separate PR that closes this one.

@the-mikedavis the-mikedavis deleted the md-jumplist-track-changes branch October 11, 2022 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
4 participants