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 gj/gk in visual block mode #2046

Merged
merged 11 commits into from
Nov 2, 2017
Merged

Conversation

orn688
Copy link
Contributor

@orn688 orn688 commented Sep 30, 2017

gj and gk did not work in visual block mode, because no motion for visual block mode corresponding to Move<Up/Down>ByScreenLine was implemented. Rather than truly moving up and down by a screen line, Move<Up/Down>ByScreenLineVisualBlockMode essentially just special-case MoveUp and MoveDown, following the example of Move<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!

@xconverge
Copy link
Member

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

@orn688
Copy link
Contributor Author

orn688 commented Sep 30, 2017

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 gk and gj in visual block mode (i.e., move up and down by one line), because that's all gk and gj do in visual line mode. Now that I think of it though, I guess that's not very sound reasoning since visual line mode selects lines at a time, not screen lines. I'll take a crack at making it actually work for wrapped lines.

@xconverge
Copy link
Member

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!

@Chillee
Copy link
Member

Chillee commented Oct 1, 2017

@oliver-newman Just out of curiosity, what's the motivation behind wanting this?

@orn688
Copy link
Contributor Author

orn688 commented Oct 1, 2017

I have j and k mapped to gj and gk in non-insert modes so I can move vertically through wrapped lines (I'm not a fan of horizontal scrolling, so I prefer to keep wrapping on). gj and gk work in visual block mode in Vim, and I'm so used to just using j and k to move through wrapped lines at this point that it'd be a pain to either get used to using gj/gk explicitly or to using arrow keys when in visual block mode. The reason I submitted this pull request even without it properly working was that, for my purposes, moving up/down by lines rather than screen lines with gj and gk in visual block mode is a whole lot better than having gj and gk not work at all, even if it diverges slightly from Vim's functionality.

@xconverge
Copy link
Member

@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

@jpoon
Copy link
Member

jpoon commented Oct 31, 2017

@oliver-newman, can you resolve the conflicts? Then we can merge this in.

@orn688
Copy link
Contributor Author

orn688 commented Nov 2, 2017

@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!

@jpoon
Copy link
Member

jpoon commented Nov 2, 2017

looks like the line endings got changed from LF to CRLF since I created this PR

Yeah, that was my f*ck up. Sorry.

@jpoon jpoon merged commit 7a1ac28 into VSCodeVim:master Nov 2, 2017
@jpoon
Copy link
Member

jpoon commented Nov 2, 2017

Thanks @oliver-newman!

@orn688 orn688 deleted the gj-gk-visual-block branch November 2, 2017 15:18
This pull request was closed.
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.

4 participants