fix redoing visual mode operations with .

[jacekkopecky]

Fixes #737.
Differs from VIM in that if the visual mode selection was characterwise over multiple lines, and the last line of the selection was longer than the corresponding line where we are redoing the operation, VIM would remove the newline and this PR doesn't. See #737 for more information on this.
Any comments will be welcome.

[bronson]

Very very impressive. I've banged on it, can't find anything wrong. As promised, it's in vim-mode-next 0.54.12.

Vim's charwise multi-line behavior feels like a bug. I definitely vote for your current behavior.

[DaanDD]

I've been trying this in vim-mode-next (currently 0.54.15) and repeating commands in line wise visual mode doesn't seem to work correctly. For example, take this file:

foo
bar

Now select both lines with Vj and indent with >

  foo
  bar

When I try to repeat the command with . only the first line gets indented again.

    foo
  bar

[bronson]

Wow. I'm using 0.54.15 too and I don't see that. Both lines indent every time I hit ..

Silly question, you sure vim-mode didn't re-enable itself somehow?

[DaanDD]

I tried it again with a clean install on a different computer and it still behaves as I described.

[jacekkopecky]

I've just reproduced it - the problem shows up as described on a file with only two lines. Try this in a new file:
ifoo<cr>bar<esc>kVj>.
Strangely enough, it works with v instead of V, and it also works with an extra <cr> after bar and an extra k before V. I'll have to look into it. But I hope a fix for this can be another PR if this one can otherwise be merged - it seems to work well enough in situations where I've tried it. If this PR is merged, an issue for the strange behaviour should be opened.

[bronson]

Nice!! Yes, the missing newline on the last line must be the culprit. I'm seeing it too.

I've noticed that files that don't end in newlines trip up some other vim-mode operations too.

[maxbrunsfeld]

You might be able to use some more abstract methods on Point and Range to do this. Could you try something along these lines?

selectCharacters: ->
  lastSelectionExtent = @lastSelection.getExtent()
  for selection in @editor.getSelections()
    {start} = selection.getBufferRange()
    selection.setBufferRange([start, start.traverse(lastSelectionExtent)])
  return

[maxbrunsfeld]

Just a thought: maybe if the motion @wasLinewise, we could use the same code path as for characters, but then call @editor.selectLinesContainingCursors(), or selection.selectLine() on each selection? Not sure if those methods have the right behavior in all cases.

[jacekkopecky]

An attempt just committed, @maxbrunsfeld .
Using Point.traverse doesn't do wrapping so that's extra, but still the code is nicer.
Doing selectLines with selectCharacters, with some linewise stuff around, would work but actually the special-cased code is faster and simpler, IMO.

[maxbrunsfeld]

Got it. I agree.

[maxbrunsfeld]

Could you explain what this if wrap section is for again? I would think that when redoing visual mode operations, we'd always want to select the same number of lines as the original operation.

[jacekkopecky]

Oh, I misinterpreted what VIM was doing. When the last line of the original charwise selection has more characters than are available, VIM eats the newline and I thought it ate more than that. I'll fix it so that vim-mode doesn't eat the newline because I find it illogical behaviour.

[maxbrunsfeld]

I'll fix it so that vim-mode doesn't eat the newline because I find it illogical behaviour.

I agree.

[jacekkopecky]

Done.
(noticed your reply above, edited to remove "let me know if you should disagree with this divergence from VIM, @ maxbrunsfeld.")

[maxbrunsfeld]

✨