[BUG] :bn/:bp do not respect shortmess+=F when 'hidden' is set (ATTN: chrisbra)

20 views
Skip to first unread message

Jason Franklin

unread,
May 16, 2019, 10:18:50 AM5/16/19
to vim_dev
To reproduce:

$ ./vim --clean # at latest commit on master

:set hidden shortmess+=F
:e file1
:e file2
" Notice, no messages are output, as expected.
:bn
:bp
" Notice fileinfo messages are output.

The main problem appears to be in the enter_buffer() function:

/* Make sure the buffer is loaded. */
if (curbuf->b_ml.ml_mfp == NULL) /* need to load the file */
{
/* If there is no filetype, allow for detecting one. Esp. useful for
* ":ball" used in a autocommand. If there already is a filetype we
* might prefer to keep it. */
if (*curbuf->b_p_ft == NUL)
did_filetype = FALSE;

open_buffer(FALSE, NULL, 0);
}
else
{
if (!msg_silent)
need_fileinfo = TRUE; /* display file info after redraw */
(void)buf_check_timestamp(curbuf, FALSE); /* check if file changed */

When 'nohidden', the first path is executed (we have to load
the buffer). When 'hidden' is set, the "else" is executed, and so
we rely on msg_silent to avoid the fileinfo message. However, msg_silent
is not reliably set for shortmess+=F in all scenarios.

Thus, I recommend the following patch:

diff --git a/src/buffer.c b/src/buffer.c
index e825a99..08595a9 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1742,9 +1742,9 @@ enter_buffer(buf_T *buf)
}
else
{
- if (!msg_silent)
- need_fileinfo = TRUE; /* display file info after redraw */
- (void)buf_check_timestamp(curbuf, FALSE); /* check if file changed */
+ if (!msg_silent && !shortmess(SHM_FILEINFO))
+ need_fileinfo = TRUE;
+ (void)buf_check_timestamp(curbuf, FALSE);
curwin->w_topline = 1;
#ifdef FEAT_DIFF
curwin->w_topfill = 0;

This fixes the problem with :bn/:bp, and all of the tests still pass.

At this point, I also want to point out a problem with the existing test,
reproduced below:

func Test_shortmess_F2()
e file1
e file2
call assert_match('file1', execute('bn', ''))
call assert_match('file2', execute('bn', ''))
set shortmess+=F
call assert_true(empty(execute('bn', '')))
call assert_true(empty(execute('bn', '')))
set hidden
call assert_true(empty(execute('bn', '')))
call assert_true(empty(execute('bn', '')))
set nohidden
call assert_true(empty(execute('bn', '')))
call assert_true(empty(execute('bn', '')))
set shortmess&
call assert_match('file1', execute('bn', ''))
call assert_match('file2', execute('bn', ''))
bwipe
bwipe
endfunc

This test is correct, meaning it should pass for the commands to be considered
working properly. The question is: Why doesn't this test catch the problem I noticed here?

Well, we can go back to the master branch (no patch) and see:

$ ./vim --clean

:set hidden
:e file1
:e file2
:let x = execute('bn', '')
" Notice, the message appears, as expected.
:echo x
" But, it wasn't captured!

I'm not sure how to fix this problem. The bug appears to be that execute()
does not capture messages that are set to be printed after the next redraw,
as opposed to being printed immediately. Is my understanding correct here?

Thanks,
Jason Franklin

Bram Moolenaar

unread,
May 16, 2019, 2:30:14 PM5/16/19
to vim...@googlegroups.com, Jason Franklin
Thanks, that looks OK.
You cannot get this message from execute(), because it only happens in
the main loop, after commands have been executed. I think the only way
is to check whether the need_fileinfo flag is set. And that only works
if we add a function for that.

--
"Beware of bugs in the above code; I have only proved
it correct, not tried it." -- Donald Knuth

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
Reply all
Reply to author
Forward
0 new messages