Merge pull request #7759 from bfredl/ext_options

ui: refactor external widget options

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

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

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)

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");

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;
-}

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

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];
   }
 

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)

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()

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()

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)