Undo moves the cursor to the beginning of the line

[fweep]

On this line:

expect(page).|not_to have_link 'Refund'

With the cursor at |, gs will toggle between not_to and to, as expected. If I subsequently undo, however, the cursor moves to the beginning of the line. Is it possible for the cursor to remain where it was before the undo?

[AndrewRadev]

That does seem like it would be very useful. Unfortunately, I can't really figure out a way to implement it :/.

The plugin uses :substitute to make the change, which, on its own, jumps to the beginning of the line. The only reason that the switch itself doesn't do that, is because I manually restore the cursor position. I can't really control the following undo, however.

I tried placing some :keepjumps, :keepmarks and :lockmarks commands, but nothing seems to affect the undo after the command. Even if I manually set a mark at that cursor position with, say, mz, it seems to be removed along with the undo.

I'll leave this issue open in case someone comes up with a drive-by idea how to fix this, but I'm afraid I can't really offer a solution at this time.

[kutsan]

Here is my I'm not proud but it works for me. ™ hack.

diff --git a/plugin/switch.vim b/plugin/switch.vim
index 1140ffe..001fa17 100644
--- a/plugin/switch.vim
+++ b/plugin/switch.vim
@@ -276,7 +276,9 @@ autocmd FileType rust let b:switch_definitions =
 
 command! Switch call s:Switch()
 function! s:Switch()
-  silent call switch#Switch()
+  normal! i.
+  undojoin | normal! x
+  undojoin | silent call switch#Switch()
   silent! call repeat#set(":Switch\<cr>")
 endfunction

[mg979]

This works for me:

function! switch#mapping#Replace(match) dict
  let pattern     = a:match.pattern
  let replacement = self.definitions[pattern]
  let oldsearch   = @/

  if type(replacement) == s:type_dict
    " maintain change delta for adjusting match limits
    let delta = 0

    for [pattern, sub_replacement] in items(replacement)
      let last_column     = col('$')
      let pattern         = s:LimitPattern(pattern, a:match.start, a:match.end + delta)
      let pattern         = escape(pattern, '/')
      let sub_replacement = escape(sub_replacement, '/&')

      silent! foldopen!
      call setline('.', substitute(getline('.'), pattern, sub_replacement, 'ge'))
      " remove pattern from history
      call histdel('search', -1)

      " length of the line may have changed, adjust
      let delta += col('$') - last_column
    endfor
  else
    let pattern     = s:LimitPattern(pattern, a:match.start, a:match.end)
    let pattern     = escape(pattern, '/')
    let replacement = escape(replacement, '/&')

    call setline('.', substitute(getline('.'), pattern, replacement, ''))
    " remove pattern from history
    call histdel('search', -1)
  endif

  let @/ = oldsearch
endfunction

That is, using setline() instead of :s. It's changed in two places, I could only test the second one (not the one inside the for loop).

Edit: I was using flags ge in the second setline call.

[AndrewRadev]

@mg979 This is interesting! The original reason I used :substitute is because I thought that column patterns wouldn't work with a string. The plugin adds patterns like \%>2c to limit the pattern to only the given columns that have been matched. That way, running switch on either of the booleans in foo(true, true) would work for the correct one and wouldn't just substitute the first one on the line.

But it seems like this also works for substitute! Which definitely solves one problem.

The remaining problem that I see is multiline patterns. Running the tests, here's the only one that fails for me:

switch.vim/plugin/switch.vim

Lines 137 to 138 in 49c0eff

	\ '\([{,]\_s*\)\@<=\(\k\+\)\(\s*[},]\)': '\2: \2\3',
	\ '\([{,]\_s*\)\@<=\(\k\+\): \?\2\(\s*[},]\)': '\2\3',

It might not be obvious why, but the problem is the start of the pattern, \([{,]\_s*\)\@<=. This asserts that before the pattern, there's a bracket or a comma, possibly on the previous line. This is used for the following case:

foo = {
  one: one,
  two: two,
}

Switching on the one: one would turn it into just one and another switch would go in the opposite direction. If I were to remove the problematic pattern, the switch from long to short would work fine, but in the other direction, one is just an identifier, and it doesn't seem like a good idea to switch it to one: one without checking the context around it.

With your change, only the single line is being considered, which removes that context. It's just one pattern, but it's a very convenient one for me, used in Rust as well. I could imagine checking that it's maybe alone on a line, ending with a comma, but the comma after the element is optional in coffeescript, and even in Rust, it's optional for the last element. The curly brackets seem like the most reliable way to tell.

[mg979]

The best thing to me would be to leave the multiline patterns alone, already I wasn't sure about the call in the for loop because I had no idea how to test it. Maybe there's a solution for those but it seems much work for little benefit to me. So if it works for single line patterns without drawbacks, I'd use it in that case alone.

Edit: a solution might involve building a single pattern in the for loop and its replacement, then replace it in one go with a substitute() call, but I don't know if it's really feasible, looks complicated to me.