[vim/vim] patch 9.2.0959: multiline dictionaries in :command and :autocmd cause E488 (PR #21073)
Amariah Abishai
Problem: A multiline dictionary argument to :command or :autocmd is parsed as a command block in Vim9 script.
Location: The issue is in the command-line collection path used by :command and :autocmd. In src/usercmd.c, may_get_cmd_block() relies on find_cmd_block_start() to determine whether subsequent lines belong to a command block. A trailing
{ in a multiline dictionary was not collected correctly, so the following dictionary entries were parsed as separate Ex commands and produced E488.
Solution: Extend the existing command-line collection logic to recognize a trailing dictionary opener when it follows an expression delimiter. Actual :command and :autocmd blocks continue to use find_cmd_block_start(), while commands such as normal! { remain ordinary command arguments and are not treated as blocks.
The fix uses the existing command-block collection path rather than introducing a separate parser or changing Vim9 expression parsing. This keeps the change localized to the existing :command and :autocmd handling and preserves the behavior added for nested command blocks.
Regression coverage was added for multiline dictionary arguments at script level and inside :def functions for both :command and :autocmd. Existing nested command-block and normal! { coverage also passes.
Tests:
- make -C src -j2
- make -C src/testdir test_usercommands.res
- make -C src/testdir test_autocmd.res
- make -C src/testdir test_vim9_script.res
- make -C src/testdir codestyle
- git diff --check
Fixes: #21070
The commit includes a Signed-off-by trailer and discloses AI assistance in the commit metadata.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/21073
Commit Summary
- 66332ad patch 9.2.0959: multiline dictionaries are parsed as command blocks
File Changes
(4 files)
- M src/testdir/test_autocmd.vim (21)
- M src/testdir/test_usercommands.vim (21)
- M src/usercmd.c (28)
- M src/version.c (2)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#21073)Nested braces are not collected
may_get_cmd_block() stops reading at the first line whose first non-blank
character is }:
if (*skipwhite(line) == '}') break;
The comment above the loop states this: "Does not support nesting or here-doc
constructs". That limitation is acceptable for a command block, where the
closing } is on its own line. A dictionary is different, because a nested
dictionary is ordinary:
vim9script command! Foo call Bar('x', { 'a': { 'b': 1, }, })
The line }, ends the collection, so }) is never read and the argument is
left unbalanced. A list inside the dictionary works, since ], does not
start with }, which makes the failure hard to predict from the outside.
Counting the brace depth instead of matching the first } would handle this,
and would fix the same limitation for command blocks. It changes existing
behaviour, so it deserves its own patch, but extending this collector to
dictionaries without it moves the limitation into a construct where nesting is
common.
Detecting the opener
find_cmd_dict_start() requires the line to end exactly with {:
char_u *p = line + STRLEN(line); if (p == line || p[-1] != '{') return NULL;
A trailing white space after { makes the detection fail. The existing
find_cmd_block_start() uses ends_excmd2() and does not have that problem,
so the two paths disagree on what "ends with a curly" means.
The set of characters accepted before { is (, ,, = and :. [ is
missing, and a list of dictionaries is a normal thing to write:
command! Foo call Bar('x', [{ 'key': 'value', }])
version.c
The patch number is assigned when the change is committed. Including a
version.c bump in the pull request will conflict with whatever number is
current at that time.
Line length
if (p == line || (p[-1] != '(' && p[-1] != ',' && p[-1] != '=' && p[-1] != ':'))
The continuation line is 89 columns wide. Aligning it under the first
condition keeps it within 80.
Tests
The two new tests cover the flat case. Since CheckScriptSuccess() already
fails on E488, the assert_match() calls add little; checking that the
dictionary body was collected, not only the first line, would say more. A
case with a nested dictionary and one with [{ would cover what is discussed
above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Amariah Abishai
AmariahAK left a comment (vim/vim#21073)@h-east
Thanks for the detailed review. I addressed the distinction between command blocks and dictionary expression continuation.
Actual command blocks continue to use find_cmd_block_start(). Dictionary arguments now use a separate predicate and are collected with brace-depth tracking, so nested dictionaries and list-of-dictionaries are handled without restoring the previous broad behavior that incorrectly treated normal!
The opener detection now trims trailing whitespace, accepts [, and uses vim_strchr(). The tests define and execute Bar(), then verify the collected dictionary contents, including nested values. The src/version.c change was also removed from the patch.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Amariah Abishai
AmariahAK left a comment (vim/vim#21073)@chrisbra
This is an edge case, but it uses existing Vim9 expression syntax rather than introducing new functionality. :command and :autocmd already accept arbitrary command text, and dictionaries are a normal way to pass structured values to functions.
The fix is narrowly scoped to the existing command-line collector. It distinguishes dictionary continuation from actual command blocks and adds regression coverage without changing the general Vim9 expression parser.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#21073)Please hold off on merging this.
I do not think we should take this patch, and it is better to say why than to
have the points above fixed first.
A trailing { cannot be classified without parsing the argument. It can open
a dictionary, a lambda body or a command block, and it can also be part of
normal! {, of a string, or of a pattern. This patch draws the line at four
characters before the curly. The next report will be about [{, then about a
nested dictionary, then about a { inside a string. Each of those is
fixable, and the boundary stays impossible to state in the documentation.
That looseness is what 9.2.0908 removed. Before it, any line ending in {
started a block, which is why normal! { was taken for one (#20918) and why
the dictionary in #21070 happened to work. Restoring one half of it brings
back the part that cannot be defined.
The documented answer already exists. |vim9-line-continuation| says that a
command used as the argument of another command needs a backslash, and a
dictionary closed on the same line was never affected:
command! -nargs=0 Foo call Bar('x', {'key': 'value'}) command! -nargs=0 Foo call Bar('x', { \ 'key': 'value', \ })
That note currently names only :windo. I will add :command and
:autocmd to it, with tests for the form above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#21073)The update addresses the details I listed, but it does so by adding a Vim
script lexer to usercmd.c: string skipping, comment handling and brace
counting. That is the parsing that the argument of :command and :autocmd
is defined not to need, so the objection is the same as before. Three
defects in the new code, as examples of what maintaining that lexer means:
count_cmd_curly() always treats " as the start of a string. In legacy
script it starts a comment. That matters here because the new path is not
limited to Vim9 script: may_get_cmd_block() is called from usercmd.c and
autocmd.c without a Vim9 check, so in legacy script a line such as
command Foo call Bar('x', { now starts collecting the following lines when
it did not before.
# is taken for a comment whenever in_vim9script() is true, without looking
at what precedes it, so an argument such as s/{/#/ ends the count early.
A { that is not a brace, as in s/{/x/, keeps the depth above zero, so the
collector reads to the end of the file and reports E1026.
in_vim9script() in the counter is also about the script being read, while
the argument may be executed with the other syntax. That distinction is what
the UC_VIM9 flag is for.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Amariah Abishai
AmariahAK left a comment (vim/vim#21073)@h-east
Thanks for explaining the concern. I took a step back to take a look and your way makes sense & is accurate
I withdrew the parser changes and restored the pre-workaround behavior. The revised patch now follows the documented Vim9 continuation mechanism instead.
The vim9-line-continuation documentation now names :command and :autocmd and includes examples using a backslash at the start of the continuation lines. The regression tests define and execute the called function and verify the resulting dictionary contents for both :command and :autocmd.
No source parser changes or src/version.c changes remain in the revised patch.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Amariah Abishai
AmariahAK left a comment (vim/vim#21073)@h-east
Thanks, i already withdrew the parser
The lexer-based changes, including count_cmd_curly(), the delimiter heuristic, and brace-depth collection, have been removed. The current revision no longer parses :command or :autocmd arguments in C and therefore does not introduce the legacy/Vim9 comment, string, pattern, or UC_VIM9-context issues described above.
The patch now documents the existing backslash-continuation mechanism for :command and :autocmd in vim9-line-continuation and adds regression tests that define and execute the called function and verify the dictionary contents.
No source parser changes or src/version.c changes remain.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#21073)Thanks for taking the step back, this is the right shape now. The tests are
better than what I had in mind, since they run the command and check the
value that reaches the function.
Three things left.
Please indent the example with a tab, as in the echo examples just above it.
It currently uses two spaces, so it does not line up with the rest of the
file:
backslash has to be used. For example: >
command! Foo call Bar('x', {
\ 'key': 'value',
\ })
Please revert the "Last change" line at the top of vim9.txt. It is updated
when a release is made.
Please squash the three commits into one. The C change is added and then
taken out again in the history, and the headlines claim patch 9.2.0959 and
patch 9.2.XXXX. The number is assigned when the change is committed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Amariah Abishai
AmariahAK left a comment (vim/vim#21073)@h-east
its done , could you kindly take a look?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.