[vim/vim] Add autocmd_get() and autocmd_set() functions to get and set autocmds (PR #10291)
Yegappan Lakshmanan
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/10291
Commit Summary
- fe350e3 Add autocmd_get() and autocmd_set() functions to get and set autocmds
File Changes
(8 files)
- M runtime/doc/autocmd.txt (8)
- M runtime/doc/builtin.txt (112)
- M runtime/doc/usr_41.txt (5)
- M src/autocmd.c (316)
- M src/evalfunc.c (5)
- M src/proto/autocmd.pro (2)
- M src/testdir/test_autocmd.vim (231)
- M src/testdir/test_vim9_builtin.vim (9)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
codecov[bot]
Codecov Report
Merging #10291 (fe350e3) into master (87f7410) will decrease coverage by
0.01%.
The diff coverage is72.02%.
@@ Coverage Diff @@ ## master #10291 +/- ## ========================================== - Coverage 82.44% 82.43% -0.02% ========================================== Files 148 148 Lines 171691 171858 +167 Branches 38825 38877 +52 ========================================== + Hits 141550 141666 +116 - Misses 17523 17548 +25 - Partials 12618 12644 +26
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 82.43% <72.02%> (-0.02%) |
⬇️ |
| linux | 82.43% <72.02%> (-0.02%) |
⬇️ |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/evalfunc.c | 89.44% <ø> (ø) |
|
| src/autocmd.c | 83.10% <72.02%> (-1.75%) |
⬇️ |
| src/if_xcmdsrv.c | 76.07% <0.00%> (-0.90%) |
⬇️ |
| src/profiler.c | 83.02% <0.00%> (-0.70%) |
⬇️ |
| src/term.c | 73.53% <0.00%> (-0.11%) |
⬇️ |
| src/gui_gtk_x11.c | 51.97% <0.00%> (-0.10%) |
⬇️ |
| src/getchar.c | 83.52% <0.00%> (-0.07%) |
⬇️ |
| src/gui.c | 71.18% <0.00%> (+0.19%) |
⬆️ |
| src/netbeans.c | 71.64% <0.00%> (+0.29%) |
⬆️ |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 87f7410...fe350e3. Read the comment docs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
bfrg
Is there a way to set a callback function directly instead of a command?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
Is there a way to set a callback function directly instead of a command, just like we can set callbacks for jobs,
timers or popups?
Currently you can use only an Ex command to handle an autocmd event.
It is possible to modify this to support a callback function.
- Yegappan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
What does autocmd_set() do when there already is an autocommand with the same group? When using ":autocmd" it would get added, which probably works best. Having a "delete" flag for autocmd_set() seems strange. Perhaps it's better to use autocmd_add() and autocmd_delete().
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
What does autocmd_set() do when there already is an autocommand with the same group? When using ":autocmd" it would get added, which probably works best.
Having a "delete" flag for autocmd_set() seems strange. Perhaps it's better to use autocmd_add() and autocmd_delete().
vim-dev ML
On Tue, Apr 26, 2022 at 2:00 PM Bram Moolenaar ***@***.***>
wrote:
> What does autocmd_set() do when there already is an autocommand with the
> same group? When using ":autocmd" it would get added, which probably works
> best.
>
Yes. The behavior of autocmd_set() is the same as the ":autocmd" command.
If an autocommand already exists for a given group, event and pattern,
then the new command is added to the list.
> Having a "delete" flag for autocmd_set() seems strange. Perhaps it's
> better to use autocmd_add() and autocmd_delete().
>
>
>
I will add a separate function for deleting autocmds.
Regards,
Yegappan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
bfrg
@yegappan Currently it is not documented how autocmd_set() behaves when both bufnr and pattern are specified.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
bfrg
I think this PR also addresses #6339.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
@yegappan pushed 1 commit.
- d8e5d91 Add autocmd_add() and autocmd_delete()
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan Currently it is not documented how
autocmd_set()behaves when bothbufnrandpatternare specified.
I have updated the help text to describe how buffer-local autocmds can be added and deleted.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
bfrg
Personally I preferred the previous version where a buffer number could be specified directly without doing any string concatenation like '<buffer=' .. bufnr(mybuf) .. '>'.
The documentation just wasn't clear enough whether the bufnr or pattern option had precedence when both were specified.
I think it would be a lot more readable if we could set the buffer number directly, especially in vim9script.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
Personally I preferred the previous version where a buffer number could be specified directly without doing
any string concatenation like'<buffer=' .. bufnr(mybuf) .. '>'.The documentation just wasn't clear enough whether the
bufnrorpatternoption had precedence when both
were specified.I think it would be a lot more readable if we could set the buffer number directly, especially in
vim9script.
I have updated the PR to support specifying the buffer number and updated the doc.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
I'll leave this open for comments a couple of days.
Nit: "automatic commands" isn't used in the help, only "autocmds" and "autocommands". It's a weird name anyway, but we're stuck with it.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
bfrg
I think the documentation in :h autocmd_delete() is outdated.
In the examples of autocmd_delete() the function is called with the entry 'delete' which doesn't exist.
And what is the purpose of nested or once in autocmd_delete()? Aren't these options only useful for autocmd_add()?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
bfrg
The documentation uses {cmds} or {cmd} to refer to the arguments in autocmd_delete({cmds}) and autocmd_add({cmds}). I think this is a bit confusing since there's already an entry called cmd.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
@yegappan can you respond to the comments by @bfrg ?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
I think the documentation in
:h autocmd_delete()is outdated.In the examples of
autocmd_delete()the function is called with the entry'delete'which doesn't exist.
And what is the purpose of
nestedoronceinautocmd_delete()? Aren't these options only useful forautocmd_add()?
vim-dev ML
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
lacygoill
One common pitfall when adding an autocmd is to forget to wrap it inside a self-clearing augroup.
Bad:
autocmd CursorHold * echomsg 'do something'
Good:
augroup MyGroup autocmd! autocmd CursorHold * echomsg 'do something' augroup END
The first snippet is bad, because if for some reason the script is sourced multiple times, there will be a duplicate autocmd:
vim9script var code = 'autocmd CursorHold * echomsg "do something"' [code]->writefile('/tmp/script.vim') source /tmp/script.vim source /tmp/script.vim autocmd CursorHold
--- Autocommands ---
CursorHold
* echomsg "do something"
echomsg "do something"
Depending on the cost of the executed command and/or the frequency of the event, this might slow Vim down noticeably, or even cause errors.
Now, here is how we can rewrite the good snippet with the new functions:
vim9script if exists('#MyGroup') autocmd_delete([{group: 'MyGroup', event: '*'}]) endif autocmd_add([{group: 'MyGroup', event: 'CursorHold', pattern: '*', cmd: "echomsg 'do something'"}])
The whole if exists() block is cumbersome to read/write every single time we need to add an autocmd. Would it make sense for the dictionaries expected by autocmd_add() to support an extra unique boolean option (true by default)?
When unique is true, the autocmd would not be added if it's inside an augroup which already includes the same autocmd. And since unique would be true by default, we could completely omit the previous if exists() block.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
> One common pitfall when adding an autocmd is to forget to wrap it inside a self-clearing augroup.
>
> Bad:
> ```
> Good:
> autocmd!
> autocmd CursorHold * echomsg 'do something'
> augroup END
> ```
> The first snippet is bad, because if for some reason the script is sourced multiple times, there will be a duplicate autocmd:
> var code = 'autocmd CursorHold * echomsg "do something"'
> [code]->writefile('/tmp/script.vim')
> source /tmp/script.vim
> source /tmp/script.vim
> autocmd CursorHold
> ```
> --- Autocommands ---
> CursorHold
> * echomsg "do something"
> echomsg "do something"
>
>
> Depending on the cost of the executed command and/or the frequency of the event, this might slow Vim down noticeably, or even cause errors.
>
> Now, here is how we can rewrite the good snippet with the new functions:
> if exists('#MyGroup')
> autocmd_delete([{group: 'MyGroup', event: '*'}])
> endif
> autocmd_add([{group: 'MyGroup', event: 'CursorHold', pattern: '*', cmd: "echomsg 'do something'"}])
> ```
> The whole `if exists()` block is cumbersome to read/write every single
> time we need to add an autocmd.
> Would it make sense for the dictionaries expected by `autocmd_add()`
> When `unique` is true, the autocmd would not be added if it's inside
> an augroup which already includes the same autocmd. And since
> `unique` would be `true` by default, we could completely omit the
> previous `if exists()` block.
this group and event already exists, replace it with this one. This
works better when someone edits the .vimrc to change the command,
sourcing the file again should use the new one, not keep the old one.
autocmd_add([{replace: true, group: 'MyGroup', event: 'CursorHold', pattern: '*', cmd: "echomsg 'do something'"}])
--
There is a fine line between courage and foolishness.
Unfortunately, it's not a fence.
/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
Nick Jensen
@brammool's comment from the mailing list, which hasn't come through to github:
The whole
if exists()block is cumbersome to read/write every single time we need to add an autocmd.
Using "silent!" helps though. Here you don't care about errors.
Would it make sense for the dictionaries expected by
autocmd_add()to support an extrauniqueboolean option (trueby default)?
When
uniqueis true, the autocmd would not be added if it's inside an augroup which already includes the same autocmd. And sinceuniquewould betrueby default, we could completely omit the previousif exists()block.
It's probably better to add a "replace" argument: If an autocommand for this group and event already exists, replace it with this one. This works better when someone edits the .vimrc to change the command, sourcing the file again should use the new one, not keep the old one.
autocmd_add([{replace: true, group: 'MyGroup', event: 'CursorHold', pattern: '*', cmd: "echomsg 'do something'"}])
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Nick Jensen
It's probably better to add a "replace" argument: If an autocommand for this group and event already exists, replace it with this one.
I think either unique or replace are both fine (although unique is more descriptive in my mind), as long as the default value is true so we normally don't need to use it, or even know about it! It is very rare that an autocmd should not replace an existing one with the same group and event.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
It's probably better to add a "replace" argument: If an autocommand for this group and event already exists, replace it with this one.
I think either
uniqueorreplaceare both fine (althoughuniqueis more descriptive in my mind), as long as the default value istrueso we normally don't need to use it, or even know about it! It is very rare that an autocmd should not replace an existing one with the same group and event.
vim-dev ML
On Mon, May 23, 2022 at 1:40 PM Nick Jensen ***@***.***>
wrote:
> It's probably better to add a "replace" argument: If an autocommand for
> this group and event already exists, replace it with this one.
>
> I think either unique or replace are both fine (although unique is more
> descriptive in my mind), as long as the default value is true so we
> normally don't need to use it, or even know about it! It is very rare that
> event.
>
>
> If the 'replace' item is specified, then autocmd_add() will remove all the
commands
associated with an autocmd event in the group and add the new command. If a
group is not specified, then the default group will be used. This is same
as running
the following command:
autocmd! {group} {event} {pat} {cmd}
If we make 'replace' the default, then a call to autocmd_add() in a .vimrc
file will
remove commands added by a plugin (in the default group) for that event.
A user may not expect this. So we should not make 'replace' as the default.
- Yegappan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
If a group is not specified, then the default group will be used.
[...]
If we make 'replace' the default, then a call to autocmd_add() in a .vimrc
file will remove commands added by a plugin (in the default group) for that event.
A user may not expect this.
IMO, an autocmd should always be added in a named augroup; not in the default one. Unless we're quickly testing some feature.
I was not clear enough in the previous post, sorry. My idea was for unique/replace to be considered only when the autocmd was placed in a named augroup. Because AFAIK the default augroup can't be cleared specifically (right?). So, it wouldn't make sense for autocmd_add() to provide a feature which was not also provided by :autocmd.
And if replace is ignored for the default augroup, then there would be no issue (right?).
The only counter-argument I can find is that it would make replace more complicated to understand. But IMO, doing what the user expects is more important. And to quote :help local-options:
Unfortunately, doing what the user expects is a bit complicated...
Anyway, the mere introduction of replace would already be a big improvement. Making it default for named augroups would be even better, but not that important.
Thank you very much for working on these functions, they're very useful.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
If a group is not specified, then the default group will be used.
[...]
If we make 'replace' the default, then a call to autocmd_add() in a .vimrc
file will remove commands added by a plugin (in the default group) for that event.
A user may not expect this.IMO, an autocmd should always be added in a named augroup; not in the default one. Unless we're quickly testing some feature.
I was not clear enough in the previous post, sorry. My idea was for
unique/replaceto be considered only when the autocmd was placed in a named augroup. Because AFAIK the default augroup can't be cleared specifically (right?). So, it wouldn't make sense forautocmd_add()to provide a feature which was not also provided by:autocmd.
And if
replaceis ignored for the default augroup, then there would be no issue (right?).
autocmd CursorHold * echo "cmd2"
autocmd! CursorHold * echo "cmd3"
vim-dev ML
>
> Unfortunately, doing what the user expects is a bit complicated...
>
> Anyway, the mere introduction of replace would already be a big
> but not that important.
>
> Thank you very much for working on these functions, they're very useful.
>
>
>
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
Yegappan wrote:
> On Mon, May 23, 2022 at 1:40 PM Nick Jensen <vim-dev...@256bit.org>
> wrote:
>
> > It's probably better to add a "replace" argument: If an autocommand for
> > this group and event already exists, replace it with this one.
> >
> > I think either unique or replace are both fine (although unique is more
> > descriptive in my mind), as long as the default value is true so we
> > normally don't need to use it, or even know about it! It is very rare that
> >
> >
> > If the 'replace' item is specified, then autocmd_add() will remove all the
> commands
> associated with an autocmd event in the group and add the new command. If a
> group is not specified, then the default group will be used. This is same
> as running
> the following command:
>
> autocmd! {group} {event} {pat} {cmd}
>
> If we make 'replace' the default, then a call to autocmd_add() in a .vimrc
> file will
> remove commands added by a plugin (in the default group) for that event.
> A user may not expect this. So we should not make 'replace' as the default.
be expected. For example:
autocmd_add( ... some group and event ..., command one)
autocmd_add( ... some group and event ..., command two)
Here the user adds two commands for the same group and event. Both
should be added. Having the second one remove the first one is very
unexpected. Note that the two commands could be far apart.
--
Facepalm statement #4: "3000 year old graves? That's not possible, it's only
2014!"
vim-dev ML
Yegappan wrote:
> On Mon, May 23, 2022 at 1:40 PM Nick Jensen ***@***.***>
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
lacygoill
For example:
autocmd_add( ... some group and event ..., command one) autocmd_add( ... some group and event ..., command two)Here the user adds two commands for the same group and event.
I was thinking that the comparison would not be based only on the group and event, but also on the actual command. But maybe that would have been too tricky to implement. Anyway, I'm fine with replace being false by default. I've started using it here, and it seems to work as expected.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
lacygoill
There is another issue: it's not easy to use autocmd_add() to install an autocmd listening to several events.
vim9script autocmd_add([{group: 'SomeGroup', event: 'BufEnter,WinEnter,VimEnter', pattern: '*', cmd: 'echomsg "do something"'}]) autocmd
--- Autocommands ---
SomeGroup BufEnter
* echomsg "do something"
Notice that autocmd_add() has ignored everything after the comma (i.e. WinEnter and VimEnter).
It would be useful for the event key to either accept a comma-separated list of events, or better yet a list of strings. Trying the latter currently gives E730:
vim9script autocmd_add([{group: 'SomeGroup', event: ['BufEnter', 'WinEnter', 'VimEnter'], pattern: '*', cmd: 'echomsg "do something"'}])
E730: Using a List as a String
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
lacygoill
And I guess the same limitation applies to a comma-separated list of patterns.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
> There is another issue: it's not easy to use `autocmd_add()` to install an autocmd listening to several events.
> vim9script
> autocmd_add([{group: 'SomeGroup', event: 'BufEnter,WinEnter,VimEnter', pattern: '*', cmd: 'echomsg "do something"'}])
> autocmd
> ```
> --- Autocommands ---
> SomeGroup BufEnter
> * echomsg "do something"
>
> Notice that `autocmd_add()` has ignored everything after the comma
This should at least give an error.
> It would be useful for the `event` key to either accept a
> comma-separated list of events, or better yet a list of strings.
> ```vim
> vim9script
> autocmd_add([{group: 'SomeGroup', event: ['BufEnter', 'WinEnter', 'VimEnter'], pattern: '*', cmd: 'echomsg "do something"'}])
> ```
> E730: Using a List as a String
event: 'BufEnter,WinEnter,VimEnter',
event: ['BufEnter', 'WinEnter', 'VimEnter']
Is there a reason to use a list?
--
You cannot propel yourself forward by patting yourself on the back.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
lacygoill
Is there a reason to use a list?
I'm not sure. While it's more verbose, it seems that an explicit list data structure is a better fit for a list of events/patterns than a string, where the list is implicit (we have to mentally split the string when reading the code).
It might also help in some corner cases. For example, in the middle of a comma-separated list of patterns, if one of them starts with a comma, then it needs to be escaped to not be confused with a delimiter:
autocmd WinEnter *.a,\,*.b echomsg 'do something'
^
Here the pattern is meant to match a file whose extension is .a, or a file starting with a comma and whose extension is .b. With an actual list, there would be no need to escape the comma.
Also, the first autocmd in my vimrc listens to a list of events which is specified by a heredoc (i.e. a list of strings, not a string):
var delay_slow_call_events: list<string> =<< trim END
CursorHold
InsertEnter
WinEnter
CmdWinEnter
BufWritePost
QuickFixCmdPost
TextYankPost
TextChanged
END
augroup DelaySlowCall
autocmd!
execute $'autocmd {delay_slow_call_events->join(",")} * DelaySlowCall()'
autocmd CmdlineEnter :,/,\?,@ DelaySlowCall()
augroup END
With a string, I would probably write:
[{
group: 'DelaySlowCall',
event: delay_slow_call_events->join(','),
pattern: '*',
cmd: 'DelaySlowCall()',
}]->autocmd_add()
With a list:
[{
group: 'DelaySlowCall',
event: delay_slow_call_events,
pattern: '*',
cmd: 'DelaySlowCall()',
}]->autocmd_add()
In the latter, there is no need for ->join(',').
Although, I admit that these are corner cases. Ideally, autocmd_add() would support both a comma-separated list of events/patterns, as well as a list of events/patterns for the values of the event and pattern keys.
Just a comma-separated list would be fine too, I guess. And at least an error, instead of silently ignoring everything after the first comma.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
LemonBoy
+1 for using lists for lists of things, otherwise there would be no point in having types other than strings (and reinvent a poor man's TCL).
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
> +1 for using lists for lists of things, otherwise there would be no
> point in having types other than strings (and reinvent a poor man's
> TCL).
sense.
You could even avoid writing the list by doing:
'OneEvent,TwoEvent,ThreeEvent'->split(',')
If you are allergic to square brackets ...
--
"My particular problem is with registry entries, which seem to just
accumulate like plastic coffee cups..." -- Paul Moore
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.