fix wrapping behavior of x

[jacekkopecky]

x should only wrap to next line with vim-mode.wrapLeftRightMotion = true, and only with a count or on an empty line.
The PR also adds a bit to the tests for X.

[maxbrunsfeld]

I'm pretty sure h should perform this motion for any operator, not just d (e.g. y, gU, g~). Could you try checking @vimState.mode is 'operator-pending' instead of options.deleting? If that works, you wouldn't need the deleting option that you added on general-operators.coffee:87, and could probably 🔥 these comments ☝️.

[jacekkopecky]

Oh right, I'll check that - it would be cleaner indeed. I just need to ask for patience please, my dayjob is extra busy right now so I can't give vim-mode as much time as I'd like... Thank you.

[jacekkopecky]

Just checked it and it's not simple:

• gUl will not move the cursor in VIM, but in vim-mode it does (I may look into that)
• gUl will do nothing in VIM on an empty line, in vim-mode with wrapping on it will move to the next line (and probably try to uppercase the newline which is a no-op)
• gU3l in VIM will span a newline if done on an empty line or near the end of a line, so does vim-mode but it uppercases one more character than VIM does, in a way that's more consistent with the l motion so it doesn't bother me
• yl in VIM on a non-empty line only yanks the character under the cursor, same in vim-mode
• yl in VIM on an empty line yanks the newline and treats it as linewise for p; vim-mode does the same
• y2l in VIM on a non-empty line on the last character only yanks that character, not including the newline, in vim-mode it also yanks the newline which makes more sense to me (and p sees it as a linewise selection because that's what Utils.coffee does, can't blame it)
• y3l in VIM on the last character will yank the character under the cursor, the newline, and the first character of the next line, and the same in vim-mode

None of the above is affected by this PR or should be. Does this make sense?

[maxbrunsfeld]

My general sense is that we should make Atom feel like vim, but not necessarily reimplement all of vim's inconsistencies. The deleting option that you added makes complete sense as a fix for this particular bug, but it bothers me that a specific motion like MoveRight would have knowledge of a particular type of operator (deleting) and take on a different behavior when used with those operators.

Regarding the findings that you shared above, in your opinion, are there any operators whose behavior would badly diverge from their behavior in vim if we were to handle wrapLeftRightMotion uniformly for all operators (by replacing if options.deleting with if @vimState.mode is 'operator-pending')?

[jacekkopecky]

Ah, well put. I very much agree that "we should make Atom feel like vim, but not necessarily reimplement all of vim's inconsistencies." I've switched to checking for operator-pending and all my tests still work (as do all the other tests); the findings above change a bit but not in ways that I mind.

[jacekkopecky]

This is so that things like gUl don't put the cursor after the end of a line.

[maxbrunsfeld]

Great. Thanks for sticking with this through the back and forth.

[jacekkopecky]

Thanks for patiently explaining your thinking there! And for merging while I'm writing this comment, that was cool. 8-)