search

chrisbra / matchit

The matchit plugin from Vim
61 stars 9 forks source link

Matchit disables v_% in some situations #29

Open lkintact opened 3 years ago

lkintact commented 3 years ago

To Reproduce:

  1. Put the repository contents into $VIMRUNTIME\pack\dist\opt\matchit-master
  2. Run gvim --clean test.txt (the file: test.txt)
  3. Enable line numbers with :set number
  4. Execute :packadd matchit-master
  5. Try putting the cursor on all of the six opening/closing brackets/braces/parentheses, switching to Visual mode with v and hitting % twice. Out of the six chars only ] in line 11 gives the expected result: after the first % hit the selection gets extended to the corresponding char, [, and after the second one shortens back.

For the rest of the chars: [ in line 1, { in line 3 and ) in line 7: the selection doesn't shorten back after the second % hit. ( in line 5 and } in line 9: the selection doesn't get extended after the first % hit.

Environment:

lacygoill commented 3 years ago

Yes, this bug has always annoyed me. I really need to find a fix. I thought I found a simple one with this PR, but I was wrong. I'll try to find something better later.

lacygoill commented 3 years ago

OK, I have a patch which seems to fix the issue:

diff --git a/autoload/matchit.vim b/autoload/matchit.vim
index ef89f17..a0b7a7e 100644
--- a/autoload/matchit.vim
+++ b/autoload/matchit.vim
@@ -232,7 +232,10 @@ function matchit#Match_wrapper(word, forward, mode) range
   if exists('eolmark') && eolmark
     call setpos("''", [0, line('.'), col('$'), 0]) " set mark on the eol
   endif
-  return s:CleanUp(restore_options, a:mode, startpos, mid.'\|'.fin)
+  call s:CleanUp(restore_options, a:mode, startpos, mid.'\|'.fin)
+  if [a:word, a:mode] ==# ['', 'v']
+    normal! m'gv``
+  endif
 endfun

 " Restore options and do some special handling for Operator-pending mode.
diff --git a/plugin/matchit.vim b/plugin/matchit.vim
index 51ba3a7..03a59b0 100644
--- a/plugin/matchit.vim
+++ b/plugin/matchit.vim
@@ -38,7 +38,7 @@
 "   work but extend it.

 " Allow user to prevent loading and prevent duplicate loading.
-if exists("g:loaded_matchit") || &cp
+if exists('g:loaded_matchit') || &cp
   finish
 endif
 let g:loaded_matchit = 1
@@ -46,40 +46,39 @@ let g:loaded_matchit = 1
 let s:save_cpo = &cpo
 set cpo&vim

-nnoremap <silent> <Plug>(MatchitNormalForward)     :<C-U>call matchit#Match_wrapper('',1,'n')<CR>
-nnoremap <silent> <Plug>(MatchitNormalBackward)    :<C-U>call matchit#Match_wrapper('',0,'n')<CR>
-xnoremap <silent> <Plug>(MatchitVisualForward)     :<C-U>call matchit#Match_wrapper('',1,'v')<CR>
-      \:if col("''") != col("$") \| exe ":normal! m'" \| endif<cr>gv``
-xnoremap <silent> <Plug>(MatchitVisualBackward)    :<C-U>call matchit#Match_wrapper('',0,'v')<CR>m'gv``
-onoremap <silent> <Plug>(MatchitOperationForward)  :<C-U>call matchit#Match_wrapper('',1,'o')<CR>
-onoremap <silent> <Plug>(MatchitOperationBackward) :<C-U>call matchit#Match_wrapper('',0,'o')<CR>
+nnoremap <Plug>(MatchitNormalForward)     <cmd>call matchit#Match_wrapper('', v:true, 'n')<CR>
+nnoremap <Plug>(MatchitNormalBackward)    <cmd>call matchit#Match_wrapper('', v:false, 'n')<CR>
+xnoremap <Plug>(MatchitVisualForward)     <c-\><c-n><cmd>call matchit#Match_wrapper('', v:true, 'v')<CR>
+xnoremap <Plug>(MatchitVisualBackward)    <c-\><c-n><cmd>call matchit#Match_wrapper('', v:false, 'v')<CR>
+onoremap <Plug>(MatchitOperationForward)  <cmd>call matchit#Match_wrapper('', v:true, 'o')<CR>
+onoremap <Plug>(MatchitOperationBackward) <cmd>call matchit#Match_wrapper('', v:false, 'o')<CR>

 " Analogues of [{ and ]} using matching patterns:
-nnoremap <silent> <Plug>(MatchitNormalMultiBackward)    :<C-U>call matchit#MultiMatch("bW", "n")<CR>
-nnoremap <silent> <Plug>(MatchitNormalMultiForward)     :<C-U>call matchit#MultiMatch("W",  "n")<CR>
-xnoremap <silent> <Plug>(MatchitVisualMultiBackward)    :<C-U>call matchit#MultiMatch("bW", "n")<CR>m'gv``
-xnoremap <silent> <Plug>(MatchitVisualMultiForward)     :<C-U>call matchit#MultiMatch("W",  "n")<CR>m'gv``
-onoremap <silent> <Plug>(MatchitOperationMultiBackward) :<C-U>call matchit#MultiMatch("bW", "o")<CR>
-onoremap <silent> <Plug>(MatchitOperationMultiForward)  :<C-U>call matchit#MultiMatch("W",  "o")<CR>
+nnoremap <Plug>(MatchitNormalMultiBackward)    <cmd>call matchit#MultiMatch('bW', 'n')<CR>
+nnoremap <Plug>(MatchitNormalMultiForward)     <cmd>call matchit#MultiMatch('W',  'n')<CR>
+xnoremap <Plug>(MatchitVisualMultiBackward)    <c-\><c-n><cmd>call matchit#MultiMatch('bW', 'n')<CR>m'gv``
+xnoremap <Plug>(MatchitVisualMultiForward)     <c-\><c-n><cmd>call matchit#MultiMatch('W',  'n')<CR>m'gv``
+onoremap <Plug>(MatchitOperationMultiBackward) <cmd>call matchit#MultiMatch('bW', 'o')<CR>
+onoremap <Plug>(MatchitOperationMultiForward)  <cmd>call matchit#MultiMatch('W',  'o')<CR>

 " text object:
-xmap <silent> <Plug>(MatchitVisualTextObject) <Plug>(MatchitVisualMultiBackward)o<Plug>(MatchitVisualMultiForward)
+xmap <Plug>(MatchitVisualTextObject) <Plug>(MatchitVisualMultiBackward)o<Plug>(MatchitVisualMultiForward)

-if !exists("g:no_plugin_maps")
-  nmap <silent> %  <Plug>(MatchitNormalForward)
-  nmap <silent> g% <Plug>(MatchitNormalBackward)
-  xmap <silent> %  <Plug>(MatchitVisualForward)
-  xmap <silent> g% <Plug>(MatchitVisualBackward)
-  omap <silent> %  <Plug>(MatchitOperationForward)
-  omap <silent> g% <Plug>(MatchitOperationBackward)
+if !exists('g:no_plugin_maps')
+  nmap %  <Plug>(MatchitNormalForward)
+  nmap g% <Plug>(MatchitNormalBackward)
+  xmap %  <Plug>(MatchitVisualForward)
+  xmap g% <Plug>(MatchitVisualBackward)
+  omap %  <Plug>(MatchitOperationForward)
+  omap g% <Plug>(MatchitOperationBackward)

   " Analogues of [{ and ]} using matching patterns:
-  nmap <silent> [% <Plug>(MatchitNormalMultiBackward)
-  nmap <silent> ]% <Plug>(MatchitNormalMultiForward)
-  xmap <silent> [% <Plug>(MatchitVisualMultiBackward)
-  xmap <silent> ]% <Plug>(MatchitVisualMultiForward)
-  omap <silent> [% <Plug>(MatchitOperationMultiBackward)
-  omap <silent> ]% <Plug>(MatchitOperationMultiForward)
+  nmap [% <Plug>(MatchitNormalMultiBackward)
+  nmap ]% <Plug>(MatchitNormalMultiForward)
+  xmap [% <Plug>(MatchitVisualMultiBackward)
+  xmap ]% <Plug>(MatchitVisualMultiForward)
+  omap [% <Plug>(MatchitOperationMultiBackward)
+  omap ]% <Plug>(MatchitOperationMultiForward)

   " Text object
   xmap a% <Plug>(MatchitVisualTextObject)
@@ -88,8 +87,8 @@ endif
 " Call this function to turn on debugging information.  Every time the main
 " script is run, buffer variables will be saved.  These can be used directly
 " or viewed using the menu items below.
-if !exists(":MatchDebug")
-  command! -nargs=0 MatchDebug call matchit#Match_debug()
+if exists(':MatchDebug') != 2
+  command -nargs=0 MatchDebug call matchit#Match_debug()
 endif

 let &cpo = s:save_cpo

Not sure it's entirely correct because I don't know what was the purpose of this test:

\:if col("''") != col("$") \| exe ":normal! m'" \| endif<cr>gv``
  ^----------------------^

For the code to work as expected, the ' mark needs to be set where a match has been found by searchpairpos(). This is not the case when the match is at the end of the line. I don't know for what special case the original author of the plugin thought this condition was necessary.

We also need our visual mapping to escape before invoking matchit#Match_wrapper() so that startpos is correctly set:

let startpos = [line("."), col(".")]
lacygoill commented 3 years ago

We also need our visual mapping to escape before invoking matchit#Match_wrapper() so that startpos is correctly set:

In a future refactoring, it might not be necessary to escape; nor should it be necessary to manually pass the mode to the function. Now that we have <cmd>, we might be able to stay in whatever mode we are.

lacygoill commented 3 years ago

Sorry, I tried to fix too many things at once. I guess it's better to stay focused on one thing at a time. So, here is a shorter patch which only deals with the current issue:

diff --git a/plugin/matchit.vim b/plugin/matchit.vim
index 51ba3a7..6b4ade7 100644
--- a/plugin/matchit.vim
+++ b/plugin/matchit.vim
@@ -48,8 +48,7 @@ set cpo&vim

 nnoremap <silent> <Plug>(MatchitNormalForward)     :<C-U>call matchit#Match_wrapper('',1,'n')<CR>
 nnoremap <silent> <Plug>(MatchitNormalBackward)    :<C-U>call matchit#Match_wrapper('',0,'n')<CR>
-xnoremap <silent> <Plug>(MatchitVisualForward)     :<C-U>call matchit#Match_wrapper('',1,'v')<CR>
-      \:if col("''") != col("$") \| exe ":normal! m'" \| endif<cr>gv``
+xnoremap <silent> <Plug>(MatchitVisualForward)     <C-\><C-N>:call matchit#Match_wrapper('',1,'v')<CR>m'gv``
 xnoremap <silent> <Plug>(MatchitVisualBackward)    :<C-U>call matchit#Match_wrapper('',0,'v')<CR>m'gv``
 onoremap <silent> <Plug>(MatchitOperationForward)  :<C-U>call matchit#Match_wrapper('',1,'o')<CR>
 onoremap <silent> <Plug>(MatchitOperationBackward) :<C-U>call matchit#Match_wrapper('',0,'o')<CR>
lkintact commented 3 years ago

@lacygoill Thank you, just tried the patch from your post above, fixes the issue for me.

lkintact commented 3 years ago

Thank you, just tried the patch from your post above, fixes the issue for me.

Unfortunately, as I just discovered, it also reanimates bug #24.