[vim/vim] Fix cmdheight problems (PR #10893)

50 views
Skip to first unread message

Shougo

unread,
Aug 11, 2022, 6:43:40 AM8/11/22
to vim/vim, Subscribed

Note: neovim does not exist the problems.

I have fixed two problems.

  • 'showmode' does work even cmdheight=0.

  • Double echoes :!{cmd} when cmdheight=0.

Fix #10881


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

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

Commit Summary

  • 4f3a943 Fix :! echo problem
  • ee417f3 Skip 'showmode' when cmdheight == 0

File Changes

(2 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/10893@github.com>

Shougo

unread,
Aug 11, 2022, 6:48:17 AM8/11/22
to vim/vim, Push

@Shougo pushed 1 commit.

  • de3c3db Skip <C-c> messages when cmdheight == 0


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

codecov[bot]

unread,
Aug 11, 2022, 6:57:28 AM8/11/22
to vim/vim, Subscribed

Codecov Report

Merging #10893 (de3c3db) into master (87f3a2c) will increase coverage by 0.87%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@

##           master   #10893      +/-   ##

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

+ Coverage   81.80%   82.67%   +0.87%     

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

  Files         158      148      -10     

  Lines      186293   173602   -12691     

  Branches    42110    39225    -2885     

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

- Hits       152393   143525    -8868     

+ Misses      21409    17482    -3927     

- Partials    12491    12595     +104     
Flag Coverage Δ
huge-clang-none 82.67% <87.50%> (-0.01%) ⬇️
linux 82.67% <87.50%> (-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/ex_cmds.c 85.80% <83.33%> (+0.11%) ⬆️
src/normal.c 90.28% <100.00%> (-0.98%) ⬇️
src/screen.c 78.75% <100.00%> (-0.62%) ⬇️
src/highlight.c 78.40% <0.00%> (-2.81%) ⬇️
src/time.c 87.08% <0.00%> (-2.56%) ⬇️
src/misc2.c 86.71% <0.00%> (-2.41%) ⬇️
src/help.c 80.06% <0.00%> (-2.34%) ⬇️
src/buffer.c 84.15% <0.00%> (-2.31%) ⬇️
src/session.c 63.15% <0.00%> (-1.94%) ⬇️
src/menu.c 81.19% <0.00%> (-1.85%) ⬇️
... and 131 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


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/10893/c1211833770@github.com>

Bram Moolenaar

unread,
Aug 11, 2022, 3:12:14 PM8/11/22
to vim/vim, Subscribed


> Note: neovim does not exist the problems.
>
> I have fixed two problems.
>
> * &#39;showmode&#39; does work even `cmdheight=0`.
>
> * Double echoes `:!{cmd}` when `cmdheight=0`.
>
> Fix #10881

Please add regression tests for these fixes.

--
"Intelligence has much less practical application than you'd think."
-- Scott Adams, Dilbert.

/// 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 are subscribed to this thread.Message ID: <vim/vim/pull/10893/c1212384774@github.com>

Shougo

unread,
Aug 12, 2022, 7:00:10 AM8/12/22
to vim/vim, Subscribed

I want to add the tests, but it is for screen tests.
I don't know how to add screen tests.
neovim has helper functions for it.


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/10893/c1212988088@github.com>

lacygoill

unread,
Aug 12, 2022, 7:10:35 AM8/12/22
to vim/vim, Subscribed

Not sure it helps here but there is:


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/10893/c1212996069@github.com>

lacygoill

unread,
Aug 12, 2022, 7:14:18 AM8/12/22
to vim/vim, Subscribed

See also the README.txt file in src/testdir/:

TO ADD A SCREEN DUMP TEST:

Mostly the same as writing a new style test.  Additionally, see help on
"terminal-dumptest".  Put the reference dump in "dumps/Test_func_name.dump".

TO ADD A NEW STYLE TEST:

1) Create a test_<subject>.vim file.
2) Add test_<subject>.res to NEW_TESTS_RES in Make_all.mak in alphabetical
   order.
3) Also add an entry "test_<subject>" to NEW_TESTS in Make_all.mak.
4) Use make test_<subject> to run a single test.

At 2), instead of running the test separately, it can be included in
"test_alot".  Do this for quick tests without side effects.  The test runs a
bit faster, because Vim doesn't have to be started, one Vim instance runs many
tests.

At 4), to run a test in GUI, add "GUI_FLAG=-g" to the make command.


What you can use (see test_assert.vim for an example):

- Call assert_equal(), assert_true(), assert_false(), etc.

- Use assert_fails() to check for expected errors.

- Use try/catch to avoid an exception aborts the test.

- Use test_alloc_fail() to have memory allocation fail.  This makes it possible
  to check memory allocation failures are handled gracefully.  You need to
  change the source code to add an ID to the allocation.  Add a new one to
  alloc_id_T, before aid_last.

- Use test_override() to make Vim behave differently, e.g.  if char_avail()
  must return FALSE for a while.  E.g. to trigger the CursorMovedI autocommand
  event. See test_cursor_func.vim for an example.

- If the bug that is being tested isn't fixed yet, you can throw an exception
  with "Skipped" so that it's clear this still needs work.  E.g.: throw
  "Skipped: Bug with <c-e> and popupmenu not fixed yet"

- The following environment variables are recognized and can be set to
  influence the behavior of the test suite (see runtest.vim for details)

  - $TEST_MAY_FAIL=Test_channel_one    - ignore those failing tests
  - $TEST_FILTER=Test_channel    - only run test that match this pattern
  - $TEST_SKIP_PAT=Test_channel  - skip tests that match this pattern
  - $TEST_NO_RETRY=yes           - do not try to re-run failing tests
  You can also set them in Vim:
    :let $TEST_MAY_FAIL = 'Test_channel_one'
    :let $TEST_FILTER = '_set_mode'
    :let $TEST_SKIP_PAT = 'Test_loop_forever'
    :let $TEST_NO_RETRY = 'yes'
  Use an empty string to revert, e.g.:
    :let $TEST_FILTER = ''

- See the start of runtest.vim for more help.


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/10893/c1212998949@github.com>

Shougo

unread,
Aug 12, 2022, 7:15:39 AM8/12/22
to vim/vim, Push

@Shougo pushed 4 commits.

  • a2b1d08 Fix :! echo problem
  • 59ce3ce Skip 'showmode' when cmdheight == 0
  • d999c17 Skip <C-c> messages when cmdheight == 0
  • 98355f6 Add screen tests

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

Shougo

unread,
Aug 12, 2022, 7:15:53 AM8/12/22
to vim/vim, Subscribed

I have added tests.


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/10893/c1213000322@github.com>

Shougo

unread,
Aug 12, 2022, 7:34:15 AM8/12/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Shougo

unread,
Aug 12, 2022, 9:15:19 AM8/12/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Shougo

unread,
Aug 12, 2022, 9:17:17 AM8/12/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Bram Moolenaar

unread,
Aug 12, 2022, 9:41:00 AM8/12/22
to vim/vim, Subscribed

In the tests "redraw!" is used before restoring 'cmdheight', shouldn't that be the other way around?


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/10893/c1213123071@github.com>

Bram Moolenaar

unread,
Aug 12, 2022, 9:42:45 AM8/12/22
to vim/vim, Subscribed

Clearing the screen at the end of msg_clr_eos_force() looks like a brute force method. Isn't there a better way?


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/10893/c1213125160@github.com>

Shougo

unread,
Aug 14, 2022, 1:24:34 AM8/14/22
to vim/vim, Push

@Shougo pushed 2 commits.

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

Shougo

unread,
Aug 14, 2022, 1:24:45 AM8/14/22
to vim/vim, Subscribed

In the tests "redraw!" is used before restoring 'cmdheight', shouldn't that be the other way around?

Fixed.


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/10893/c1214288483@github.com>

Shougo

unread,
Aug 14, 2022, 1:36:33 AM8/14/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Shougo

unread,
Aug 14, 2022, 1:38:01 AM8/14/22
to vim/vim, Subscribed

Clearing the screen at the end of msg_clr_eos_force() looks like a brute force method. Isn't there a better way?

I want to just redraw msg_row lines, but I cannot find the method in drawscreen.c...
Do you know the function?


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/10893/c1214289612@github.com>

Shougo

unread,
Aug 14, 2022, 1:44:30 AM8/14/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Bram Moolenaar

unread,
Aug 14, 2022, 8:30:55 AM8/14/22
to vim/vim, Subscribed


> > Clearing the screen at the end of msg_clr_eos_force() looks like a
> > brute force method. Isn't there a better way?
>
> I want to just redraw `msg_row` lines, but I cannot find the method in
> `drawscreen.c`... Do you know the function?

So this is when there is a message at the bottom of the screen, and you
want to remove it by redrawing the window at the bottom.

The optimal would be to set b_mod_top and b_mod_bot. You need to figure
out the buffer lines then (perhaps "w_botline - 1" works if there is only
one screen line affected).

If only the last window is affected, then redraw_win_later(wp,
NOT_VALID) should work. If the screen scrolled, then
redraw_all_later(VALID) probaby can be used, since "msg_scrolled" is
handled in update_screen().

--
Apologies for taking up the bandwidth with the apology. Anything else I
can apologise for ...... er no can't think of anything, sorry about that.
Andy Hunt (Member of British Olympic Apology Squad)


/// 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 are subscribed to this thread.Message ID: <vim/vim/pull/10893/c1214362135@github.com>

Shougo

unread,
Aug 15, 2022, 6:14:16 AM8/15/22
to vim/vim, Push

@Shougo pushed 1 commit.

  • cd59d27 Revert resize behavior changes

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

Shougo

unread,
Aug 15, 2022, 6:26:07 AM8/15/22
to vim/vim, Subscribed

The optimal would be to set b_mod_top and b_mod_bot. You need to figure
out the buffer lines then (perhaps "w_botline - 1" works if there is only
one screen line affected).

Well, but both b_mod_top and b_mod_top are line number.
It is not screen position. How to convert it?


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/10893/c1214863230@github.com>

Shougo

unread,
Aug 15, 2022, 6:34:28 AM8/15/22
to vim/vim, Subscribed

If only the last window is affected, then redraw_win_later(wp,
NOT_VALID) should work. If the screen scrolled, then
redraw_all_later(VALID) probaby can be used, since "msg_scrolled" is
handled in update_screen().

And how to compute what windows are affected?
I have read the code and I have not found the optimization.
It seems simple redraw routine.


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/10893/c1214869510@github.com>

Shougo

unread,
Aug 15, 2022, 6:49:27 AM8/15/22
to vim/vim, Subscribed

Hm... I have created the routine, but it is too difficult and does not work well.

diff --git a/src/message.c b/src/message.c
index 48012bb32..dbe41be04 100644
--- a/src/message.c
+++ b/src/message.c
@@ -3477,7 +3477,46 @@ msg_clr_eos_force(void)
    }
     }
     else
-   redraw_all_later(VALID);
+    {
+   int scrolled;
+   int type = VALID;
+   win_T   *wp;
+
+   scrolled = Rows - msg_row;
+   if (scrolled > Rows - 5)        // clearing is faster
+       type = CLEAR;
+   else if (type != CLEAR)
+   {
+       check_for_delay(FALSE);
+       if (screen_ins_lines(0, 0, scrolled, (int)Rows, 0, NULL)
+                                      == FAIL)
+       type = CLEAR;
+       FOR_ALL_WINDOWS(wp)
+       {
+       if (wp->w_winrow < scrolled)
+       {
+           if (W_WINROW(wp) + wp->w_height > scrolled
+               && wp->w_redr_type < REDRAW_TOP
+               && wp->w_lines_valid > 0
+               && wp->w_topline == wp->w_lines[0].wl_lnum)
+           {
+           wp->w_upd_rows = scrolled - W_WINROW(wp);
+           wp->w_redr_type = REDRAW_TOP;
+           }
+           else
+           {
+           wp->w_redr_type = NOT_VALID;
+           if (W_WINROW(wp) + wp->w_height + wp->w_status_height
+                                  <= msg_scrolled)
+               wp->w_redr_status = TRUE;
+           }
+       }
+       }
+       redraw_tabline = TRUE;
+   }
+
+   redraw_all_later(type);
+    }
 }


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/10893/c1214879746@github.com>

Shougo

unread,
Aug 15, 2022, 6:50:27 AM8/15/22
to vim/vim, Push

@Shougo pushed 1 commit.

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

Shougo

unread,
Aug 15, 2022, 6:51:54 AM8/15/22
to vim/vim, Subscribed

redraw_all_later(CLEAR); is not smart. But it works well without problems.
I like works version of code.


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/10893/c1214881327@github.com>

Shougo

unread,
Aug 15, 2022, 6:56:21 AM8/15/22
to vim/vim, Subscribed

message.c: In function ‘msg_clr_eos_force’:

message.c:3481:26: error: ‘CLEAR’ undeclared (first use in this function); did you mean ‘CAR’?

 3481 |         redraw_all_later(CLEAR);

      |                          ^~~~~

      |                          CAR

Why?

CLEAR is defined in vim.h and it is already included.


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/10893/c1214884111@github.com>

Shougo

unread,
Aug 15, 2022, 6:57:46 AM8/15/22
to vim/vim, Push

@Shougo pushed 1 commit.


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

Bram Moolenaar

unread,
Aug 15, 2022, 7:13:27 AM8/15/22
to vim/vim, Subscribed


```

message.c: In function ‘msg_clr_eos_force’:
message.c:3481:26: error: ‘CLEAR’ undeclared (first use in this function); did you mean ‘CAR’?
3481 | redraw_all_later(CLEAR);
| ^~~~~
| CAR
```

Why?

`CLEAR` is defined in `vim.h` and it is already included.

It was renamed UPD_CLEAR in 9.0.0206

--
A real patriot is the fellow who gets a parking ticket and rejoices
that the system works.



/// 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, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/pull/10893/c1214897375@github.com>

Shougo

unread,
Aug 28, 2022, 2:23:44 AM8/28/22
to vim/vim, Push

@Shougo pushed 15 commits.


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

Shougo

unread,
Aug 28, 2022, 2:31:36 AM8/28/22
to vim/vim, Push

@Shougo pushed 1 commit.

  • 15cb7ab check p_ch in skip_showmode() is not needed

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

Bram Moolenaar

unread,
Aug 28, 2022, 7:31:43 AM8/28/22
to vim/vim, Subscribed

The messages test fails in some builds, and there is an ASAN error. Probably the tests uncovered a problem, I'll have a look.


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/10893/c1229437764@github.com>

Bram Moolenaar

unread,
Aug 28, 2022, 9:44:36 AM8/28/22
to vim/vim, Subscribed

I have included the tests and fixed a few problems.
Are the code changes in this PR still needed? If yes, then please add a test that fails without them.


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/10893/c1229460118@github.com>

Shougo

unread,
Aug 29, 2022, 8:37:26 PM8/29/22
to vim/vim, Subscribed

Thanks. I will rebase it later.


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/10893/c1231010598@github.com>

Shougo

unread,
Aug 29, 2022, 8:44:06 PM8/29/22
to vim/vim, Subscribed

Closed #10893.


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/10893/issue_event/7283051057@github.com>

Shougo

unread,
Aug 29, 2022, 8:44:08 PM8/29/22
to vim/vim, Subscribed

I think the patch is not needed anymore. But I will re-create another PR to revert some change.


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/10893/c1231014425@github.com>

Bram Moolenaar

unread,
Aug 30, 2022, 6:15:33 AM8/30/22
to vim/vim, Subscribed


> I think the patch is not needed anymore. But I will re-create another
> PR to revert some change.

While trying out cmdheight=0 I noticed that once in a while the last
statusline goes missing, the last window is too tall. I also had to fix
a crash where an index goes over Rows, the two are probably related.
I haven't been able to reproduce it, it might have something to do with
a shell command.

Anyway, please check my question on the vim-dev maillist, discussing
whether we should keep supporting 'cmdheight' zero or revert it.


--
ARTHUR: It is I, Arthur, son of Uther Pendragon, from the castle of Camelot.
King of all Britons, defeator of the Saxons, sovereign of all England!
[Pause]
SOLDIER: Get away!
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD


/// 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 are subscribed to this thread.Message ID: <vim/vim/pull/10893/c1231460957@github.com>

Shougo

unread,
Aug 30, 2022, 7:22:26 AM8/30/22
to vim/vim, Subscribed

While trying out cmdheight=0 I noticed that once in a while the last
statusline goes missing, the last window is too tall. I also had to fix
a crash where an index goes over Rows, the two are probably related.
I haven't been able to reproduce it, it might have something to do with
a shell command.

Well, I don't reproduce the problem.
I will check it if it is reproduced.


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/10893/c1231527976@github.com>

Shougo

unread,
Aug 30, 2022, 7:22:57 AM8/30/22
to vim/vim, Subscribed

Anyway, please check my question on the vim-dev maillist, discussing
whether we should keep supporting 'cmdheight' zero or revert it.

OK. I will take look later.


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/10893/c1231528527@github.com>

Reply all
Reply to author
Forward
0 new messages