[vim/vim] Add support for specifying a list of autocmd event names to autocmd_add() (PR #10483)
Yegappan Lakshmanan
The autocmd_add() function now supports either a String with one or more
autocmd event names separated by comma or a List of event names.
It also supports a String or a List for the autocmd pattern.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/10483
Commit Summary
- d54b1cb Add support for specifying a list of autocmd event names to autocmd_add()
File Changes
(4 files)
- M runtime/doc/builtin.txt (7)
- M src/autocmd.c (155)
- M src/errors.h (6)
- M src/testdir/test_autocmd.vim (46)
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 #10483 (d54b1cb) into master (c3caa7f) will increase coverage by
0.92%.
The diff coverage is75.32%.
@@ Coverage Diff @@ ## master #10483 +/- ## ========================================== + Coverage 81.63% 82.55% +0.92% ========================================== Files 158 148 -10 Lines 185112 172507 -12605 Branches 41868 39012 -2856 ========================================== - Hits 151118 142421 -8697 + Misses 21479 17475 -4004 - Partials 12515 12611 +96
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | 82.55% <75.32%> (+<0.01%) |
⬆️ |
| linux | 82.55% <75.32%> (+<0.01%) |
⬆️ |
| mingw-x64-HUGE | ? |
|
| mingw-x64-HUGE-gui | ? |
|
| windows | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/autocmd.c | 83.11% <75.32%> (-1.81%) |
⬇️ |
| src/highlight.c | 78.69% <0.00%> (-2.64%) |
⬇️ |
| src/misc2.c | 86.38% <0.00%> (-2.52%) |
⬇️ |
| src/help.c | 80.07% <0.00%> (-2.38%) |
⬇️ |
| src/buffer.c | 84.09% <0.00%> (-2.36%) |
⬇️ |
| src/time.c | 87.08% <0.00%> (-2.35%) |
⬇️ |
| src/libvterm/src/screen.c | 51.96% <0.00%> (-2.04%) |
⬇️ |
| src/session.c | 63.15% <0.00%> (-1.94%) |
⬇️ |
| src/menu.c | 81.03% <0.00%> (-1.86%) |
⬇️ |
| src/gui.c | 71.15% <0.00%> (-1.75%) |
⬇️ |
| ... and 130 more |
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 c3caa7f...d54b1cb. 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.
LemonBoy
@LemonBoy commented on this pull request.
In src/autocmd.c:
> {
- event = event_name2nr(event_name, &end);
- if (event == NUM_EVENTS)
- {
- semsg(_(e_no_such_event_str), event_name);
- retval = VVAL_FALSE;
- break;
- }
+ event_name = di->di_tv.vval.v_string;
+ if (event_name == NULL)
+ continue;
Empty strings should be an error, no?
In src/autocmd.c:
> {
- event = event_name2nr(event_name, &end);
- if (event == NUM_EVENTS)
- {
- semsg(_(e_no_such_event_str), event_name);
- retval = VVAL_FALSE;
- break;
- }
+ event_name = di->di_tv.vval.v_string;
+ if (event_name == NULL)
+ continue;
+ }
+ else if (di->di_tv.v_type == VAR_LIST)
+ {
+ event_list = di->di_tv.vval.v_list;
+ if (event_list == NULL)
Ditto for empty lists (?). And what happens when a range is passed?
In src/autocmd.c:
> } + else + continue;
Unsupported type -> error message.
In src/autocmd.c:
> {
- if (delete)
- pat = vim_strsave((char_u *)"");
+ if (di->di_tv.v_type == VAR_STRING)
+ {
+ pat = di->di_tv.vval.v_string;
Ditto as above wrt error conditions.
In src/autocmd.c:
> {
- retval = VVAL_FALSE;
- break;
+ if (event_list != NULL)
+ {
+ if (eli == NULL)
+ eli = event_list->lv_first;
+ else
+ eli = eli->li_next;
+ if (eli == NULL)
+ break;
+ if (eli->li_tv.v_type != VAR_STRING
tv_get_string_strict? Let's add more checks/errors instead of silently ignoring errors.
In src/autocmd.c:
> + if (pat != NULL)
+ {
+ if (do_autocmd_event(event, pat, once, nested, cmd,
+ delete | replace, group, 0) == FAIL)
+ {
+ retval = VVAL_FALSE;
+ break;
+ }
+ }
+ else if (pat_list != NULL)
+ {
+ FOR_ALL_LIST_ITEMS(pat_list, pli)
+ {
+ if (pli->li_tv.v_type != VAR_STRING
+ || pli->li_tv.vval.v_string == NULL)
+ continue;
Ditto, checks checks check.
In src/autocmd.c:
> + if (eli->li_tv.v_type != VAR_STRING
+ || eli->li_tv.vval.v_string == NULL)
+ continue;
+ p = eli->li_tv.vval.v_string;
+ }
+ else
+ {
+ if (end == NULL)
+ p = end = event_name;
+ if (end == NULL || *end == NUL)
+ break;
+ }
+ if (p == NULL)
+ continue;
+
+ event = event_name2nr(p, &end);
I don't see any check for trailing characters after the event name, nor checks for trailing commas when passing a string.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
either a String with one or more autocmd event names separated by comma or a List of event names
Shouldn't we be more consistent here and either allow String for single events or list with multiple events? Or maybe even better disallow strings at all and only allow a list of events (since this is pretty new, shouldn't break users expectations yet I hope)?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
The most obvious would be allow for a string with a single item, and a list of strings with at least one item.
Each string should be one event name. I don't think we need to strip white space.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
either a String with one or more autocmd event names separated by comma or a List of event names
Shouldn't we be more consistent here and either allow String for single events or list with multiple events? Or maybe even better disallow strings at all and only allow a list of events (since this is pretty new, shouldn't break users expectations yet I hope)?
vim-dev ML
On Thu, May 26, 2022 at 1:07 AM Christian Brabandt <
> either a String with one or more autocmd event names separated by comma or
>
> Shouldn't we be more consistent here and either allow String for single
> events or list with multiple events? Or maybe even better disallow strings
> at all and only allow a list of events (since this is pretty new, shouldn't
> break users expectations yet I hope)?
>
>
> The autocmd_get() function returns a String for the event name and pattern.
If you want to pass the return value of the autocmd_get() function to the
autocmd_add() function, then we need to support the String type for the
event
name and pattern.
Regards,
Yegappan
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
@yegappan pushed 1 commit.
- 52d23fa Display error messages for invalid values
—
View it on GitHub or unsubscribe.
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.