New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] External ui #5686

Open
wants to merge 30 commits into
base: master
from

Conversation

Projects
None yet
@dzhou121
Contributor

dzhou121 commented Nov 29, 2016

This pull request is inspired by the external popupmenu and try to address the same approach for windows.

This patch will make neovim to treat each window individually and will output cmdline, tab, wildmenu to API without drawing them.

It will only have effect when win_external is set by API so tui is still working in the old way.

The achieved result is shown in this screencast:
https://youtu.be/rzclz1seo0g

@marvim marvim added the RFC label Nov 29, 2016

@justinmk

This comment has been minimized.

Member

justinmk commented Nov 29, 2016

will output cmdline, tab, wildmenu to API without drawing them.

Nice. Should there be a different foo_exernal flag for each of them? Currently there's only win_external. @bfredl do you think we should continue to have "granular" opt-in externalized widgets?

@dzhou121 Do you plan to add tests?

@bfredl

This comment has been minimized.

Member

bfredl commented Nov 29, 2016

Very interesting! I will need to take a deeper look into this, for instance how does this integrate with window ^W commands?

But at a quick glance, It looks like we could have a separate cmd_msg_external that makes messages and command line (including wildmode) external without enabling separate windows. (it should only free the last row in the grid as if cmdheight=0). win_external will then simply imply cmd_msg_external.

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Nov 29, 2016

@justinmk @bfredl

This patch's aim is to external everything that makes sense to be externalised. So probably win_external should be changed to ui_external.

Re window ^W commands, it still let neovim to handle window layout management. But the external API will know the windows' sizes, positions, etc. So the most direct benefit is the line between vertical splits.

@justinmk

This comment has been minimized.

Member

justinmk commented Nov 29, 2016

There's still the minor question: do we want to allow UIs to opt-in to externalizing only certain pieces? If we put everything behind a single ui_external flag then that means UIs must implement all externalized widgets instead of only some. If it makes the code/logic significantly simpler then I would lean towards "no granularity". But if it's low-cost to provide granularity then it might be worth doing so. cc @rhysd @qvacua @rogual @carlosdcastillo

@teto

This comment has been minimized.

Contributor

teto commented Nov 29, 2016

very cool ! I vote for granularity since it should be simple to provide (bitfield like) while adding a convenience: easier to provide a WIP UI, if some widget is buggy, the dev can disable it in favor of the native etc...

@qvacua

This comment has been minimized.

qvacua commented Nov 29, 2016

I agree with @teto. The external popup menu does not have a very high priority for me and it'd be nice if it'd be an opt-in such that the "text"-based popup menus would continue to work.

@chemzqm

This comment has been minimized.

Contributor

chemzqm commented Dec 1, 2016

I do think it's better to just have a single ui_external flag, so that the GUI author could make sure to build a consist UI , instead of part of them, which would be confusing for the user, and also affect building the theme for GUI.

I also think the previewWindow should be ui_external, it would useful to avoid redraw when completing.

dzhou121 added some commits Dec 2, 2016

Added new command fnew to create a floating window
The floating window is useful for plugins like
Crtl-P
@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Dec 2, 2016

I've added floating windows support. And preview-window can be an opt in. You can make it floating or still let Neovim to handle the window layout for it.

But when you external window layout, you imply external cmdline, tabline as well, because the screen rendering happens at window level. So there's no "screen" for cmdline, tabline any more.

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Dec 2, 2016

https://www.youtube.com/watch?v=UgS-B12E0ps

This screencast demonstrates the floating windows capability:

The error information of the syntax is displayed in a floating preview-window and positioned at the cursor by the GUI.

The FZF plugin is using the floating window that can be centered by the GUI.

@teto

This comment has been minimized.

Contributor

teto commented Dec 2, 2016

The floating window support seems very interesting, While I don't think it can help the Window Manager handle windows instead of nvim (#2161), I wonder if it could be used to provide UI-enhanced quickfix/location lists as displayed in https://a.fsdn.com/con/app/proj/texstudio/screenshots/errorHighlighting.png . I would love to have a button to toggle warning messages (I have a QFGrep mapping for that but sometimes buttons are a nice to have as well)?

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Dec 2, 2016

@teto

We could have a new wincmd command that can cycle through all floating windows. Something like

wincmd fnext

@teto

This comment has been minimized.

Contributor

teto commented Dec 2, 2016

As you seem pretty fired up, I wonder if your UI display vim "menus" in any special way ?
I think that :emenu completion relies on wildmode so if you support wildmode that should cover it (I find the vim-menus quite useful to run rare commands I am sure to forget, yet it's not popular with plugins :/).

To reproduce your screencast, is using https://github.com/dzhou121/envim with this PR enough ?

@justinmk justinmk referenced this pull request Jan 11, 2017

Open

Fold improvements discussion #5931

3 of 8 tasks complete
@acornejo

This comment has been minimized.

acornejo commented Jan 24, 2017

I am also curious as to what are the required steps to get this merged. It seems a pitty not to let others enjoy dzhou's work.

@timeyyy

This comment has been minimized.

Contributor

timeyyy commented Feb 2, 2017

@justinmk How can we make this easier for reviewers to digest? Do we want a p/r per externalized feature?

Following the pattern of #4432 might be worthwhile.
commits like

api/ui exteranlize x
api/ui: add tests for externalizing x
api/ui: add documentation for externalizing x

I don't think master is so stable so maybe rebasing onto e7de3b5 for the time being

@justinmk

This comment has been minimized.

Member

justinmk commented Feb 7, 2017

Keeping this rebased on master would be really helpful. @dzhou121 do you plan to write tests for this?

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Feb 22, 2017

@justinmk

I'll write tests once I know how should I proceed. Do I split the features into multiple PRs or still keep them in this one PR?

@justinmk

This comment has been minimized.

Member

justinmk commented Feb 22, 2017

@dzhou121 If you can split the features that would be extremely helpful, and will allow us to proceed much more quickly.

@teto

This comment has been minimized.

Contributor

teto commented Feb 22, 2017

For what is worth, I would vote to externalize command line first as there are some work on providing a sublime-like palette and the 2 combined together would be very cool.

I had a look at the code and dropping the global variables ScreenLines etc might lead to an easier refactoring of the monstreous win_line function.

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Feb 23, 2017

Just submitted a new PR #6162 for cmdline_external

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Feb 24, 2017

PR #6168 for wildmenu_external

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Feb 24, 2017

PR #6170 for tabline_external

@autozimu

This comment has been minimized.

autozimu commented Feb 24, 2017

PR #6170 for cmdline_external

There seems to be a typo here. Maybe tabline_external?

@dzhou121

This comment has been minimized.

Contributor

dzhou121 commented Feb 25, 2017

@autozimu Thanks for the note. Have edited it.

@justinmk justinmk added the ui-ext label Oct 29, 2017

justinmk added a commit that referenced this pull request Oct 31, 2017

@justinmk

This comment has been minimized.

Member

justinmk commented Oct 31, 2017

I believe everything here except floating windows (#6619) has been merged to master. @dzhou121 if there's any thing else here can you extract it to a new PR?

@justinmk justinmk added this to the 0.3 milestone Oct 31, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented Oct 31, 2017

There is still externalizing echo and error messages. It will be needed before external window layout.

@chandlerc

This comment has been minimized.

chandlerc commented Jul 7, 2018

Is anyone continuing to work on this?

@teto

This comment has been minimized.

Contributor

teto commented Jul 7, 2018

@chandlerc yes some developers are working on this as can be read in the linked PRs

@bfredl

This comment has been minimized.

Member

bfredl commented Jul 7, 2018

See also #8455 for per-window grids (which also will go a bit further than this, and let clients handle all wincmd:s if they want to)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment