external UI: tabline #6583

justinmk · 2017-04-25T01:32:28Z

Continues #6170

Now that :help api-contract formalizes how we can grow the API, we can accelerate UI widget "externalization". We don't need to perfectly implement the UI events because :help api-contract allows for adding new items to existing events.

#6170 is simple enough that it's worth including in 0.2.

Todo (future):

tabline click events (StlClickRecord) aren't handled by this change. See [RDY] allow external ui to draw tabline #6170 (comment)

cc @bfredl

justinmk · 2017-04-25T02:13:08Z

runtime/doc/msgpack_rpc.txt

@@ -264,10 +264,13 @@ a dictionary with these (optional) keys:
 				colors.
 				Set to false to use terminal color codes (at
 				most 256 different colors).
-	`popupmenu_external`	Instead of drawing the completion popupmenu on


Un-document popupmenu_external but continue to support it. ~~We can try removing it on master after 0.2 and see if it breaks anything.~~

Do you mean why remove it? I am not sure if anyone is actually using it, yet. OTOH 0.1.7 will be in debian stable, so yeah it's not a good idea to remove it.

bfredl · 2017-04-25T06:43:56Z

src/nvim/api/ui.c

    if (value.type != kObjectTypeBoolean) {
      api_set_error(error, kErrorTypeValidation,
                    "popupmenu_external must be a Boolean");
      return;
    }
-    ui->pum_external = value.data.boolean;
+    ui->ui_ext[kUIPopupmenu] = value.data.boolean;
+  } else if (strequal(name.data, "ui_ext")) {


Why reimplement ui options inside ui options? Inner platform effect?

Would you prefer a bunch of ext_foo fields? I'm fine with that, I suppose, the pesudo-namespacing seemed like a signal that they shouldn't be separate options. Also the documentation for each one will be very redundant.

Flattened the options. I'm generally an advocate of avoiding needless hierarchy :)

Because ui options was introduced for smart ui (that is, external widgets) in the first place, so it seems weird to introduce a sub-option for smart ui options... It seems we will end up with two options, rgb, and ui_ext containing the rest of the options....

(note I'm talking of the external interface, I'm not against ui->ui_ext)

bfredl · 2017-04-25T06:48:57Z

src/nvim/screen.c

+    win_T *cwp = (tp == curtab) ? curwin : tp->tp_curwin;
+    get_trans_bufname(cwp->w_buffer);
+    Array tab = ARRAY_DICT_INIT;
+    ADD(tab, INTEGER_OBJ(tp->handle));


TABPAGE_OBJ

If I do that, the test result looks like this:

[tab] = { [id] = '' }

That is purely a limit of neovim-lua objects not printing themselves properly. This only affects debug output, not if the test passes or not. Compare with nvim.Tabpage.new(1) etc.

or compare against objects from nvim('list_tabpages'), this is what the API tests do.

Thanks. Seems awkward. How does it help clients?

It is awkward due to limitations in neovim-lua. In the python-client (and similarly in other plugin hosts) you get a useful object you can call methods on, assign to nvim.current.tabpage etc. The lua client should at least be fixed to decode the id so that it prints like {[id]=1}.

see neovim/lua-client#31

@bfredl Thanks! That worked nicely.

bfredl · 2017-04-25T06:50:26Z

Is nvim_set_current_tabpage roughly equivalent to clicking on the tabline? If we provide no other means, external ui:s are going to use it.

bfredl · 2017-04-25T08:22:36Z

It is a problem though that ui options aren't introspectible. It will be an error if a newer gui tries to enable a widget in an older nvim. Maybe the metadata should contain the supported ui options?

justinmk · 2017-04-25T08:25:22Z

nvim_set_current_tabpage and ex_tabnext both resolve to goto_tabpage_tp so I think we're ok there. However there's still the mouse-click issue mentioned in #6583 (comment).

It will be an error if a newer gui tries to enable a widget in an older nvim.

Yeah, I thought about silently ignoring unknown options but I think it's better for UIs to explicitly check api_info.version.

Maybe the metadata should contain the supported ui options

That's also a good idea, though in practice most clients probably are fine with checking api_info.version.

bfredl · 2017-04-25T08:30:33Z

That will work for releases, but not for master version. Yes, users of master gui/plugins should use master nvim, but it nice to be able to easily avoid one annoying error.

bfredl · 2017-04-25T08:41:30Z

Supporting 'tabline' in general is going to be tricky. But as this PR uses a dict, it can be done backwards-compatible later on.

bfredl · 2017-04-25T08:57:55Z

src/nvim/screen.c

+  FOR_ALL_TABS(tp) {
+    win_T *cwp = (tp == curtab) ? curwin : tp->tp_curwin;
+    get_trans_bufname(cwp->w_buffer);
+    Array tab = ARRAY_DICT_INIT;


this array seems redundant, can't we have a tabpage field in the dict?

justinmk · 2017-04-26T00:43:44Z

Updated, much better thanks to suggestions.

justinmk · 2017-04-26T01:01:50Z

src/nvim/screen.c

+  if (ui_is_external(kUITabline)) {
+    ui_ext_tabline_update();
+    return;
+  }


@dzhou121 I moved this below redraw_tabline = false to avoid unnecessary draw_tabline() calls. Did you have it swapped on purpose?

No, it wasn't on purpose. It should be below redraw_tabline = false

- Work with a bool[] array parallel to the UIWidget enum. - Rename some functions. - Documentation.

dzhou121 · 2017-04-27T07:43:47Z

src/nvim/screen.c

+  Array tabs = ARRAY_DICT_INIT;
+  FOR_ALL_TABS(tp) {
+    Dictionary tab_info = ARRAY_DICT_INIT;
+    PUT(tab_info, "tab", TABPAGE_OBJ(tp->handle));


@justinmk I just tested this on my remote ui. But TABPAGE_OBJ seems to break the msgpack, i.e., I can't decode the msgpack messages properly. And it's fine when I change it back to INTEGER_OBJ.

You need an msgpack library capable of extension types. The EXT value can either be sent as-is to any method expecting a tabpage, or the internal data can be decoded as a msgpack INT.

I'm using github.com/neovim/go-client, does it support it?

Hmm, looks like it should...

@dzhou121 could you report at the go client repo?

Thanks for the info. Have reported it at the go client repo.

FEATURES: bc4a2e1 help, man.vim: "outline" (TOC) feature neovim#5169 58422f1 'guicursor' works in the TUI (and sends info to UIs) neovim#6423 129f107 api: nvim_get_mode() neovim#6247 0b59f98 api/ui: externalize tabline neovim#6583 bc6d868 'listchars': `Whitespace` highlight group neovim#6367 6afa7d6 writefile() obeys 'fsync' option neovim#6427 c60e409 eval.c refactor (also improves some error messages) neovim#5119 9d200cd getcompletion("cmdline") neovim#6376 2ea7bfc terminal: Support extra arguments in 'shell'. neovim#4504 bf51102 DirChanged autocmd neovim#5928 neovim#6262 1743df8 'cpoptions': "_" flag to toggle `cw` behaviour neovim#6235 22337b1 CTRL-R omits trailing ^M when pasting to cmdline neovim#6137 0e44916 :edit allows unescaped spaces in filename neovim#6119 abdbfd2 eval: Add id() function and make printf("%p") useful neovim#6095 bdfa147 findfile(), :find, gf work in :terminal. neovim#6009 2f38ed1 providers: Disable if `g:loaded_*` exists. b5560a6 setpos() can set lowercase marks in other buffers neovim#5753 7c513d6 Throttle :! output, pulse "..." message. neovim#5396 d2e8c76 v:exiting neovim#5651 :terminal improvements neovim#6185 neovim#6142 - cursor keeps position after leaving insert-mode. - 4ceec30 Follows output only if cursor is at end of buffer. - e7bbd35 new option: 'scrollback' - fedb844 quasi-support for undo and 'modifiable' - b45ddf7 disables 'list' by default - disables 'relativenumber' by default :help now contains full API documentation at `:help api`. man.vim saw numerous improvements. Windows support: - Windows is no longer "experimental", it is fully supported. - Windows package includes a GUI, curl.exe and other utilities. "Vim 8" features: partials, lambdas, packages. FIXES: 12fc1de ops: fix i<c-r> with multi-byte text neovim#6524 dd391bf Windows: system() and friends neovim#6497 13352c0 Windows: os_get_hostname() neovim#6413 16babc6 tui: Less-noisy mouse seqs neovim#6411 3a9dd13 (vim bug) folding edge-cases neovim#6207 f6946c6 job-control: set CLOEXEC on pty processes. neovim#5986 d1afd43 rplugin: Call s:LoadRemotePlugins() on startup. 1215084 backtick-expansion works with `shell=fish` neovim#6224 e32ec03 tui: Improved behavior after resize. neovim#6202 86c2adc edit.c: CTRL-SPC: Insert previously-inserted text. neovim#6090 c318d8e b:changedtick now follows VimL rules neovim#6112 34e24cb terminal: Initialize colors in reverse order neovim#6160 e889917 undo: Don't set b_u_curhead in ex_undojoin() neovim#5869 d25649f undo: :earlier, g-: Set b_u_seq_cur correctly. (neovim#6016) 043d8ba 'Visual-mode put from @. register' neovim#5782 42c922b open_buffer(): Do `BufEnter` for directories. 50d0d89 inccommand: Preview :sub commands only after delimiter neovim#5932 1420e10 CheckHealth improvements neovim#5519 c8d5e92 jobstart(): Return -1 if cmd is not executable. neovim#5671 CHANGES: NVIM_TUI_ENABLE_CURSOR_SHAPE was removed. Use 'guicursor' instead. See https://github.com/neovim/neovim/wiki/Following-HEAD#20170402 81525dc 'mouse=a' is no longer the default. (This will probably change again after it is improved.) neovim#6022 0c1f783 defaults: 'showcmd', 'belloff', 'ruler' neovim#6087 eb0e94f api: {get,set}_option update local options as appropriate neovim#6405 bdcb2a3 "Reading from stdin..." message was removed. neovim#6298

FEATURES: bc4a2e1 help, man.vim: "outline" (TOC) feature neovim#5169 58422f1 'guicursor' works in the TUI (and sends info to UIs) neovim#6423 129f107 api: nvim_get_mode() neovim#6247 0b59f98 api/ui: externalize tabline neovim#6583 bc6d868 'listchars': `Whitespace` highlight group neovim#6367 6afa7d6 writefile() obeys 'fsync' option neovim#6427 c60e409 eval.c refactor (also improves some error messages) neovim#5119 9d200cd getcompletion("cmdline") neovim#6376 2ea7bfc terminal: Support extra arguments in 'shell'. neovim#4504 bf51102 DirChanged autocmd neovim#5928 neovim#6262 1743df8 'cpoptions': "_" flag to toggle `cw` behaviour neovim#6235 22337b1 CTRL-R omits trailing ^M when pasting to cmdline neovim#6137 0e44916 :edit allows unescaped spaces in filename neovim#6119 abdbfd2 eval: Add id() function and make printf("%p") useful neovim#6095 bdfa147 findfile(), :find, gf work in :terminal. neovim#6009 2f38ed1 providers: Disable if `g:loaded_*` exists. b5560a6 setpos() can set lowercase marks in other buffers neovim#5753 7c513d6 Throttle :! output, pulse "..." message. neovim#5396 d2e8c76 v:exiting neovim#5651 :terminal improvements neovim#6185 neovim#6142 - cursor keeps position after leaving insert-mode. - 4ceec30 Follows output only if cursor is at end of buffer. - e7bbd35 new option: 'scrollback' - fedb844 quasi-support for undo and 'modifiable' - b45ddf7 disables 'list' by default - disables 'relativenumber' by default :help now contains full API documentation at `:help api`. man.vim saw numerous improvements. Windows support: - Windows is no longer "experimental", it is fully supported. - Windows package includes a GUI, curl.exe and other utilities. "Vim 8" features: partials, lambdas. SECURITY FIXES: CVE-2017-5953 CVE-2017-6349 CVE-2017-6350 neovim#6485 CHANGES: NVIM_TUI_ENABLE_CURSOR_SHAPE was removed. Use 'guicursor' instead. See https://github.com/neovim/neovim/wiki/Following-HEAD#20170402 81525dc 'mouse=a' is no longer the default. (This will probably change again after it is improved.) neovim#6022 0c1f783 defaults: 'showcmd', 'belloff', 'ruler' neovim#6087 eb0e94f api: {get,set}_option update local options as appropriate neovim#6405 bdcb2a3 "Reading from stdin..." message was removed. neovim#6298 FIXES: 12fc1de ops: fix i<c-r> with multi-byte text neovim#6524 dd391bf Windows: system() and friends neovim#6497 13352c0 Windows: os_get_hostname() neovim#6413 16babc6 tui: Less-noisy mouse seqs neovim#6411 3a9dd13 (vim bug) folding edge-cases neovim#6207 f6946c6 job-control: set CLOEXEC on pty processes. neovim#5986 d1afd43 rplugin: Call s:LoadRemotePlugins() on startup. 1215084 backtick-expansion works with `shell=fish` neovim#6224 e32ec03 tui: Improved behavior after resize. neovim#6202 86c2adc edit.c: CTRL-SPC: Insert previously-inserted text. neovim#6090 c318d8e b:changedtick now follows VimL rules neovim#6112 34e24cb terminal: Initialize colors in reverse order neovim#6160 e889917 undo: Don't set b_u_curhead in ex_undojoin() neovim#5869 d25649f undo: :earlier, g-: Set b_u_seq_cur correctly. (neovim#6016) 043d8ba 'Visual-mode put from @. register' neovim#5782 42c922b open_buffer(): Do `BufEnter` for directories. 50d0d89 inccommand: Preview :sub commands only after delimiter neovim#5932 1420e10 CheckHealth improvements neovim#5519 c8d5e92 jobstart(): Return -1 if cmd is not executable. neovim#5671

justinmk mentioned this pull request Apr 25, 2017

[RDY] allow external ui to draw tabline #6170

Closed

justinmk force-pushed the ui-tabline branch from d41f6ee to 761682d Compare April 25, 2017 02:03

justinmk commented Apr 25, 2017

View reviewed changes

justinmk force-pushed the ui-tabline branch from 761682d to fc99d82 Compare April 25, 2017 02:24

bfredl reviewed Apr 25, 2017

View reviewed changes

bfredl mentioned this pull request Apr 25, 2017

unpack object ids for easier introspection neovim/lua-client#31

Merged

api/ui: externalize tabline

88023d5

justinmk force-pushed the ui-tabline branch 2 times, most recently from f73b70c to 907a8b8 Compare April 26, 2017 00:39

justinmk force-pushed the ui-tabline branch 2 times, most recently from 5f88224 to 7bd63be Compare April 26, 2017 01:00

justinmk commented Apr 26, 2017

View reviewed changes

justinmk added 3 commits April 26, 2017 03:41

api/ui: externalize tabline

0084390

- Work with a bool[] array parallel to the UIWidget enum. - Rename some functions. - Documentation.

api: nvim_ui_attach(): Flatten ext_* options.

c8e1af9

api/ext_tabline: List of Dicts.

6944aba

justinmk force-pushed the ui-tabline branch from 7bd63be to 6944aba Compare April 26, 2017 08:59

justinmk merged commit 0b59f98 into neovim:master Apr 26, 2017

justinmk deleted the ui-tabline branch April 26, 2017 10:00

dzhou121 reviewed Apr 27, 2017

View reviewed changes

dzhou121 mentioned this pull request Apr 27, 2017

msgpack error when the event contains TABPAGE_OBJ neovim/go-client#15

Closed

bryphe mentioned this pull request Apr 27, 2017

UI: Externalize tab lines onivim/oni#361

Closed

justinmk mentioned this pull request May 1, 2017

NVIM v0.2.0 #6634

Merged

justinmk added the ui-extensibility UI extensibility, events, protocol, externalized UI label Jul 4, 2017

Uh oh!

external UI: tabline #6583

external UI: tabline #6583

Uh oh!

Conversation

justinmk commented Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmk Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl commented Apr 25, 2017

Uh oh!

bfredl commented Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmk commented Apr 25, 2017

Uh oh!

bfredl commented Apr 25, 2017

Uh oh!

bfredl commented Apr 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk commented Apr 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

justinmk commented Apr 25, 2017 •

edited

Loading

justinmk Apr 25, 2017 •

edited

Loading

justinmk Apr 25, 2017 •

edited

Loading

bfredl Apr 25, 2017 •

edited

Loading

bfredl commented Apr 25, 2017 •

edited

Loading