Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CmdLineEnter, CmdLineLeave are triggered from mappings #2889

Closed
justinmk opened this issue May 9, 2018 · 8 comments
Closed

CmdLineEnter, CmdLineLeave are triggered from mappings #2889

justinmk opened this issue May 9, 2018 · 8 comments

Comments

@justinmk
Copy link
Contributor

justinmk commented May 9, 2018

Actual behavior:

:au CmdLineEnter * echom localtime()
:nnoremap a :<CR>

Then typing a will trigger the CmdLineEnter handler.

Expected behavior:

Given that CmdLineEnter, CmdLineLeave are mostly for interactive use-cases, it seems like an oversight to also invoke them from mappings. Also based on :help CmdLineEnter.

Could check KeyTyped:

diff --git a/src/ex_getln.c b/src/ex_getln.c
index 0124098da519..7a7e4537bfbd 100644
--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -418,7 +418,8 @@ getcmdline(
 
     /* Trigger CmdlineEnter autocommands. */
     cmdline_type = firstc == NUL ? '-' : firstc;
-    trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINEENTER);
+    if (KeyTyped)
+       trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINEENTER);
 
 #ifdef FEAT_CMDHIST
     init_history();
@@ -1957,7 +1958,8 @@ cmdline_not_changed:
 
 cmdline_changed:
        /* Trigger CmdlineChanged autocommands. */
-       trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINECHANGED);
+       if (KeyTyped)
+           trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINECHANGED);
 
 #ifdef FEAT_SEARCH_EXTRA
        /*
@@ -2158,7 +2160,8 @@ returncmd:
        need_wait_return = FALSE;
     
     /* Trigger CmdlineLeave autocommands. */
-    trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINELEAVE);
+    if (KeyTyped)
+       trigger_cmd_autocmd(cmdline_type, EVENT_CMDLINELEAVE);
 
     State = save_State;
 #ifdef HAVE_INPUT_METHOD
@andymass
Copy link

andymass commented May 9, 2018

What if the user has a map like nnoremap \b :b<space>? Would/should the autocmd be triggered?

@brammool
Copy link
Contributor

brammool commented May 10, 2018 via email

@justinmk
Copy link
Contributor Author

Not sure what the actual problem is we are trying to fix here.

It's a potential performance problem: : in maps will trigger CmdLineEnter for no reason. As I said, my assumption (and :help CmdLineEnter) is that CmdLineEnter is for interactive use-cases, so triggering it in mappings will make many mappings slow which previously were not.

However, I just noticed that InsertCharPre works the same way.

:au InsertCharPre * echom localtime()
:nnoremap a ifoo<esc>
a

I'll leave it up to discussion whether to close this.

@markonm
Copy link

markonm commented May 10, 2018

So perhaps triggering could be postponed until waiting for a character to be typed?

+1. I would like that change.

What would happen if mapping exits command line immediately without waiting for typed character? Would CmdLineEnter event be skipped? What about CmdLineLeave, would it also be skipped? If user calls command line window via q:, CmdwinEnter event would be emitted before CmdlineEnter, that would mess up order of events.

Events can already be postponed until waiting for a character with the help of timers.

function! s:do_domething() abort
  echom localtime()
endfunction
au vimrc CmdlineEnter : let s:do = timer_start(0, {-> s:do_domething()})
au vimrc CmdlineLeave : call timer_stop(s:do)

@justinmk
Copy link
Contributor Author

Events can already be postponed until waiting for a character with the help of timers.

That doesn't solve the problem at all, it just creates more problems for plugin authors. If timers were a solution we wouldn't have needed CmdLineEnter in the first place.

@brammool
Copy link
Contributor

brammool commented May 10, 2018 via email

@mg979
Copy link
Contributor

mg979 commented May 12, 2018

I was trying to use this autocmd to check if the user left the command line without typing anything.
Maybe the autocmd could be activated on demand via set something? So you would do, for example:

set cmdlineau

and somewhere else:

au CmdlineLeave * call func() | set nocmdlineau

and the event wouldn't trigger at all if the setting is off.

justinmk added a commit to justinmk/neovim that referenced this issue Nov 19, 2018
justinmk added a commit to justinmk/neovim that referenced this issue Nov 25, 2018
- develop.txt is for design/guidelines; architecture/concepts should
  live elsewhere (currently src/nvim/README.md)
- move dev-jargon to intro.txt
- replace https://neovim.io/community (deprecated) with
  https://neovim.io/#chat
- <Cmd> avoids CmdlineEnter/Leave
  vim/vim#2889
justinmk added a commit to justinmk/neovim that referenced this issue Nov 28, 2018
- develop.txt is for design/guidelines; architecture/concepts should
  live elsewhere (currently src/nvim/README.md)
- move dev-jargon to intro.txt
- replace https://neovim.io/community (deprecated) with
  https://neovim.io/#chat
- <Cmd> avoids CmdlineEnter/Leave
  vim/vim#2889
justinmk added a commit to justinmk/neovim that referenced this issue Nov 28, 2018
- develop.txt is for design/guidelines; architecture/concepts should
  live elsewhere (currently src/nvim/README.md)
- move dev-jargon to intro.txt
- replace https://neovim.io/community (deprecated) with
  https://neovim.io/#chat
- <Cmd> avoids CmdlineEnter/Leave
  vim/vim#2889
@lacygoill
Copy link

Now that we have the pseudo-key <cmd> (since 8.2.1978), I think the issue can be closed. If we want the Cmdline* events to be fired, we can enter the command-line with :. If we don't want them to be fired, we can enter the command-line with <cmd>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants