[RFC] implement <Cmd> key #4419

bfredl · 2016-03-06T15:14:04Z

I'm not sure we actually want this, but implementation turned out to be surprisingly simple (probably due to the existing work on K_EVENT), so I thought the best way to spawn some discussion was to show a prototype.

The command will execute the same way as an autocommand or remote vim_command.

I've tested serial and nested invocation works as expected.

Pros:

This allows you to do

map <command> <Leader>m echoerr mode()

and the command will actually be executed in visual mode if it is active so mode() shows v. (and visual mode will not be left unless the command leaves it).

Also this works: (compare :help complete())

imap <command> <F5> call complete(col('.'), ['alfa', 'omega'])

Cons:

Doesn't really enable anything that can't already be done with :map <expr>, :imap <c-r>=...<cr> or :map <c-u>...<cr> with some quirks and workarounds. As it adds yet another mapping mode, mental overhead of implementing plugin mappings could just as well increase instead of decrease.

vim might not add it, so it will result in an incompatibility. (I relied on neovim-isms to convince myself the semantics and implementation was sound)

justinmk · 2016-03-06T16:29:42Z

It is convenient, like how autocmd works. How is count handled?

bfredl · 2016-03-06T16:34:03Z

map <command> ,m echomsg v:count1

At least 5,m works as expected

bfredl · 2016-03-06T16:37:24Z

And v:register as well (I tested this first with nvim-miniyank :) but it is irritating you cannot distinguish "",m from ,m ... Maybe reusing v:event.regname would be out-of-scope for v:event ...

bfredl · 2016-03-06T17:37:53Z

Also it always ends operator-pending mode. It works to change
:onoremap in( :<c-u>normal! f(vi(<cr>
to
:omap <command> in( normal! f(vi(
and mode(1) still works.

We could allow a command to optionally not end operator-pending mode like V and ^V but don't really see a usecase.

Shougo · 2016-03-06T23:00:47Z

I think it seems good feature.
If it can also use in Vim, it is better.

bfredl · 2016-03-06T23:44:23Z

Given vim's recent work on channels and messaging it should likely also support "just executing a command" directly at any (mapped) input state (of implementation complexity I have no idea) But nvim's new state mechanism made this completely clear in neovim (sometimes, refactors make code easier to understand :)

romgrk · 2016-03-11T15:05:30Z

Works great, interesting.
Maybe adding it to syntax? (If you could add it to this PR rather than creating a separate one 😄 )

diff --git a/runtime/syntax/vim.vim b/runtime/syntax/vim.vim
index cf51830..215f213 100644
--- a/runtime/syntax/vim.vim
+++ b/runtime/syntax/vim.vim
@@ -307,11 +307,11 @@ syn keyword   vimUnmap        cu[nmap] iu[nmap] lu[nmap] nun[map] ou[nmap] sunm[ap] unm[
 syn keyword    nvimUnmap       tunm[ap] skipwhite nextgroup=vimMapBang,vimMapMod,vimMapLhs
 syn match  vimMapLhs   contained   "\S\+"          contains=vimNotation,vimCtrlChar skipwhite nextgroup=vimMapRhs
 syn match  vimMapBang  contained   "!"         skipwhite nextgroup=vimMapMod,vimMapLhs
-syn match  vimMapMod   contained   "\c<\(buffer\|expr\|\(local\)\=leader\|nowait\|plug\|script\|sid\|unique\|silent\)\+>" contains=vimMapModKey,vimMapModErr skipwhite nextgroup=vimMapMod,vimMapLhs
+syn match  vimMapMod   contained   "\c<\(command\|buffer\|expr\|\(local\)\=leader\|nowait\|plug\|script\|sid\|unique\|silent\)\+>" contains=vimMapModKey,vimMapModErr skipwhite nextgroup=vimMapMod,vimMapLhs
 syn match  vimMapRhs   contained   ".*" contains=vimNotation,vimCtrlChar   skipnl nextgroup=vimMapRhsExtend
 syn match  vimMapRhsExtend contained   "^\s*\\.*$"         contains=vimContinue
 syn case ignore
-syn keyword    vimMapModKey    contained   buffer  expr    leader  localleader nowait  plug    script  sid silent  unique
+syn keyword    vimMapModKey    contained   command buffer  expr    leader  localleader nowait  plug    script  sid silent  unique
 syn case match

bfredl · 2016-03-12T22:35:52Z

I haven't thought of the name really, maybe it should be <cmd> to be consistent with :autocmd. (If I could choose freely it would be :mapcmd modespec lhs cmd where modespec could be any subsequence of nvoict or * but that would probably break the existing expectations too much, and I like the low linecount of this PR :)

ZyX-I · 2016-03-12T23:02:54Z

src/nvim/eval.c

@@ -12339,6 +12339,7 @@ static void get_maparg(typval_T *argvars, typval_T *rettv, int exact)
      dict_add_nr_str(dict, "rhs",     0L, mp->m_orig_str);
      dict_add_nr_str(dict, "noremap", mp->m_noremap ? 1L : 0L, NULL);
      dict_add_nr_str(dict, "expr",    mp->m_expr    ? 1L : 0L, NULL);
+      dict_add_nr_str(dict, "command", mp->m_cmd    ? 1L : 0L, NULL);


This looks shifted.

ZyX-I · 2016-03-12T23:15:27Z

I haven't thought of the name really, maybe it should be <cmd> to be consistent with :autocmd. (If I could choose freely it would be :mapcmd modespec lhs cmd where modespec could be any subsequence of nvoict or * but that would probably break the existing expectations too much, and I like the low linecount of this PR :)

expr is also an abbreviation. I think <cmd> is better.

ZyX-I · 2016-03-12T23:17:03Z

src/nvim/getchar.c

@@ -2035,8 +2037,13 @@ static int vgetorpeek(int advance)
              save_m_str = vim_strsave(mp->m_str);


switch (mp->m_type).

bfredl · 2016-03-12T23:44:08Z

Thanks for the comments. Tests are missing as well...

ZyX-I · 2016-03-12T23:51:11Z

Tests I would write:

exc_exec('map <expr><cmd> lhs rhs')
exc_exec('map <cmd><expr> lhs rhs')
exc_exec('normal fail') with command('map <cmd> fail xxx_failing_command_xxx')
exc_exec('normal fail') with command('map <cmd> fail echomsg "Err"\|call setline(1, 1)'), also check buffer contents.
exc_exec('normal fail') with command('map <cmd> fail throw "Err"\|call setline(1, 2)'), also check buffer contents.
buffer contents after foo with map <cmd> foo call append(1, 1)\|call append(1, 2): should run two commands
buffer contents after foo with map <cmd> foo for i in range(5)\|if i==1\|echoerr "Err"\|endif\|call append(1, i)\|endwhile: without exc_exec (which wraps everything into try|catch) this should (not) end mapping.
buffer contents after 5m with nnoremap <cmd> m call append(1, [v:count, v:register])
buffer contents after "am with nnoremap <cmd> m call append(1, [v:count, v:register])
buffer contents after "a5m with nnoremap <cmd> m call append(1, [v:count, v:register])
buffer contents after 5"am with nnoremap <cmd> m call append(1, [v:count, v:register])
mode() and mode(1) output for normal mapping mode
mode() and mode(1) output for visual mapping mode
mode() and mode(1) output for select mapping mode
mode() and mode(1) output for command mapping mode
mode() and mode(1) output for operator-pending mapping mode
mode() and mode(1) output for insert mapping mode
mode() and mode(1) output for insert mapping mode in replace editor mode
mode() and mode(1) output for insert mapping mode in virtual replace editor mode
mode() and mode(1) output for langmap mapping mode in insert editor mode
whether and how startinsert works when run from <cmd> mapping in normal mode
whether and how startinsert works when run from <cmd> mapping in command mode and where is currently typed command going
whether and how startinsert works when run from <cmd> mapping in operator-pending mode and what active operator does in this case
startinsert does nothing when run from <cmd> mapping in insert mode
whether and how startinsert works when run from <cmd> mapping in visual mode and whether last selection and its type (visualmode()) are saved
whether and how startinsert works when run from <cmd> mapping in select mode and whether last selection and its type (visualmode()) are saved
how throw works when run from <cmd> mapping in command mode and where is currently typed command going
how throw works when run from <cmd> mapping in insert mode
how throw works when run from <cmd> mapping in normal mode
how throw works when run from <cmd> mapping in visual mode and whether last selection and its type (visualmode()) are saved
how throw works when run from <cmd> mapping in select mode and whether last selection and its type (visualmode()) are saved
how throw works when run from <cmd> mapping in operator-pending mode and whether operator does something
what :map cmdmap outputs with map <cmd> cmdmap echo 1
check that <cmd> does (not) work with abbrev command family
if extending visual selection is possible using <cmd> mapping then add tests which first do this and then run startinsert/throw, check selection and mode saved after this
also tests for extending visual selection if possible
also tests for extending visual selection in select mode if possible
how feedkeys() works when run from <cmd> mapping
how feedkeys(, 'x') works when run from <cmd> mapping
how normal works when run from <cmd> mapping

After adding all the tests I guess you would not think this PR is so simple. Expecting tests to find some bugs.

bfredl · 2016-03-13T11:08:03Z

Thanks for the laundry list. 👍 I should clarify: the "simplicity" is because were executing commands in contexts were we already are processing arbitrary commands (autocommands, rpc calls), so we already must have "reasonable" behavior for mode() and modeswitching , but there is some missing cases for :omap which is not a proper state and K_EVENT currently is forbidden. (it should behave like d: w.r.t errors and repeat)

justinmk · 2018-02-26T19:36:36Z

src/nvim/ex_getln.c

+  } else if (s->c == K_COMMAND) {
+    // NB: we use getexmodeline to not invoke K_COMMAND nestedly
+    // but it should probably be "incomplete keyseq" state
+    do_cmdline(NULL, getcmdkeycmd, NULL, 0);


the comment mentions getexmodeline, is it stale?

justinmk · 2018-02-26T19:44:37Z

test/functional/ui/input_spec.lua

+    eq(eval('m'), 't')
+  end)
+
+  it('behaves correctly in visual mode', function()


can just remove "behaves correctly" from these descriptions, it's redundant

justinmk · 2018-02-26T19:46:53Z

test/functional/ui/input_spec.lua

+      feed_command('noremap! '..lhs..' <Cmd>'..rhs..'<cr>')
+    end
+
+    cmdmap('<f3> ', 'let m = mode(1)')


is the space after <f3> significant?

justinmk · 2018-02-26T19:52:01Z

Is there a test for recursive <cmd>? I didn't see one. E.g.

nnoremap <f3> <cmd>foo<cr>
nmap <f4> <cmd><f3><cr>

Is the list proposed by @ZyX-I in #4419 (comment) covered by the tests, or why not?

If anyone still has complaints about the name <cmd>, an alternative would be <cmdline> (and I think if Vim adopted this it would use <cmdline> because "command" also means "normal-mode commands"). Over-thinking it, let's stick with <cmd>.

bfredl · 2018-02-26T20:48:10Z

Recursive invocation is disabled, but I suppose a test should test exactly that.

At a quick glance tests for v:count is obviously missing, also startinsert (which I defined, but forgot to use apperently...). A bit more error handling tests certainly won't hurt. Also existing tests needs a bit more comments, most behaviors regarding visual and operator-pending are tested but its not clear what's going on.

If anyone still has complaints about the name <cmd>, an alternative would be <cmdline>

I would associate cmdline with cmdline mode, this is also what help cmdline shows. Also compare autocmd etc, in general help says {cmd} or {command} when a command is executed without being typed at the cmdline.

bfredl · 2018-03-22T18:02:21Z

@justinmk I think I've covered the entire list now. Also added quite a bit of screen tests, which makes it easier to see what the tests are doing, also because screen tests.

justinmk · 2018-03-23T07:14:41Z

src/nvim/ex_getln.c

+    if (s->c == K_EVENT) {
+      multiqueue_process_events(main_loop.events);
+    } else {
+      do_cmdline(NULL, getcmdkeycmd, NULL, DOCMD_NOWAIT);


Why DOCMD_NOWAIT here, but 0 in the insert_handle_key case?

Because it displays error messages in a way that makes sense in the respective mode.

justinmk · 2018-03-23T07:16:37Z

runtime/doc/map.txt

 If you want the mapping to do any of these let the returned characters do
-that.
+that. Alternatively use a |<Cmd>| mapping which doesn't have these


<expr> is not title-case so I guess <cmd> should not be either.

But <Cmd> is a special key, not a flag (yes, confusing to use <> for both, but that's vim for you), so it should be title-case.

justinmk · 2018-03-23T07:18:52Z

runtime/doc/map.txt

+  noremap <f3> <Cmd>echo mode(1)<cr>
+
+							*E5520*
+The command must be complete and ended with a `<Cr>`. If the command is


Don't need backticks around keycodes in the help. Also it's spelled <CR> everywhere, :helpgrep <Cr> returns no results.

Should also use <CR> in the error messages.

FEATURES: 3cc7ebf #7234 built-in VimL expression parser 6a7c904 #4419 implement <Cmd> key to invoke command in any mode b836328 #7679 'startup: treat stdin as text instead of commands' 58b210e :digraphs : highlight with hl-SpecialKey #2690 7a13611 #8276 'startup: Let `-s -` read from stdin' 1e71978 events: VimSuspend, VimResume #8280 1e7d5e8 #6272 'stdpath()' f96d99a #8247 server: introduce --listen e8c39f7 #8226 insert-mode: interpret unmapped META as ESC 98e7112 msg: do not scroll entire screen (#8088) f72630b #8055 let negative 'writedelay' show all redraws 5d2dd2e win: has("wsl") on Windows Subsystem for Linux #7330 a4f6cec cmdline: CmdlineEnter and CmdlineLeave autocommands (#7422) 207b7ca #6844 channels: support buffered output and bytes sockets/stdio API: f85cbea #7917 API: buffer updates 418abfc #6743 API: list information about all channels/jobs. 36b2e3f #8375 API: nvim_get_commands 273d2cd #8329 API: Make nvim_set_option() update `:verbose set …` 8d40b36 #8371 API: more reliable/descriptive VimL errors ebb1acb #8353 API: nvim_call_dict_function 9f994bb #8004 API: nvim_list_uis 3405704 #7520 API/UI: forward option updates to UIs 911b1e4 #7821 API: improve nvim_command_output WINDOWS OS: 9cefd83 #8084, #8516 build/win: support MSVC ee4e1fd win: Fix reading content from stdin (#8267) TUI: ffb8904 #8309 TUI: add support for mouse release events in urxvt 8d5a46e #8081 TUI: implement "standout" attribute 6071637 TUI: support TERM=konsole-256color 67848c0 #7653 TUI: report TUI info with -V3 ('verbose' >= 3) 3d0ee17 TUI/rxvt: enable focus-reporting d109f56 #7640 TUI: 'term' option: reflect effective terminal behavior FIXES: ed6a113 #8273 'job-control: avoid kill-timer race' 4e02f1a #8107 'jobs: separate process-group' 451c48a terminal: flush vterm output buffer on pty output #8486 5d6732f :checkhealth fixes #8335 53f11dc #8218 'Fix errors reported by PVS' d05712f inccommand: pause :terminal redraws (#8307) 51af911 inccommand: do not execute trailing commands #8256 84359a4 terminal: resize to the max dimensions (#8249) d49c1dd #8228 Make vim_fgets() return the same values as in Vim 60e96a4 screen: winhl=Normal:Background should not override syntax (#8093) 0c59ac1 #5908 'shada: Also save numbered marks' ba87a2c cscope: ignore EINTR while reading the prompt (#8079) b1412dc #7971 ':terminal Enter/Leave should not increment jumplist' 3a5721e TUI: libtermkey: force CSI driver for mouse input #7948 6ff13d7 #7720 TUI: faster startup 1c6e956 #7862 TUI: fix resize-related segfaults a58c909 #7676 TUI: always hide cursor when flushing, never flush buffers during unibilium output 303e1df #7624 TUI: disable BCE almost always 249bdb0 #7761 mark: Make sure that jumplist item will not have zero lnum 6f41ce0 #7704 macOS: Set $LANG based on the system locale a043899 #7633 'Retry fgets on EINTR' CHANGES: ad60927 #8304 default to 'nofsync' f3f1970 #8035 defaults: 'fillchars' a6052c7 #7984 defaults: sidescroll=1 b69fa86 #7888 defaults: enable cscopeverbose 7c4bb23 defaults: do :filetype stuff unless explicitly "off" 2aa308c #5658 'Apply :lmap in macros' 8ce6393 terminal: Leave 'relativenumber' alone (#8360) e46534b #4486 refactor: Remove maxmem, maxmemtot options 131aad9 win: defaults: 'shellcmdflag', 'shellxquote' #7343 c57d315 #8031 jobwait(): return -2 on interrupt also with timeout 6452831 clipboard: macOS: fallback to tmux if pbcopy is broken #7940 300d365 #7919 Make 'langnoremap' apply directly after a map ada1956 #7880 'lua/executor: Remove lightuserdata' INTERNAL: de0a954 #7806 internal statistics for list impl dee78a4 #7708 rewrite internal list impl

marvim added the RFC label Mar 6, 2016

ZyX-I reviewed Mar 12, 2016
View reviewed changes

bfredl force-pushed the cmdmap branch 4 times, most recently from c60e4f2 to 6aa13ea Compare March 13, 2016 17:20

bfredl force-pushed the cmdmap branch from e2ee4b9 to 3af55e9 Compare February 26, 2018 17:52

justinmk reviewed Feb 26, 2018

View reviewed changes

bfredl force-pushed the cmdmap branch 4 times, most recently from 85a7383 to 29e4dc8 Compare March 4, 2018 18:28

bfredl force-pushed the cmdmap branch 4 times, most recently from e967d20 to 81fbd1e Compare March 17, 2018 21:30

bfredl force-pushed the cmdmap branch 3 times, most recently from 2530721 to ae404da Compare March 22, 2018 17:56

justinmk reviewed Mar 23, 2018

View reviewed changes

getchar: implement <Cmd> key to invoke command in any mode

d407a48

bfredl force-pushed the cmdmap branch from ae404da to d407a48 Compare March 23, 2018 13:04

justinmk merged commit 6a7c904 into neovim:master Mar 24, 2018

justinmk removed the RFC label Mar 24, 2018

frangio mentioned this pull request May 26, 2018

<Cmd> is not listed in vim-differences #8442

Closed

blueyed mentioned this pull request Aug 7, 2019

Support for "<Cmd>" in mappings (Neovim patch) vim/vim#4784

Closed

justinmk added the has:vim-patch issue is fixed in vim and patch needs to be ported label Aug 8, 2019

shadmansaleh mentioned this pull request Dec 18, 2021

feat(api): add support for lua function & description in keymap #16594

Merged

		@@ -2035,8 +2037,13 @@ static int vgetorpeek(int advance)
		save_m_str = vim_strsave(mp->m_str);

Uh oh!

[RFC] implement <Cmd> key #4419

[RFC] implement <Cmd> key #4419

Uh oh!

Conversation

bfredl commented Mar 6, 2016

Pros:

Cons:

Uh oh!

justinmk commented Mar 6, 2016

Uh oh!

bfredl commented Mar 6, 2016

Uh oh!

bfredl commented Mar 6, 2016

Uh oh!

bfredl commented Mar 6, 2016

Uh oh!

Shougo commented Mar 6, 2016

Uh oh!

bfredl commented Mar 6, 2016

Uh oh!

romgrk commented Mar 11, 2016

Uh oh!

bfredl commented Mar 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZyX-I commented Mar 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl commented Mar 12, 2016

Uh oh!

ZyX-I commented Mar 12, 2016

Uh oh!

bfredl commented Mar 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bfredl commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bfredl commented Mar 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

justinmk commented Feb 26, 2018 •

edited

Loading

bfredl commented Feb 26, 2018 •

edited

Loading