Feedback about `:` command pallet.

[jordwalke]

Oni Version:0.3.4 OSX

I noticed that the : command pallet is kind of slow. The other command pallets (Like the recent files pallet) are faster.

Also, the : command pallet deactivates the currently active tab's highlight which is kind of distracting.

[bryphe]

Thanks for the feedback, @jordwalke !

cc @Akin909 - Akin did the majority of the externalization / React work here so he might have some ideas.

The : menu actually has a different architecture than the other menus (like quick open / command palette) - all the UI is round-tripped to Neovim (and there isn't any typing prediction today IIRC) - so it'd be worth profiling the latency from keypress -> rendering.

There are several potential bottlenecks along that pipeline. Another potential bottleneck could be the wildmenu results we get from Neovim - might be some cases where we are rendering a lot of items.

[akinsho]

@bryphe @jordwalke my first thought would be whether or not you have any cabbrev mappings or commandline mappings as those can have a timeout I think. If the slowness is due to the speed at which the ui appears then theres an 80ms wait which is there to prevent flickering if for example navigating between buffers using a mapping e.g. bnext in quick succession, without it the ui would visibly appear and disappear, which is what actually happens in neovim but due to placement of the commandline is less noticeable.

As for the round trip we don't actively make any calls to neovim, we just listen for commandline events (along with all the other neovim ones) and put the results in redux then read them out, there's no processing or bottle neck along that path other than perhaps the time it takes neovim to register the action and dispatch the event we listen for. I think a profile would be really helpful I tried a quick comparison between my terminal nvim setup and oni, and don't get a discernible difference but could just be fudging it on my end.

As for the highlighting thats definitely a bug looking at the reducer theres no obvious relationship between them but we do depend on an active bufferId when highlighting the buffer's tab and its possible that that is set to null when the commandline is open 🤔

[jordwalke]

I took a look at the timing chart and I think it could be a combination of:

1. The fact that there's an 80ms ease-in animation.
2. The dialog is created and destroyed every time you hit :. Would it be possible to avoid creating/destroying those DOM nodes and simply hide it? (That might be beneficial for all of the modal UI dialogs). That way, there would be fewer things in between the keystrokes and showing the UI.

[justinmk]

theres an 80ms wait which is there to prevent flickering if for example navigating between buffers using a mapping e.g. bnext in quick succession, without it the ui would visibly appear and disappear, which is what actually happens in neovim but due to placement of the commandline is less noticeable.

Maybe this should be avoided in Nvim core. @bfredl any reason we couldn't?

[bfredl]

@justinmk we already avoid sending temporary cmdline states when there are no spurious ui_flush events. So the question is where during the mapping is ui_flush() invoked and could this be avoided.

[bfredl]

Also as a temporary workaround : can be replaced with <Cmd> in any mapping to avoid showing the cmdline

[akinsho]

@jordwalke we could definitely look at hiding the elements rather than creating a new one each time, re the wait @justinmk that would be amazing if its possible as for the work around using <Cmd> would that be a change in nvim or oni? (I'm assuming nvim as in oni currently we don't really interact with users nvim bindings at all)

[justinmk]

The <Cmd> workaround is per-mapping (i.e. it's up to the user or whoever defines the mapping). E.g. instead of:

nnoremap x :echo 'foo'<cr>

do this:

nnoremap x <cmd>echo 'foo'<cr>

See :help <cmd> (Nvim 0.3).

[CrossR]

If the highlight is the little colour tab on the top of the current tab, I think that is to do with the mode changing and us not having defined a fallback/custom mode for the cmdline.

That is, this code:

https://github.com/onivim/oni/blob/7c09bcf76cf3cf7fc61d39c99b550f711f9a3d04/browser/src/UI/components/Tabs.tsx#L256-L265

uses the current mode to get the highlight colour for the current tab, but doing : swaps us from normal mode to cmdline_normal, so it ends up defaulting to transparent.

We'd need to either special case this to re-use the normal mode highlight, or define new highlight.mode.cmdline_normal.background in our themes.

[bfredl]

Thinking of it more, doesn't <silent> mapping avoid showing the cmdline (while also being vim compatible unlike <cmd>) ? if not, it is an obvious nvim bug.

but doing : swaps us from normal mode to cmdline_normal

This should definitely be disabled in nvim core if ext_cmdline is enabled (as TUI-like cursor is no longer involved with cmdline at all)

[justinmk]

People often don't use <silent> in mappings though--they won't notice anything that gets clobbered by a redraw.

I guess this goes back to the idea of "collapsing" redraws more aggressively in Nvim core.

[bfredl]

Indeed, I use <silent> in my plugins because it is sort of expected (and hides implementation details), but not in my vimrc because I in fact like the extra visual feedback.

[akinsho]

I think we haven't quite tested whether the timeout is a valuable tradeoff escpecially as it makes the ui appear sluggish not sure what you think @bryphe but I think removing the timeout and trying to pre-empt any issues by adding documentation to suggest that users use <Cmd> for commandline mappings would be preferable?

@CrossR thats a great find 👍, we special case showmatch I personally don't think theres much value in having the tab state change to a different color because the commandline is open I don't think the significance would filter through or that there'd be value there even if it did.

I'm looking into removing the cmdline timeout so can add a special case for the commandline as well to just render the same color as normal mode.

[jordwalke]

To be clear the delay I saw was not with key mappings to :cmd, but when I hit : explicitly to open the command bar. Are there also additional delays in keymaps that map to : commands? (Solved by <Cmd>?

[akinsho]

@jordwalke the delay re hitting : was/is due to the 80ms wait (i've opened #2334 which removes that) re the <Cmd> that workaround is aimed at alleviating a ui flickering scenario the wait was aimed at. With the wait removed if a user has e.g. :bnext mapped and wanted to moved through buffers quickly hitting the mapping several times or a similar tab mapping the commandline would appear and disappear every time very briefly but with the Cmd workaround that doesn't happen. If #2334 goes in we will have to be clear that if anyone is hitting the flicker bug they would have to change their mapping(s) to be <Cmd>bnext which would solve the issue

[jordwalke]

I can see there's some tradeoffs with removing the delay. Hopefully this can be fixed at a deeper level but I agree with your point that people with command mappings would be okay adjusting them to avoid UI delay. Does that delay currently impact mappings like map <c-l> :echo 'hi'<Cr>. Do those mappings experience a delay? Or merely showing of the command line?

I think this is one interaction that's so important it could be special cased one way or another. Have you considered having : be a special Oni mapping (customizable of course) which would instantly render that modal if not in insert mode, and then in 80ms sync back up with Neovim?

[akinsho]

Yes would be tradeoff but I think it's not likely to be as much of an issue as I initially imagined and importantly we've got a workaround, re the command you raised that would hit the delay in showing the command line.

Re oni based mappings I think theres still a big gap between oni mappings and neovim in terms of there being no real relationship so special casing a : or having oni handle it differently would take quite a bit of leg work.

On the other hand I've been running the PR branch without the timeout for a few days now and tbh I have the odd mapping with : and don't really hit the flicker although I changed a couple of mappings to use the <Cmd> workaround (but the ones I noticed I was using quite frequently)

[jordwalke]

Is <Cmd> a neovim specific feature? I share a lot of my vimrc config with vim and neovim/Oni. Would I have to do some if/else checks to ensure I only use <Cmd> inside of Oni?

[akinsho]

@jordwalke I believe it is new in the latest release of nvim, but another alternative which was mentioned which works both on vim and nvim is <silent> ~tbh with #2334~, but yeah I think use of <Cmd> probably needs a guard around it or perhaps just using silent ought to be the recommendation

[akinsho]

@bryphe @CrossR After having the commandline changes on master for a couple of days seems it might need to be reverted as it seems its not just down to the user changing their bindings I have a few plugins which seem to use commandline mappings without silent causing the flicker whilst using those meaning a user is unlikely to be able control the flickering in those cases not sure how big an issue it might be we could always wait for feedback after the next release but I think its going to prove to be too much of an annoyance for ppl