Add 'showcmd' statusline item and 'showcmdloc' option by luukvbaal · Pull Request #11684 · vim/vim

luukvbaal · 2022-12-09T13:15:24Z

This statusline item is mostly intended for use with cmdheight == 0 in Neovim. Nevertheless, users have expressed interest in displaying showcmd in their statusline and even tabline.

Just checking if you are open to including this to avoid future naming conflicts of the statusline item. The item name is still up for debate, @justinmk suggested dot. instead.

I assume potential users of this item would set noshowcmd, therefore I decoupled the showcmd routines from p_sc, except for the actual drawing in the cmdline.

I tried adding a non-screendump test to Test_statusline() but only the visual mode test passes. The pending operator test yields an empty statusline. Does the call assert_match() clear the showcmd buffer? That's what it seems like atleast:

  " %S: 'showcmd' content.
  set statusline=%S
  call assert_match('^\s*$', s:get_statusline())
  call feedkeys("\<C-V>l", "xt")
  call assert_match('^1x2\s*$', s:get_statusline())
  call feedkeys("\<Esc>1234", "xt")
  call assert_match('^1234\s*$', s:get_statusline())
  call feedkeys("\<Esc>", "xt")
  " result:
  " RunTheTest[44]..Test_statusline line 130: Pattern '^\\[1234\\]s*$' does not match '\[  occurs 80 times]'

Therefore I grouped them as 2 screendump tests instead. It works as intended in practice.

Redrawing the statusline upon command input caused a test failure in Test_aa_terminal_focus_events(), updated the dump files to reflect the statusline update.

zeertzjq · 2022-12-09T13:23:37Z

feedkeys() with x flag needs to cancel pending operator to finish, so it is not possible to stop at op-pending mode after that. I guess it may be possible to check the screen in op-pending mode using timers, but that may make the test confusing.

codecov · 2022-12-09T13:27:14Z

Codecov Report

Merging #11684 (1d5eacf) into master (98aeb21) will decrease coverage by 0.00%.
The diff coverage is 41.37%.

@@            Coverage Diff             @@
##           master   #11684      +/-   ##
==========================================
- Coverage   81.81%   81.80%   -0.01%     
==========================================
  Files         164      164              
  Lines      190973   191298     +325     
  Branches    43370    43437      +67     
==========================================
+ Hits       156242   156491     +249     
- Misses      22046    22097      +51     
- Partials    12685    12710      +25

Flag	Coverage Δ
huge-clang-none	`82.66% <42.85%> (-0.03%)`	⬇️
huge-gcc-none	`54.56% <13.79%> (-0.01%)`	⬇️
huge-gcc-testgui	`53.03% <13.79%> (+0.01%)`	⬆️
huge-gcc-unittests	`0.29% <0.00%> (-0.01%)`	⬇️
linux	`82.43% <41.37%> (-0.02%)`	⬇️
mingw-x64-HUGE	`76.23% <38.46%> (+<0.01%)`	⬆️
mingw-x86-HUGE	`77.02% <38.46%> (+<0.01%)`	⬆️
windows	`77.87% <38.46%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
src/buffer.c	`87.21% <0.00%> (-0.14%)`	⬇️
src/drawscreen.c	`80.96% <0.00%> (-0.29%)`	⬇️
src/screen.c	`80.18% <16.66%> (-0.19%)`	⬇️
src/normal.c	`90.88% <72.72%> (-0.08%)`	⬇️
src/optionstr.c	`91.53% <100.00%> (+0.02%)`	⬆️
src/vim9type.c	`90.26% <0.00%> (-0.76%)`	⬇️
src/vim9expr.c	`91.41% <0.00%> (-0.67%)`	⬇️
src/if_xcmdsrv.c	`77.25% <0.00%> (-0.35%)`	⬇️
src/userfunc.c	`88.96% <0.00%> (-0.19%)`	⬇️
src/libvterm/src/screen.c	`55.57% <0.00%> (-0.18%)`	⬇️
... and 20 more

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

brammool · 2022-12-09T14:35:36Z

Please first add documentation. Reverse engineering the code to guess what it's supposed to do isn't the right way to add a new feature.
One problem I can spot is that it's redrawing the status lines even when the showcmd item isn't in there.
Also, what happens when 'showcmd' is off is unclear.

luukvbaal · 2022-12-09T16:16:40Z

Please first add documentation.

Right, completely forgot sorry.

One problem I can spot is that it's redrawing the status lines even when the showcmd item isn't in there.

True, should 'statusline' be parsed for this item? Is there precedent for this?

Also, what happens when 'showcmd' is off is unclear.

When 'showcmd' is off, we still fill the showcmd_buf, because the user might have the item in use. If we parse the status/tabline we could set a global and avoid unnecessary work I guess.

luukvbaal · 2022-12-09T17:41:10Z

If we parse the status/tabline we could set a global and avoid unnecessary work I guess.

I guess there's no reason not to, latest commit sets a global boolean when the 'showcmd' item is encountered. Status line will only be redrawn when TRUE, so reverted the terminal focus testdump change.

brammool · 2022-12-09T19:26:08Z

> Please first add documentation. Right, completely forgot sorry. > One problem I can spot is that it's redrawing the status lines even when the showcmd item isn't in there. True, should 'statusline' be parsed for this item? Is there precedent for this?

I don't think so. Currently there is no item that changes on every key stroke, AFAIK. Parsing the option only works when when it doesn't start with "!%". I think the only way would be to draw the status line, and set a flag if the "showcmd" value is used. But then perhaps it isn't always called (e.g. if there is not enough room), that makes it unreliable.

> Also, what happens when 'showcmd' is off is unclear. When 'showcmd' is off, we still fill the `showcmd_buf`, because the user might have the item in use. If we parse the status/tabline we could set a global and avoid unnecessary work I guess.

If 'showcmd' would have been a number or string option we could have done something, but since it's a boolean we can't change it. We could tackle both problems with a 'showcmdwhere' option (better name?), to tell Vim where the command will be displayed, thus knowing when to update status lines. Since the user has to switch 'showcmd' off, setting another option isn't really a problem.

…

-- MAN: You don't frighten us, English pig-dog! Go and boil your bottoms, son of a silly person. I blow my nose on you, so-called Arthur-king, you and your silly English K...kaniggets. He puts hands to his ears and blows a raspberry. "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 ///

luukvbaal · 2022-12-09T23:48:04Z

Parsing the option only works when when it doesn't start with "!%". I think the only way would be to draw the status line, and set a flag if the "showcmd" value is used.

Yeah I realized, the latest commit does just that.

We could tackle both problems with a 'showcmdwhere' option (better
name?), to tell Vim where the command will be displayed, thus knowing when to update status lines.

Right, let me know if that is a route you want to take to include this feature. I can update accordingly. Do we want to facilitate displaying this item in both the status line and tabline? Or even multiple(status/tab/cmdline) at the same time? In which case I suppose it should be a comma separated string option.

Since the user has to switch 'showcmd' off, setting another option isn't really a problem.

I guess in theory they don't have to but it would make the most sense yeah.

brammool · 2022-12-10T11:16:49Z

> Parsing the option only works when when it doesn't start with "!%". > I think the only way would be to draw the status line, and set a > flag if the "showcmd" value is used. Yeah I realized, the latest commit does just that. > We could tackle both problems with a 'showcmdwhere' option (better >name?), to tell Vim where the command will be displayed, thus knowing >when to update status lines. Right, let me know if that is a route you want to take to include this feature. I can update accordingly. Do we want to facilitate displaying this item in both the status line and tabline? Or even multiple(status/tab/cmdline) at the same time? In which case I suppose it should be a comma separated string option.

Would someone really want it in multiple places? That sounds unusual. I would not implement that at first, but make the option value in a way that it's possible in the future. When you say "status line" it doesn't say where it shows up. Only the status line for the most bottom-right window? Otherwise it's specified by the 'statusline' option. But it could also be done when 'statusline' is empty. I find it hard to imagine showcmd showing up in the tabline. I suppose some space would need to be reserved for it. Then the choice is on the far left or the far right.

> Since the user has to switch 'showcmd' off, setting another option > isn't really a problem. I guess in theory they don't **have** to but it would make the most sense yeah.

With the extra option, probably 'showcmdloc', then 'showcmd' would still be on. If it is off then it won't show anywhere. That's simple. 'showcmdloc' would now be one word, such as "statusline", "tablineright", etc. If we ever would want to show it in more than one place the option can become a comma separated list of these words. I still have the feeling that very few users are going to use this, thus consider it low priority.

…

-- ARTHUR: (as the MAN next to him is squashed by a sheep) Knights! Run away! Midst echoing shouts of "run away" the KNIGHTS retreat to cover with the odd cow or goose hitting them still. The KNIGHTS crouch down under cover. "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 ///

luukvbaal · 2022-12-10T11:36:51Z

Would someone really want it in multiple places? That sounds unusual.
I would not implement that at first, but make the option value in a way
that it's possible in the future.

When you say "status line" it doesn't say where it shows up. Only the
status line for the most bottom-right window? Otherwise it's specified
by the 'statusline' option. But it could also be done when 'statusline'
is empty.

I find it hard to imagine showcmd showing up in the tabline. I suppose
some space would need to be reserved for it. Then the choice is on the
far left or the far right.

Yes showing it in multiple places would be unusual. The way I implemented it currently and what seems to make the most sense to me is to redraw the status line for curwin. Whether it shows up is not our concern I don't think. It would indeed be up to the user to reserve space, if they find the item high enough priority.

With the extra option, probably 'showcmdloc', then 'showcmd' would still
be on. If it is off then it won't show anywhere. That's simple.

'showcmdloc' would now be one word, such as "statusline",
"tablineright", etc. If we ever would want to show it in more than one
place the option can become a comma separated list of these words.

I still have the feeling that very few users are going to use this, thus
consider it low priority.

Very possible, again I only made the PR here to avoid future naming conflicts with Neovim, and get a feel for what item-symbol you would choose. But if we are going to add the feature here we have to add it in a way that makes sense. I think we may not need the 'showcmdloc' option. Since we now pass the option name to build_stl_str_hl(), we can simply set a flag for status/tabline and redraw accordingly in display_showcmd().

brammool · 2022-12-10T12:27:25Z

Yes showing it in multiple places would be unusual. The way I implemented it currently and what seems to make the most sense to me is to redraw the status line for `curwin`. Whether it shows up is not our concern I don't think. It would indeed be up to the user to reserve space, if they find the item high enough priority.

It does make sense to show it in the current window. That's closest to where the user is looking. When 'statusline' is empty it should still show up. Left of the ruler would be good.

Very possible, again I only made the PR here to avoid future naming conflicts with Neovim, and get a feel for what item-symbol you would choose. But if we are going to add the feature here we have to add it in a way that makes sense. I think we may not need the 'showcmdloc' option. Since we now pass the option name to `build_stl_str_hl()`, we can simply set a flag for status/tabline and redraw accordingly in `display_showcmd()`.

In my opinion we do need to add the 'showcmdloc' option. Without it, the user would have to reset 'showcmd' and add the item to the 'statusline' option. This would black adding 'showcmdloc' later, because then 'showcmd' would remain on, thus it can't be backwards compatible.

…

-- In his lifetime van Gogh painted 486 oil paintings. Oddly enough, 8975 of them are to be found in the United States. /// 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 ///

brammool · 2022-12-10T13:18:46Z

I wrote:

> Yes showing it in multiple places would be unusual. The way I > implemented it currently and what seems to make the most sense to me > is to redraw the status line for `curwin`. Whether it shows up is not > our concern I don't think. It would indeed be up to the user to > reserve space, if they find the item high enough priority. It does make sense to show it in the current window. That's closest to where the user is looking. When 'statusline' is empty it should still show up. Left of the ruler would be good. > Very possible, again I only made the PR here to avoid future naming > conflicts with Neovim, and get a feel for what item-symbol you would > choose. But if we are going to add the feature here we have to add it > in a way that makes sense. I think we may not need the 'showcmdloc' > option. Since we now pass the option name to `build_stl_str_hl()`, we > can simply set a flag for status/tabline and redraw accordingly in > `display_showcmd()`. In my opinion we do need to add the 'showcmdloc' option. Without it, the user would have to reset 'showcmd' and add the item to the 'statusline' option. This would black adding 'showcmdloc' later,

s/black/block/

…

because then 'showcmd' would remain on, thus it can't be backwards compatible.

-- press CTRL-ALT-DEL for more information /// 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 ///

luukvbaal · 2022-12-10T18:47:15Z

When 'statusline' is empty it should still show up. Left of the ruler would be good.

Latest commit adds the 'showcmdloc' option. Will need to figure out how to add this to the default statusline if 'showcmdloc' == "statusline".

luukvbaal · 2022-12-11T01:10:38Z

Will need to figure out how to add this to the default statusline if 'showcmdloc' == "statusline".

Done in the latest commit, as well as for the default tabline. (I don't see it being all that useful in the tabline, I just added it because I came across https://vi.stackexchange.com/questions/16857/showcmd-on-first-line-instead-of-last-line).

I think the attributes for the showcmd text in the default tabline should be attr_nosel without underline. I tried attr_nosel & ~HL_UNDERLINE but that also changed the color. Any way I can use attr_nosel but remove the underline?

luukvbaal · 2022-12-12T01:07:47Z

Any way I can use attr_nosel but remove the underline?

I guess it doesn't make sense to forcefully remove the underline from a highlight group (didn't actually figure out how to do that). This is finished then as far as I'm concerned. Tests for the default status/tabline are still missing. If you are open to including this in it's current form I could add additional tests.

brammool · 2022-12-13T14:22:30Z

I thought I had already mentioned this: when 'showcmd' is off then the command should not be displayed anywhere.
It is too surprising that with 'noshowcmd' and then 'showcmdloc' set the command shows anyway.

The default value for 'showcmdloc' should be "last". Currently you could replace it with "statusline" or "tabline".
If we ever decide to show it in multiple places, it could be "last,statusline", for example.

Coding style: Please put the opening curly on the next line.

src/buffer.c

luukvbaal · 2022-12-13T15:10:54Z

I thought I had already mentioned this: when 'showcmd' is off then the command should not be displayed anywhere.
It is too surprising that with 'noshowcmd' and then 'showcmdloc' set the command shows anyway.

The default value for 'showcmdloc' should be "last". Currently you could replace it with "statusline" or "tabline".
If we ever decide to show it in multiple places, it could be "last,statusline", for example.

Coding style: Please put the opening curly on the next line.

Done and additional tests added.

luukvbaal · 2022-12-14T10:43:57Z

Should the tests be in test_normal.vim instead? Existing showcmd test is in there.

brammool · 2022-12-14T13:50:53Z

Should the tests be in `test_normal.vim` instead? Existing `showcmd` test is in there.

test_normal.vim is already very big. Perhaps we have enough testing for this feature to justify a new test file? In the past we thought the overhead of starting a new Vim instance would matter, but now it matters more that if a test fails, e.g. there is a memory leak, it is limited in what might cause it. Otherwise, test_statusline.vim would also be related.

…

-- E M A C S s e l o h c t t n i a a t f p r t e o l /// 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 ///

luukvbaal · 2022-12-14T14:04:46Z

test_normal.vim is already very big. Perhaps we have enough testing for this feature to justify a new test file? In the past we thought the overhead of starting a new Vim instance would matter, but now it matters more that if a test fails, e.g. there is a memory leak, it is limited in what might cause it. Otherwise, test_statusline.vim would also be related.

Yeah depends on whether you want to group these tests under 'showcmd', in which case a test_showcmd.vim might be warranted. Currently I have placed the showcmdloc=statusline test in test_statusline.vim and the showcmdloc=tabline in tabline.vim, your call.

luukvbaal · 2022-12-14T15:41:40Z

I wonder if adding a builtin showcmd() function, analogous to e.g. mode() and searchcount() will be more useful/idiomatic than adding a statusline item?

brammool · 2022-12-15T12:50:48Z

I wonder if adding a builtin `showcmd()` function, analogous to e.g. `mode()` and `searchcount()` will be more useful/idiomatic than adding a statusline item?

It looks like it is hardly any code. We can just do both. Let me include it now. Then we can have some users try it out and see if we need some adjustments.

…

-- You had connectors? Eeee, when I were a lad we 'ad to carry the bits between the computer and the terminal with a spoon... /// 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 ///

Problem: Cannot display 'showcmd' somewhere else. Solution: Add the 'showcmdloc' option. (Luuk van Baal, closes vim/vim#11684) vim/vim@ba936f6 Co-authored-by: Luuk van Baal <luukvbaal@gmail.com>

luukvbaal force-pushed the statusline branch from 3f37edc to 4a1e041 Compare December 9, 2022 13:30

luukvbaal force-pushed the statusline branch 2 times, most recently from a500f6b to 27be343 Compare December 9, 2022 17:38

luukvbaal force-pushed the statusline branch from 27be343 to 29c0e80 Compare December 9, 2022 17:48

luukvbaal force-pushed the statusline branch from 29c0e80 to 56879bd Compare December 10, 2022 18:25

luukvbaal changed the title ~~Add 'showcmd' statusline item~~ Add 'showcmd' statusline item and 'showcmdloc' option Dec 10, 2022

luukvbaal force-pushed the statusline branch from 56879bd to 933b7b3 Compare December 10, 2022 18:29

luukvbaal force-pushed the statusline branch from 933b7b3 to e182aee Compare December 11, 2022 01:06

luukvbaal force-pushed the statusline branch from e182aee to 0c887fd Compare December 11, 2022 14:00

luukvbaal mentioned this pull request Dec 11, 2022

feat: add 'showcmdloc' option and statusline item neovim/neovim#21202

Merged

luukvbaal force-pushed the statusline branch from 0c887fd to 2673ca4 Compare December 12, 2022 00:57

luukvbaal force-pushed the statusline branch from 2673ca4 to f9e9691 Compare December 13, 2022 14:50

luukvbaal commented Dec 13, 2022

View reviewed changes

src/buffer.c Show resolved Hide resolved

luukvbaal force-pushed the statusline branch from f9e9691 to e5dc80d Compare December 13, 2022 15:10

Add 'showcmd' statusline item and 'showcmdloc' option

1d5eacf

luukvbaal force-pushed the statusline branch from e5dc80d to 1d5eacf Compare December 14, 2022 00:10

brammool closed this in ba936f6 Dec 15, 2022

bvngee mentioned this pull request Nov 3, 2023

Flickering with "%S" vim statusline component nvim-lualine/lualine.nvim#1129

Open

Uh oh!

Conversation

luukvbaal commented Dec 9, 2022

Uh oh!

zeertzjq commented Dec 9, 2022

Uh oh!

codecov bot commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

brammool commented Dec 9, 2022

Uh oh!

luukvbaal commented Dec 9, 2022

Uh oh!

luukvbaal commented Dec 9, 2022

Uh oh!

brammool commented Dec 9, 2022 via email

Uh oh!

luukvbaal commented Dec 9, 2022

Uh oh!

brammool commented Dec 10, 2022 via email

Uh oh!

luukvbaal commented Dec 10, 2022

Uh oh!

brammool commented Dec 10, 2022 via email

Uh oh!

brammool commented Dec 10, 2022 via email

Uh oh!

luukvbaal commented Dec 10, 2022

Uh oh!

luukvbaal commented Dec 11, 2022

Uh oh!

luukvbaal commented Dec 12, 2022

Uh oh!

brammool commented Dec 13, 2022

Uh oh!

Uh oh!

luukvbaal commented Dec 13, 2022

Uh oh!

luukvbaal commented Dec 14, 2022

Uh oh!

brammool commented Dec 14, 2022 via email

Uh oh!

luukvbaal commented Dec 14, 2022

Uh oh!

luukvbaal commented Dec 14, 2022

Uh oh!

brammool commented Dec 15, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 9, 2022 •

edited

Loading