helix: Fix normal-mode selection expansion during vim::Scroll commands#47024
Conversation
vim::Scroll commands
vim::Scroll commandsvim::Scroll commands
|
When I build this, I still see the issue. Am I missing something or should the condition check be something like this? let is_helix_mode = matches!(mode, Mode::HelixNormal | Mode::HelixSelect);
if selection.is_empty() || is_helix_mode {
selection.collapse_to(new_head, goal)
} else {
selection.set_head(new_head, goal)
}; |
|
That isn't quite correct - I expect it to expand the selection if you're in select mode, which is helix's visual mode. I think that would miss vim's normal mode as well, and you'd still see selection expansion if you make it with the mouse and then use the scroll commands. What situation are you still seeing the incorrect behavior in? |
36ad3a8 to
96cf4fa
Compare
|
Updated the test to ensure that the selection does get extended in helix select mode. |
dinocosta
left a comment
There was a problem hiding this comment.
Hey @jrobsonchase 👋
Thank you so much for fixing this and including tests for it too! I've left a single comment regarding the test, as I believe there's a place where state is being set where the intent might have been to actually assert state.
Let me know if you have any questions! 🙂
96cf4fa to
2c21d5a
Compare
2c21d5a to
765fb59
Compare
dinocosta
left a comment
There was a problem hiding this comment.
Thank you for taking another look @jrobsonchase ! I've pushed a single commit fixing indentation in one comment, otherwise everything's good to go.
Will merge this shortly. Thanks! 🙂
vim::Scroll commandsvim::Scroll commands
Fixes the
vim::ScrollUp/Downcommands when a normal-mode vim or helix selection exists.Pretty straightforward: make the
scroll_editorfunction aware of the current vim/helix mode, and only expand the selection if a visual mode is active.Closes #47022
Release Notes:
vim::Scrollcommands