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

Fix incremental search not scrolling to current match #561

Merged
merged 2 commits into from
Dec 27, 2022

Conversation

citizenmatt
Copy link
Member

Fixes a couple of issues:

  • Fixes an issue with incremental search not scrolling to the current match. The caret was being moved using the platform's moveToOffset rather than IdeaVim's, which also handles scrolling
  • Fixes an issue with remembering the "end of line" state for $ not invalidating if the caret was moved and then moved back to its previous location. E.g. $hlj would act like $j and put the caret at the end of the next line, rather than simply moving down to the same column

@citizenmatt
Copy link
Member Author

I think there's an issue with the way listeners are registered - we're registering them with disposable, so they should be automatically unregistered, but we're manually removing them too, which causes a number of asserts during test run. I'm not sure if we're using the right disposable, either - I assume it should be the lifetime of Vim support being enabled/disabled, but I don't think it is.

@AlexPl292
Copy link
Member

Do you believe there is a way to fix these issues without caret subscribing at least for $hlj case? I'm not sure if this will be easy to get a caret subscription in Fleet.
I'll ask if there is such kind of subscription in Fleet.

@AlexPl292
Copy link
Member

By the way, big question: Do we know the reason why we store the last column by ourselves instead of using the last position that is stored somewhere in IJ for the same purpose?

@citizenmatt
Copy link
Member Author

Subscribing was the easiest way - I didn't look for anything else. We can fix it for Vim motions by explicitly invalidating the cache whenever any motion is made, but there is still a problem if the caret is moved by mouse, or by another IntelliJ action, and then moved back. Subscribing to caret move handles this. This data is stored in the caret's user data, and Fleet's carets are immutable, so what happens to the data when a Fleet caret is moved? Is there some kind of version field so we can know it's the same caret? If it's a different caret, it's moved and this data is invalid.

IntelliJ stores a "desired X" value inside CaretImpl, and Caret.moveCaretRelatively would allow us to use this (possibly with other undesirable side effects, I'm not 100% sure exactly what else this method does). But the big problem is that there isn't an "end of line" marker, so we'd still need to store a flag for $, and then we're back in the scenario of having to invalidate it.

@AlexPl292 AlexPl292 merged commit c0e17a6 into JetBrains:master Dec 27, 2022
@citizenmatt citizenmatt deleted the bugfix/incsearch branch December 28, 2022 12:27
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