-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 gj/gk in visual block mode #2046
Conversation
I just tested this and this now kind of works...but it does not actually visual block on wrapped lines and gj and gk all work for example, but if you are trying to visual block on a wrapped line section it does not properly |
I was aware of that; I didn't try to make it actually work with wrapped lines, I just wanted it to actually respond to |
it is going to be pretty tricky, just a warning! The way we fake visual block mode is a bit tricky if I remember correctly... sorry I can't be of more help/guidance but we definitely appreciate the help so far! |
@oliver-newman Just out of curiosity, what's the motivation behind wanting this? |
I have |
@Chillee @oliver-newman I am actually OK with adding this for now and opening another issue for the case where it doesn't work (actual wrapped line situation). Since I think the gj to j binding is relatively common |
@oliver-newman, can you resolve the conflicts? Then we can merge this in. |
@jpoon looks like the line endings got changed from LF to CRLF since I created this PR. I changed them to CRLF on my branch so I think it's all set to be merged, but let me know if there are any problems. Thanks! |
Yeah, that was my f*ck up. Sorry. |
Thanks @oliver-newman! |
gj
andgk
did not work in visual block mode, because no motion for visual block mode corresponding toMove<Up/Down>ByScreenLine
was implemented. Rather than truly moving up and down by a screen line,Move<Up/Down>ByScreenLineVisualBlockMode
essentially just special-caseMoveUp
andMoveDown
, following the example ofMove<Up/Down>ByScreenLineVisualLineMode
which also just moves by lines instead of by screen lines.Fixes #1772.
I am quite inexperienced with this sort of repo (not to mention TypeScript in general) so I greatly appreciate any comments/suggestions for improvements. It seems like there should be a better way to implement this (e.g., by inheriting from
MoveByScreenLine
) but after trying a few such simpler approaches this was the first one that worked. Thanks for a great extension!