ui: refactor ui options

11 files changed, 109 insertions, 45 deletions

diff --git a/runtime/doc/api.txt b/runtime/doc/api.txt
index fd6918de43..f828f2cdc1 100644
--- a/runtime/doc/api.txt
+++ b/runtime/doc/api.txt
@@ -49,6 +49,7 @@ version.api_prerelease	Declares the current API level as unstable >
 			(version.api_prerelease && fn.since == version.api_level)
 functions		API function signatures
 ui_events		UI event signatures |ui|
+ui_options		Supported |ui-options|
 {fn}.since		API level where function {fn} was introduced
 {fn}.deprecated_since	API level where function {fn} was deprecated
 types			Custom handle types defined by Nvim
diff --git a/runtime/doc/ui.txt b/runtime/doc/ui.txt
index 1b3c10b71e..6f6df2d7ae 100644
--- a/runtime/doc/ui.txt
+++ b/runtime/doc/ui.txt
@@ -30,10 +30,22 @@ a dictionary with these (optional) keys:
 	`ext_cmdline`		Externalize the cmdline. |ui-cmdline|
 	`ext_wildmenu`		Externalize the wildmenu. |ui-ext-wildmenu|
 
-Nvim will then send msgpack-rpc notifications, with the method name "redraw"
-and a single argument, an array of screen update events.
-Update events are tuples whose first element is the event name and remaining
-elements the event parameters.
+Specifying a non-existent option is an error. To facilitate an ui that
+supports different versions of Nvim, the |api-metadata| key `ui_options`
+contains the list of supported options. Additionally Nvim currently requires
+that all connected UIs use the same set of widgets. Therefore the active
+widgets will be the intersection of the requested widget sets of all connected
+UIs. The "option_set" event will be used to specify which widgets actually are
+active.
+
+After attaching, Nvim will send msgpack-rpc notifications, with the method
+name "redraw" and a single argument, an array of screen update events.  Update
+events are arrays whose first element is the event name and remaining elements
+are each tuples of event parameters. This allows multiple events of the same
+kind to be sent in a row without the event name being repeated. This batching
+is mostly used for "put", as each "put" event just puts contents in one screen
+cell, but clients must be prepared for multiple argument sets being batched
+for all event kinds.
 
 Events must be handled in order. The user should only see the updated screen
 state after all events in the same "redraw" batch are processed (not any
@@ -93,6 +105,7 @@ Global Events							    *ui-global*
 	'linespace'
 	'showtabline'
 	'termguicolors'
+	`ext_*` (all |ui-ext-options|)
 
 	Options are not added to the list if their effects are already taken
 	care of. For instance, instead of forwarding the raw 'mouse' option
diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c
index 9baa996560..65c306d4b1 100644
--- a/src/nvim/api/private/helpers.c
+++ b/src/nvim/api/private/helpers.c
@@ -26,6 +26,7 @@
 #include "nvim/version.h"
 #include "nvim/lib/kvec.h"
 #include "nvim/getchar.h"
+#include "nvim/ui.h"
 
 /// Helper structure for vim_to_object
 typedef struct {
@@ -955,6 +956,12 @@ static void init_ui_event_metadata(Dictionary *metadata)
   msgpack_rpc_to_object(&unpacked.data, &ui_events);
   msgpack_unpacked_destroy(&unpacked);
   PUT(*metadata, "ui_events", ui_events);
+  Array ui_options = ARRAY_DICT_INIT;
+  ADD(ui_options, STRING_OBJ(cstr_to_string("rgb")));
+  for (UIExtension i = 0; i < kUIExtCount; i++) {
+    ADD(ui_options, STRING_OBJ(cstr_to_string(ui_ext_names[i])));
+  }
+  PUT(*metadata, "ui_options", ARRAY_OBJ(ui_options));
 }
 
 static void init_error_type_metadata(Dictionary *metadata)
diff --git a/src/nvim/api/ui.c b/src/nvim/api/ui.c
index e53a45bd93..4870c3fb8a 100644
--- a/src/nvim/api/ui.c
+++ b/src/nvim/api/ui.c
@@ -176,18 +176,6 @@ void nvim_ui_set_option(uint64_t channel_id, String name,
 
 static void ui_set_option(UI *ui, String name, Object value, Error *error)
 {
-#define UI_EXT_OPTION(o, e) \
-  do { \
-    if (strequal(name.data, #o)) { \
-      if (value.type != kObjectTypeBoolean) { \
-        api_set_error(error, kErrorTypeValidation, #o " must be a Boolean"); \
-        return; \
-      } \
-      ui->ui_ext[(e)] = value.data.boolean; \
-      return; \
-    } \
-  } while (0)
-
   if (strequal(name.data, "rgb")) {
     if (value.type != kObjectTypeBoolean) {
       api_set_error(error, kErrorTypeValidation, "rgb must be a Boolean");
@@ -197,13 +185,21 @@ static void ui_set_option(UI *ui, String name, Object value, Error *error)
     return;
   }
 
-  UI_EXT_OPTION(ext_cmdline, kUICmdline);
-  UI_EXT_OPTION(ext_popupmenu, kUIPopupmenu);
-  UI_EXT_OPTION(ext_tabline, kUITabline);
-  UI_EXT_OPTION(ext_wildmenu, kUIWildmenu);
+  for (UIExtension i = 0; i < kUIExtCount; i++) {
+    if (strequal(name.data, ui_ext_names[i])) {
+      if (value.type != kObjectTypeBoolean) {
+        snprintf((char *)IObuff, IOSIZE, "%s must be a Boolean",
+                 ui_ext_names[i]);
+        api_set_error(error, kErrorTypeValidation, (char *)IObuff);
+        return;
+      }
+      ui->ui_ext[i] = value.data.boolean;
+      return;
+    }
+  }
 
   if (strequal(name.data, "popupmenu_external")) {
-    // LEGACY: Deprecated option, use `ui_ext` instead.
+    // LEGACY: Deprecated option, use `ext_cmdline` instead.
     if (value.type != kObjectTypeBoolean) {
       api_set_error(error, kErrorTypeValidation,
                     "popupmenu_external must be a Boolean");
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index e8f5477db0..3f4f17824b 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -49,7 +49,7 @@
 #define MAX_UI_COUNT 16
 
 static UI *uis[MAX_UI_COUNT];
-static bool ui_ext[UI_WIDGETS] = { 0 };
+static bool ui_ext[kUIExtCount] = { 0 };
 static size_t ui_count = 0;
 static int row = 0, col = 0;
 static struct {
@@ -246,8 +246,8 @@ void ui_refresh(void)
   }
 
   int width = INT_MAX, height = INT_MAX;
-  bool ext_widgets[UI_WIDGETS];
-  for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) {
+  bool ext_widgets[kUIExtCount];
+  for (UIExtension i = 0; (int)i < kUIExtCount; i++) {
     ext_widgets[i] = true;
   }
 
@@ -255,7 +255,7 @@ void ui_refresh(void)
     UI *ui = uis[i];
     width = MIN(ui->width, width);
     height = MIN(ui->height, height);
-    for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) {
+    for (UIExtension i = 0; (int)i < kUIExtCount; i++) {
       ext_widgets[i] &= ui->ui_ext[i];
     }
   }
@@ -267,8 +267,10 @@ void ui_refresh(void)
   screen_resize(width, height);
   p_lz = save_p_lz;
 
-  for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) {
-    ui_set_external(i, ext_widgets[i]);
+  for (UIExtension i = 0; (int)i < kUIExtCount; i++) {
+    ui_ext[i] = ext_widgets[i];
+    ui_call_option_set(cstr_as_string((char *)ui_ext_names[i]),
+                       BOOLEAN_OBJ(ext_widgets[i]));
   }
   ui_mode_info_set();
   old_mode_idx = -1;
@@ -527,15 +529,7 @@ void ui_cursor_shape(void)
 }
 
 /// Returns true if `widget` is externalized.
-bool ui_is_external(UIWidget widget)
+bool ui_is_external(UIExtension widget)
 {
   return ui_ext[widget];
 }
-
-/// Sets `widget` as "external".
-/// Such widgets are not drawn by Nvim; external UIs are expected to handle
-/// higher-level UI events and present the data.
-void ui_set_external(UIWidget widget, bool external)
-{
-  ui_ext[widget] = external;
-}
diff --git a/src/nvim/ui.h b/src/nvim/ui.h
index 60adcb974f..48896a6a3f 100644
--- a/src/nvim/ui.h
+++ b/src/nvim/ui.h
@@ -13,14 +13,22 @@ typedef enum {
   kUIPopupmenu,
   kUITabline,
   kUIWildmenu,
-} UIWidget;
-#define UI_WIDGETS (kUIWildmenu + 1)
+  kUIExtCount,
+} UIExtension;
+
+EXTERN const char *ui_ext_names[] INIT(= {
+  "ext_cmdline",
+  "ext_popupmenu",
+  "ext_tabline",
+  "ext_wildmenu"
+});
+
 
 typedef struct ui_t UI;
 
 struct ui_t {
   bool rgb;
-  bool ui_ext[UI_WIDGETS];  ///< Externalized widgets
+  bool ui_ext[kUIExtCount];  ///< Externalized widgets
   int width, height;
   void *data;
 #ifdef INCLUDE_GENERATED_DECLARATIONS
diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c
index b0f8905771..c936a17e52 100644
--- a/src/nvim/ui_bridge.c
+++ b/src/nvim/ui_bridge.c
@@ -67,7 +67,7 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler)
   rv->bridge.option_set = ui_bridge_option_set;
   rv->scheduler = scheduler;
 
-  for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) {
+  for (UIExtension i = 0; (int)i < kUIExtCount; i++) {
     rv->bridge.ui_ext[i] = ui->ui_ext[i];
   }
 
diff --git a/test/functional/api/version_spec.lua b/test/functional/api/version_spec.lua
index d23f058f69..7bf54c0d1e 100644
--- a/test/functional/api/version_spec.lua
+++ b/test/functional/api/version_spec.lua
@@ -1,6 +1,7 @@
 local helpers = require('test.functional.helpers')(after_each)
 local mpack = require('mpack')
 local clear, funcs, eq = helpers.clear, helpers.funcs, helpers.eq
+local call = helpers.call
 
 local function read_mpack_file(fname)
   local fd = io.open(fname, 'rb')
@@ -18,7 +19,7 @@ describe("api_info()['version']", function()
   before_each(clear)
 
   it("returns API level", function()
-    local version = helpers.call('api_info')['version']
+    local version = call('api_info')['version']
     local current = version['api_level']
     local compat  = version['api_compatible']
     eq("number", type(current))
@@ -27,7 +28,7 @@ describe("api_info()['version']", function()
   end)
 
   it("returns Nvim version", function()
-    local version = helpers.call('api_info')['version']
+    local version = call('api_info')['version']
     local major   = version['major']
     local minor   = version['minor']
     local patch   = version['patch']
@@ -147,3 +148,14 @@ describe("api functions", function()
   end)
 
 end)
+
+describe("ui_options in metadata", function()
+  it('are correct', function()
+    -- TODO(bfredl) once a release freezes this into metadata,
+    -- instead check that all old options are present
+    local api = helpers.call('api_info')
+    local options = api.ui_options
+    eq({'rgb', 'ext_cmdline', 'ext_popupmenu',
+        'ext_tabline', 'ext_wildmenu'}, options)
+  end)
+end)
diff --git a/test/functional/eval/api_functions_spec.lua b/test/functional/eval/api_functions_spec.lua
index fea4a87a26..6f440c7d82 100644
--- a/test/functional/eval/api_functions_spec.lua
+++ b/test/functional/eval/api_functions_spec.lua
@@ -106,7 +106,8 @@ describe('api functions', function()
 
   it('have metadata accessible with api_info()', function()
     local api_keys = eval("sort(keys(api_info()))")
-    eq({'error_types', 'functions', 'types', 'ui_events', 'version'}, api_keys)
+    eq({'error_types', 'functions', 'types',
+        'ui_events', 'ui_options', 'version'}, api_keys)
   end)
 
   it('are highlighted by vim.vim syntax file', function()
diff --git a/test/functional/eval/msgpack_functions_spec.lua b/test/functional/eval/msgpack_functions_spec.lua
index 258d6ee059..a8a413f68b 100644
--- a/test/functional/eval/msgpack_functions_spec.lua
+++ b/test/functional/eval/msgpack_functions_spec.lua
@@ -463,7 +463,8 @@ describe('msgpackparse() function', function()
     eval(cmd)
     eval(cmd)  -- do it again (try to force segfault)
     local api_info = eval(cmd)  -- do it again
-    eq({'error_types', 'functions', 'types', 'ui_events', 'version'}, api_info)
+    eq({'error_types', 'functions', 'types',
+        'ui_events', 'ui_options', 'version'}, api_info)
   end)
 
   it('fails when called with no arguments', function()
diff --git a/test/functional/ui/options_spec.lua b/test/functional/ui/options_spec.lua
index bc72ca71aa..62b08c0967 100644
--- a/test/functional/ui/options_spec.lua
+++ b/test/functional/ui/options_spec.lua
@@ -10,7 +10,6 @@ describe('ui receives option updates', function()
   before_each(function()
     clear()
     screen = Screen.new(20,5)
-    screen:attach()
   end)
 
   after_each(function()
@@ -27,15 +26,21 @@ describe('ui receives option updates', function()
     linespace=0,
     showtabline=1,
     termguicolors=false,
+    ext_cmdline=false,
+    ext_popupmenu=false,
+    ext_tabline=false,
+    ext_wildmenu=false,
   }
 
   it("for defaults", function()
+    screen:attach()
     screen:expect(function()
       eq(defaults, screen.options)
     end)
   end)
 
   it("when setting options", function()
+    screen:attach()
     local changed = {}
     for k,v in pairs(defaults) do
       changed[k] = v
@@ -76,4 +81,30 @@ describe('ui receives option updates', function()
       eq(defaults, screen.options)
     end)
   end)
+
+  it('with UI extensions', function()
+    local changed = {}
+    for k,v in pairs(defaults) do
+      changed[k] = v
+    end
+
+    screen:attach({ext_cmdline=true, ext_wildmenu=true})
+    changed.ext_cmdline = true
+    changed.ext_wildmenu = true
+    screen:expect(function()
+      eq(changed, screen.options)
+    end)
+
+    screen:set_option('ext_popupmenu', true)
+    changed.ext_popupmenu = true
+    screen:expect(function()
+      eq(changed, screen.options)
+    end)
+
+    screen:set_option('ext_wildmenu', false)
+    changed.ext_wildmenu = false
+    screen:expect(function()
+      eq(changed, screen.options)
+    end)
+  end)
 end)