chrisbra / matchit

The matchit plugin from Vim
61 stars 9 forks source link

fix visual percent #27

Closed lacygoill closed 3 years ago

lacygoill commented 3 years ago

The % mapping seems broken in visual mode.

To reproduce, run this shell command:

vim -Nu <(cat <<'EOF'
    vim9script
    set rtp^=/path/to/matchit/
    var lines =<< trim END
    aaa
    bbb
    {
    ccc
    ddd
    }
    eee
    fff
    END
    writefile(lines, '/tmp/c.c')
    sil e /tmp/c.c
    feedkeys('2GVj%')
EOF
)

/path/to/matchit/ needs to be set to the actual path of the plugin.

Expected behavior: The block containing the lines ccc and ddd is selected. Actual behavior: The block containing the lines ccc and ddd is not selected.

This is broken because that's not how the builtin % command behaves without the matchit plugin.


The issue disappears if we start the selection from after the end of the block:

vim9script
var lines =<< trim END
aaa
bbb
{
ccc
ddd
}
eee
fff
END
writefile(lines, '/tmp/c.c')
sil e /tmp/c.c
feedkeys('7GVk%')

A similar issue exists when we start the selection while the cursor is right on a brace (not on a line above):

vim9script
var lines =<< trim END
aaa
bbb
{
ccc
ddd
}
eee
fff
END
writefile(lines, '/tmp/c.c')
sil e /tmp/c.c
feedkeys('3GV%%')

Notice that this time, the first % has correctly worked: the block is selected. But the second % has failed: the block should have been de-selected.

And again, the issue disappears when we start the selection from the end of the block rather than from its start.

vim9script
var lines =<< trim END
aaa
bbb
{
ccc
ddd
}
eee
fff
END
writefile(lines, '/tmp/c.c')
sil e /tmp/c.c
feedkeys('6GV%%')

I don't know what the right fix is yet. But I have noticed that using the pseudo-key <Cmd> fixed the issue. Using <Cmd> is better anyway, because it has fewer side-effects: it doesn't trigger the Cmdline* events. Also, it makes the code less verbose: no need of <silent> nor <C-U>.

This bug has always annoyed me, and is one of the reasons why I've used vim-matchup up until now. But I'm trying to move away from big/complex plugins as they are too time-consuming to debug. matchit is much shorter/simpler (hopefully it will stay that way), so I would like this bug to be fixed to use it again.


Note that even though we use <Cmd>, we still need <silent> in 1 mapping. <Cmd> only makes the command silent until the next <CR>; not beyond:

<Cmd>call matchit#Match_wrapper('',1,'v')<CR>:if col("''") != col("$") ...
     ├──────────────────────────────────┘    ├───────────────────────────┘
     │                                       └ this is NOT silent
     └ this is silent

Also, <C-\><C-N> might be replaced with <Esc>, but in a mapping, I try to avoid the latter, because it's too ambiguous. For example:

vim -es -Nu NONE -S <(cat <<'EOF'
    exe "set <F31>=\ed"
    imap <c-b> <esc>ddi
    0pu=['a', 'b', 'c']
    2
    call feedkeys("i\<c-b>", 'x')
    %p
    qa!
EOF
)

This outputs:

a
<F31>dib
c

While I would have expected:

a
c

The issue disappears if we replace <Esc> with <C-\><C-N>:

vim -es -Nu NONE -S <(cat <<'EOF'
    exe "set <F31>=\ed"
    imap <c-b> <C-\><C-N>ddi
    0pu=['a', 'b', 'c']
    2
    call feedkeys("i\<c-b>", 'x')
    %p
    qa!
EOF
)

Bottom line: <Esc> is ambiguous because it can mean "start of a special keycode", or "escape to normal mode". In contrast, <C-\><C-N> is not ambiguous; it can only mean "make sure I'm in normal mode" which is what we really want here.

lacygoill commented 3 years ago

Closing because the fix is not reliable.