[vim/vim] Optionally display error messages if expansion fails with expandcmd() (PR #10071)

7 views
Skip to first unread message

Yegappan Lakshmanan

unread,
Apr 3, 2022, 11:01:48 AM4/3/22
to vim/vim, Subscribed

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/10071

Commit Summary

  • c53ee0a Optionally display error messages if expansion fails with expandcmd()

File Changes

(6 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071@github.com>

codecov[bot]

unread,
Apr 3, 2022, 11:09:52 AM4/3/22
to vim/vim, Subscribed

Codecov Report

Merging #10071 (c53ee0a) into master (c4cb544) will decrease coverage by 79.99%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@

##           master   #10071       +/-   ##

===========================================

- Coverage   82.00%    2.01%   -80.00%     

===========================================

  Files         167      152       -15     

  Lines      187458   170589    -16869     

  Branches    42236    39471     -2765     

===========================================

- Hits       153733     3430   -150303     

- Misses      21369   166464   +145095     

+ Partials    12356      695    -11661
Flag Coverage Δ
huge-clang-none ?
huge-gcc-none ?
huge-gcc-testgui ?
huge-gcc-unittests 2.01% <0.00%> (-0.01%) ⬇️
linux 2.01% <0.00%> (-81.95%) ⬇️
mingw-x64-HUGE-gui ?
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/dict.c 4.21% <0.00%> (-80.44%) ⬇️
src/evalfunc.c 0.00% <0.00%> (-90.90%) ⬇️
src/libvterm/src/rect.h 0.00% <0.00%> (-96.78%) ⬇️
src/sha256.c 0.00% <0.00%> (-96.45%) ⬇️
src/libvterm/src/parser.c 0.00% <0.00%> (-95.91%) ⬇️
src/gui_gtk_f.c 0.00% <0.00%> (-94.72%) ⬇️
src/crypt_zip.c 0.00% <0.00%> (-94.12%) ⬇️
src/vim9compile.c 0.00% <0.00%> (-92.42%) ⬇️
src/debugger.c 0.00% <0.00%> (-92.23%) ⬇️
src/eval.c 0.10% <0.00%> (-92.14%) ⬇️
... and 155 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 c4cb544...c53ee0a. 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.Message ID: <vim/vim/pull/10071/c1086888268@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 11:18:26 AM4/3/22
to vim/vim, Push

@yegappan pushed 1 commit.

  • cc53822 Test fails if the shell is not bash

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/push/9524440136@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 12:23:46 PM4/3/22
to vim/vim, Push

@yegappan pushed 1 commit.

  • 0d68147 Optionally display error messages if expansion fails with expandcmd()

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/push/9524697957@github.com>

Bram Moolenaar

unread,
Apr 3, 2022, 1:52:23 PM4/3/22
to vim/vim, Subscribed

ASAN reports a leak, perhaps something is not cleared on failure?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/c1086917091@github.com>

LemonBoy

unread,
Apr 3, 2022, 2:14:21 PM4/3/22
to vim/vim, Subscribed

@LemonBoy commented on this pull request.

In src/evalfunc.c:

>  	return;
 
+    if (argvars[1].v_type == VAR_DICT
+	    && dict_has_key(argvars[1].vval.v_dict, "errmsg", -1))

Shouldn't this be dict_get_bool? if errmsg is present but set to v:false then emsgoff should not be set to false.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/review/929706535@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 2:58:55 PM4/3/22
to vim_dev, reply+ACY5DGEOQ2XWQJCOWD...@reply.github.com, vim/vim, Subscribed
Hi Bram,

On Sun, Apr 3, 2022 at 10:52 AM Bram Moolenaar <vim-dev...@256bit.org> wrote:

ASAN reports a leak, perhaps something is not cleared on failure?



Yes. The new test exposed an existing memory leak. I have updated the PR with a fix.

- Yegappan 

vim-dev ML

unread,
Apr 3, 2022, 2:59:14 PM4/3/22
to vim/vim, vim-dev ML, Your activity

Hi Bram,

On Sun, Apr 3, 2022 at 10:52 AM Bram Moolenaar ***@***.***>

wrote:

> ASAN reports a leak, perhaps something is not cleared on failure?
>
>
>
Yes. The new test exposed an existing memory leak. I have updated the PR
with a fix.

- Yegappan

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/c1086927951@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 2:59:20 PM4/3/22
to vim_dev, reply+ACY5DGA4YAYA4OBF2S...@reply.github.com, vim/vim, Subscribed
Hi,

On Sun, Apr 3, 2022 at 11:14 AM LemonBoy <vim-dev...@256bit.org> wrote:

@LemonBoy commented on this pull request.

In src/evalfunc.c:

>  	return;
 
+    if (argvars[1].v_type == VAR_DICT
+	    && dict_has_key(argvars[1].vval.v_dict, "errmsg", -1))

Shouldn't this be dict_get_bool? if errmsg is present but set to v:false then emsgoff should not be set to false.



Yes. I have updated the PR with this change.

- Yegappan 

vim-dev ML

unread,
Apr 3, 2022, 2:59:38 PM4/3/22
to vim/vim, vim-dev ML, Your activity

Hi,

On Sun, Apr 3, 2022 at 11:14 AM LemonBoy ***@***.***> wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/evalfunc.c
> <https://github.com/vim/vim/pull/10071#discussion_r841258614>:

>
> > return;
>
> + if (argvars[1].v_type == VAR_DICT
> + && dict_has_key(argvars[1].vval.v_dict, "errmsg", -1))
>
> Shouldn't this be dict_get_bool? if errmsg is present but set to v:false
> then emsgoff should not be set to false.
>
>
>
Yes. I have updated the PR with this change.

- Yegappan

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/c1086928035@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 2:59:56 PM4/3/22
to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

  • 4f83ad4 Fix the memory leak and use dict_get_bool()

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/push/9525302005@github.com>

Yegappan Lakshmanan

unread,
Apr 3, 2022, 3:00:26 PM4/3/22
to vim/vim, vim-dev ML, Push

@yegappan pushed 2 commits.

  • d8eed31 Optionally display error messages if expansion fails with expandcmd()
  • 01e6aa6 Fix the memory leak and use dict_get_bool()

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10071/push/9525303915@github.com>

Bram Moolenaar

unread,
Apr 3, 2022, 4:30:57 PM4/3/22
to vim/vim, vim-dev ML, Comment

Closed #10071 via 2b74b68.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.Message ID: <vim/vim/pull/10071/issue_event/6358758144@github.com>

Reply all
Reply to author
Forward
0 new messages