feat(api): broadcast events to ALL channels #28487

Problem: `vim.rpcnotify(0)` and `rpcnotify(0)` are documented as follows: If {channel} is 0, the event is broadcast to all channels. But that's not actually true. Channels must call `nvim_subscribe` to receive "broadcast" events, so it's actually "multicast". - Assuming there is a use-case for "broadcast", the current model adds an extra step for broadcasting: all channels need to "subscribe". - The presence of `nvim_subscribe` is a source of confusion for users, because its name implies something more generally useful than what it does. Presumably the use-case of `nvim_subscribe` is to avoid "noise" on RPC channels not expected a broadcast notification, and potentially an error if the channel client reports an unknown event. Solution: - Deprecate `nvim_subscribe`/`nvim_unsubscribe`. - If applications want to multicast, they can keep their own multicast list. Or they can use `nvim_list_chans()` and `nvim_get_chan_info()` to enumerate and filter the clients they want to target. - Always send "broadcast" events to ALL channels. Don't require channels to "subscribe" to receive broadcasts. This matches the documented behavior of `rpcnotify()`.
2025-02-25 18:55:25 -06:00 · 2024-05-17 07:37:39 -07:00 · 2024-05-17 07:37:39 -07:00 · aec4938a21
commit aec4938a21
parent 42aa69b076
9 changed files with 102 additions and 185 deletions
--- a/runtime/doc/api.txt
+++ b/runtime/doc/api.txt
@ -1461,24 +1461,6 @@ nvim_strwidth({text})                                        *nvim_strwidth()*
    Return: ~
        Number of cells

-nvim_subscribe({event})                                     *nvim_subscribe()*
-    Subscribes to event broadcasts.
-
-    Attributes: ~
-        |RPC| only
-
-    Parameters: ~
-      • {event}  Event type string
-
-nvim_unsubscribe({event})                                 *nvim_unsubscribe()*
-    Unsubscribes to event broadcasts.
-
-    Attributes: ~
-        |RPC| only
-
-    Parameters: ~
-      • {event}  Event type string
-
 nvim__complete_set({index}, {opts})                     *nvim__complete_set()*
    EXPERIMENTAL: this API may change in the future.

--- a/runtime/doc/deprecated.txt
+++ b/runtime/doc/deprecated.txt
@ -12,76 +12,82 @@ They should not be used in new scripts, and old scripts should be updated.
 ==============================================================================
 Deprecated features

+------------------------------------------------------------------------------
 DEPRECATED IN 0.11					*deprecated-0.11*

-• N/A
+API
+- nvim_subscribe()	Plugins must maintain their own "multicast" channels list.
+- nvim_unsubscribe()	Plugins must maintain their own "multicast" channels list.

+
+------------------------------------------------------------------------------
 DEPRECATED IN 0.10					*deprecated-0.10*

+API
+• *nvim_buf_get_option()*	Use |nvim_get_option_value()| instead.
+• *nvim_buf_set_option()*	Use |nvim_set_option_value()| instead.
+• *nvim_call_atomic()*		Use |nvim_exec_lua()| instead.
+• *nvim_get_option()*		Use |nvim_get_option_value()| instead.
+• *nvim_set_option()*		Use |nvim_set_option_value()| instead.
+• *nvim_win_get_option()*	Use |nvim_get_option_value()| instead.
+• *nvim_win_set_option()*	Use |nvim_set_option_value()| instead.
+
+CHECKHEALTH
+• *health#report_error* *vim.health.report_error()*	Use |vim.health.error()| instead.
+• *health#report_info* *vim.health.report_info()*	Use |vim.health.info()| instead.
+• *health#report_ok* *vim.health.report_ok()*		Use |vim.health.ok()| instead.
+• *health#report_start* *vim.health.report_start()*	Use |vim.health.start()| instead.
+• *health#report_warn* *vim.health.report_warn()*	Use |vim.health.warn()| instead.
+
+DIAGNOSTICS
 • Configuring |diagnostic-signs| using |:sign-define| or |sign_define()|. Use
  the "signs" key of |vim.diagnostic.config()| instead.
-
-• Checkhealth functions:
-  - *health#report_error* *vim.health.report_error()*	Use |vim.health.error()| instead.
-  - *health#report_info* *vim.health.report_info()*	Use |vim.health.info()| instead.
-  - *health#report_ok* *vim.health.report_ok()*		Use |vim.health.ok()| instead.
-  - *health#report_start* *vim.health.report_start()*	Use |vim.health.start()| instead.
-  - *health#report_warn* *vim.health.report_warn()*	Use |vim.health.warn()| instead.
-
-• |API| functions:
-  - *nvim_buf_get_option()*	Use |nvim_get_option_value()| instead.
-  - *nvim_buf_set_option()*	Use |nvim_set_option_value()| instead.
-  - *nvim_call_atomic()*		Use |nvim_exec_lua()| instead.
-  - *nvim_get_option()*		Use |nvim_get_option_value()| instead.
-  - *nvim_set_option()*		Use |nvim_set_option_value()| instead.
-  - *nvim_win_get_option()*	Use |nvim_get_option_value()| instead.
-  - *nvim_win_set_option()*	Use |nvim_set_option_value()| instead.
-
 • vim.diagnostic functions:
-  - *vim.diagnostic.disable()*		Use |vim.diagnostic.enable()|
-  - *vim.diagnostic.is_disabled()*	Use |vim.diagnostic.is_enabled()|
-  - Legacy signature: `vim.diagnostic.enable(buf:number, namespace:number)`
+  • *vim.diagnostic.disable()*		Use |vim.diagnostic.enable()|
+  • *vim.diagnostic.is_disabled()*	Use |vim.diagnostic.is_enabled()|
+  • Legacy signature: `vim.diagnostic.enable(buf:number, namespace:number)`

-• vim.lsp functions:
-  - *vim.lsp.util.get_progress_messages()*	Use |vim.lsp.status()| instead.
-  - *vim.lsp.get_active_clients()*		Use |vim.lsp.get_clients()| instead.
-  - *vim.lsp.for_each_buffer_client()*		Use |vim.lsp.get_clients()| instead.
-  - *vim.lsp.util.trim_empty_lines()*		Use |vim.split()| with `trimempty` instead.
-  - *vim.lsp.util.try_trim_markdown_code_blocks()*
-  - *vim.lsp.util.set_lines()*
-  - *vim.lsp.util.extract_completion_items()*
-  - *vim.lsp.util.parse_snippet()*
-  - *vim.lsp.util.text_document_completion_list_to_complete_items()*
-  - *vim.lsp.util.lookup_section()*		Use |vim.tbl_get()| instead: >
+LSP
+• *vim.lsp.util.get_progress_messages()*	Use |vim.lsp.status()| instead.
+• *vim.lsp.get_active_clients()*		Use |vim.lsp.get_clients()| instead.
+• *vim.lsp.for_each_buffer_client()*		Use |vim.lsp.get_clients()| instead.
+• *vim.lsp.util.trim_empty_lines()*		Use |vim.split()| with `trimempty` instead.
+• *vim.lsp.util.try_trim_markdown_code_blocks()*
+• *vim.lsp.util.set_lines()*
+• *vim.lsp.util.extract_completion_items()*
+• *vim.lsp.util.parse_snippet()*
+• *vim.lsp.util.text_document_completion_list_to_complete_items()*
+• *vim.lsp.util.lookup_section()*		Use |vim.tbl_get()| instead: >
 						local keys = vim.split(section, '.', { plain = true })
 						local  vim.tbl_get(table, unpack(keys))

+LUA
 • *vim.loop*				Use |vim.uv| instead.
+• *vim.tbl_add_reverse_lookup()*
+• *vim.tbl_flatten()*			Use |Iter:flatten()| instead.
+• *vim.tbl_islist()*			Use |vim.islist()| instead.

-• vim.treesitter functions:
-  - *LanguageTree:for_each_child()*		Use |LanguageTree:children()| (non-recursive) instead.
-
+OPTIONS
 • The "term_background" UI option |ui-ext-options| is deprecated and no longer
  populated. Background color detection is now performed in Lua by the Nvim
  core, not the TUI.

-• Lua stdlib:
-  - *vim.tbl_add_reverse_lookup()*
-  - *vim.tbl_flatten()*			Use |Iter:flatten()| instead.
-  - *vim.tbl_islist()*			Use |vim.islist()| instead.
+TREESITTER
+• *LanguageTree:for_each_child()*	Use |LanguageTree:children()| (non-recursive) instead.

+
+------------------------------------------------------------------------------
 DEPRECATED IN 0.9					*deprecated-0.9*

-• vim.treesitter functions
-  - *vim.treesitter.language.require_language()*	Use |vim.treesitter.language.add()| instead.
-  - *vim.treesitter.get_node_at_pos()*			Use |vim.treesitter.get_node()| instead.
-  - *vim.treesitter.get_node_at_cursor()*		Use |vim.treesitter.get_node()|
+API
+- *nvim_get_hl_by_name()*		Use |nvim_get_hl()| instead.
+- *nvim_get_hl_by_id()*			Use |nvim_get_hl()| instead.
+
+TREESITTER
+- *vim.treesitter.language.require_language()*	Use |vim.treesitter.language.add()| instead.
+- *vim.treesitter.get_node_at_pos()*		Use |vim.treesitter.get_node()| instead.
+- *vim.treesitter.get_node_at_cursor()*		Use |vim.treesitter.get_node()|
 						and |TSNode:type()| instead.
-
-• |API| functions:
-  - *nvim_get_hl_by_name()*		Use |nvim_get_hl()| instead.
-  - *nvim_get_hl_by_id()*		Use |nvim_get_hl()| instead.
-
 • The following top level Treesitter functions have been moved:
  - *vim.treesitter.inspect_language()*    -> |vim.treesitter.language.inspect()|
  - *vim.treesitter.get_query_files()*     -> |vim.treesitter.query.get_files()|
@ -98,10 +104,12 @@ DEPRECATED IN 0.9 *deprecated-0.9*
  - *vim.treesitter.query.get_range()*     -> |vim.treesitter.get_range()|
  - *vim.treesitter.query.get_node_text()* -> |vim.treesitter.get_node_text()|

-• Lua stdlib:
+LUA
  - *nvim_exec()*				Use |nvim_exec2()| instead.
  - *vim.pretty_print()*			Use |vim.print()| instead.

+
+------------------------------------------------------------------------------
 DEPRECATED IN 0.8 OR EARLIER

 API
--- a/runtime/doc/news.txt
+++ b/runtime/doc/news.txt
@ -25,7 +25,12 @@ These changes may require adaptations in your config or plugins.

 API

-• TODO
+• `vim.rpcnotify(0)` and `rpcnotify(0)` broadcast to ALL channels. Previously
+  they would "multicast" only to subscribed channels (controlled by
+  `nvim_subscribe()`). Plugins and clients that want "multicast" behavior must
+  now maintain their own list of channels.
+  • In the future, |vim.rpcnotify()| may accept a list of channels, if there
+    is demand for this use-case.

 DEFAULTS

--- a/src/nvim/api/deprecated.c
+++ b/src/nvim/api/deprecated.c
@ -786,3 +786,23 @@ theend:
  api_clear_error(&nested_error);
  return rv;
 }
+
+/// @deprecated
+///
+/// @param channel_id Channel id (passed automatically by the dispatcher)
+/// @param event      Event type string
+void nvim_subscribe(uint64_t channel_id, String event)
+  FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
+{
+  // Does nothing. `rpcnotify(0,…)` broadcasts to all channels, there are no "subscriptions".
+}
+
+/// @deprecated
+///
+/// @param channel_id Channel id (passed automatically by the dispatcher)
+/// @param event      Event type string
+void nvim_unsubscribe(uint64_t channel_id, String event)
+  FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
+{
+  // Does nothing. `rpcnotify(0,…)` broadcasts to all channels, there are no "subscriptions".
+}
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@ -1334,36 +1334,6 @@ void nvim_put(ArrayOf(String) lines, String type, Boolean after, Boolean follow,
  });
 }

-/// Subscribes to event broadcasts.
-///
-/// @param channel_id Channel id (passed automatically by the dispatcher)
-/// @param event      Event type string
-void nvim_subscribe(uint64_t channel_id, String event)
-  FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
-{
-  size_t length = (event.size < METHOD_MAXLEN ? event.size : METHOD_MAXLEN);
-  char e[METHOD_MAXLEN + 1];
-  memcpy(e, event.data, length);
-  e[length] = NUL;
-  rpc_subscribe(channel_id, e);
-}
-
-/// Unsubscribes to event broadcasts.
-///
-/// @param channel_id Channel id (passed automatically by the dispatcher)
-/// @param event      Event type string
-void nvim_unsubscribe(uint64_t channel_id, String event)
-  FUNC_API_SINCE(1) FUNC_API_REMOTE_ONLY
-{
-  size_t length = (event.size < METHOD_MAXLEN
-                   ? event.size
-                   : METHOD_MAXLEN);
-  char e[METHOD_MAXLEN + 1];
-  memcpy(e, event.data, length);
-  e[length] = NUL;
-  rpc_unsubscribe(channel_id, e);
-}
-
 /// Returns the 24-bit RGB value of a |nvim_get_color_map()| color name or
 /// "#rrggbb" hexadecimal string.
 ///
--- a/src/nvim/msgpack_rpc/channel.c
+++ b/src/nvim/msgpack_rpc/channel.c
@ -67,8 +67,6 @@ static void log_notify(char *dir, uint64_t channel_id, const char *name)
 # define log_notify(...)
 #endif

-static Set(cstr_t) event_strings = SET_INIT;
-
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "msgpack_rpc/channel.c.generated.h"
 #endif
@ -111,9 +109,9 @@ static Channel *find_rpc_channel(uint64_t id)
  return chan;
 }

-/// Publishes an event to a channel.
+/// Publishes an event to a channel (emits a notification to method `name`).
 ///
-/// @param id Channel id. 0 means "broadcast to all subscribed channels"
+/// @param id Channel id, or 0 to broadcast to all RPC channels.
 /// @param name Event name (application-defined)
 /// @param args Array of event arguments
 /// @return True if the event was sent successfully, false otherwise.
@ -204,41 +202,6 @@ Object rpc_send_call(uint64_t id, const char *method_name, Array args, ArenaMem
  return frame.errored ? NIL : frame.result;
 }

-/// Subscribes to event broadcasts
-///
-/// @param id The channel id
-/// @param event The event type string
-void rpc_subscribe(uint64_t id, char *event)
-{
-  Channel *channel;
-
-  if (!(channel = find_rpc_channel(id))) {
-    abort();
-  }
-
-  const char **key_alloc = NULL;
-  if (set_put_ref(cstr_t, &event_strings, event, &key_alloc)) {
-    *key_alloc = xstrdup(event);
-  }
-
-  set_put(cstr_t, channel->rpc.subscribed_events, *key_alloc);
-}
-
-/// Unsubscribes to event broadcasts
-///
-/// @param id The channel id
-/// @param event The event type string
-void rpc_unsubscribe(uint64_t id, char *event)
-{
-  Channel *channel;
-
-  if (!(channel = find_rpc_channel(id))) {
-    abort();
-  }
-
-  unsubscribe(channel, event);
-}
-
 static void receive_msgpack(Stream *stream, RBuffer *rbuf, size_t c, void *data, bool eof)
 {
  Channel *channel = data;
@ -494,34 +457,24 @@ static void send_error(Channel *chan, MsgpackRpcRequestHandler handler, MessageT
  api_clear_error(&e);
 }

+/// Broadcasts a notification to all RPC channels.
 static void broadcast_event(const char *name, Array args)
 {
-  kvec_withinit_t(Channel *, 4) subscribed = KV_INITIAL_VALUE;
-  kvi_init(subscribed);
+  kvec_withinit_t(Channel *, 4) chans = KV_INITIAL_VALUE;
+  kvi_init(chans);
  Channel *channel;

  map_foreach_value(&channels, channel, {
-    if (channel->is_rpc
-        && set_has(cstr_t, channel->rpc.subscribed_events, name)) {
-      kv_push(subscribed, channel);
+    if (channel->is_rpc) {
+      kv_push(chans, channel);
    }
  });

-  if (kv_size(subscribed)) {
-    serialize_request(subscribed.items, kv_size(subscribed), 0, name, args);
+  if (kv_size(chans)) {
+    serialize_request(chans.items, kv_size(chans), 0, name, args);
  }

-  kvi_destroy(subscribed);
-}
-
-static void unsubscribe(Channel *channel, char *event)
-{
-  if (!set_has(cstr_t, &event_strings, event)) {
-    WLOG("RPC: ch %" PRIu64 ": tried to unsubscribe unknown event '%s'",
-         channel->id, event);
-    return;
-  }
-  set_del(cstr_t, channel->rpc.subscribed_events, event);
+  kvi_destroy(chans);
 }

 /// Mark rpc state as closed, and release its reference to the channel.
@ -551,7 +504,6 @@ void rpc_free(Channel *channel)
  unpacker_teardown(channel->rpc.unpacker);
  xfree(channel->rpc.unpacker);

-  set_destroy(cstr_t, channel->rpc.subscribed_events);
  kv_destroy(channel->rpc.call_stack);
  api_free_dictionary(channel->rpc.info);
 }
@ -719,12 +671,6 @@ const char *get_client_info(Channel *chan, const char *key)
 #ifdef EXITFREE
 void rpc_free_all_mem(void)
 {
-  cstr_t key;
-  set_foreach(&event_strings, key, {
-    xfree((void *)key);
-  });
-  set_destroy(cstr_t, &event_strings);
-
  multiqueue_free(ch_before_blocking_events);
 }
 #endif
--- a/src/nvim/msgpack_rpc/channel_defs.h
+++ b/src/nvim/msgpack_rpc/channel_defs.h
@ -37,7 +37,6 @@ typedef struct {
 } RequestEvent;

 typedef struct {
-  Set(cstr_t) subscribed_events[1];
  bool closed;
  Unpacker *unpacker;
  uint32_t next_request_id;
--- a/test/functional/api/server_notifications_spec.lua
+++ b/test/functional/api/server_notifications_spec.lua
@ -1,7 +1,6 @@
 local t = require('test.testutil')
 local n = require('test.functional.testnvim')()

-local assert_log = t.assert_log
 local eq, clear, eval, command, next_msg = t.eq, n.clear, n.eval, n.command, n.next_msg
 local api = n.api
 local exec_lua = n.exec_lua
@ -34,18 +33,18 @@ describe('notify', function()
    end)
  end)

-  describe('passing 0 as the channel id', function()
-    it('sends the notification/args to all subscribed channels', function()
-      api.nvim_subscribe('event2')
+  describe('channel id 0', function()
+    it('broadcasts the notification/args to all channels', function()
      eval('rpcnotify(0, "event1", 1, 2, 3)')
      eval('rpcnotify(0, "event2", 4, 5, 6)')
      eval('rpcnotify(0, "event2", 7, 8, 9)')
+      eq({ 'notification', 'event1', { 1, 2, 3 } }, next_msg())
      eq({ 'notification', 'event2', { 4, 5, 6 } }, next_msg())
      eq({ 'notification', 'event2', { 7, 8, 9 } }, next_msg())
-      api.nvim_unsubscribe('event2')
-      api.nvim_subscribe('event1')
+
      eval('rpcnotify(0, "event2", 10, 11, 12)')
      eval('rpcnotify(0, "event1", 13, 14, 15)')
+      eq({ 'notification', 'event2', { 10, 11, 12 } }, next_msg())
      eq({ 'notification', 'event1', { 13, 14, 15 } }, next_msg())
    end)

@ -78,17 +77,6 @@ describe('notify', function()
    end)
  end)

-  it('unsubscribe non-existing event #8745', function()
-    clear { env = {
-      NVIM_LOG_FILE = testlog,
-    } }
-    api.nvim_subscribe('event1')
-    api.nvim_unsubscribe('doesnotexist')
-    assert_log("tried to unsubscribe unknown event 'doesnotexist'", testlog, 10)
-    api.nvim_unsubscribe('event1')
-    assert_alive()
-  end)
-
  it('cancels stale events on channel close', function()
    local catchan = eval("jobstart(['cat'], {'rpc': v:true})")
    local catpath = eval('exepath("cat")')
@ -97,7 +85,6 @@ describe('notify', function()
      exec_lua(
        [[
      vim.rpcnotify(..., "nvim_call_function", 'chanclose', {..., 'rpc'})
-      vim.rpcnotify(..., "nvim_subscribe", "daily_rant")
      return vim.api.nvim_get_chan_info(...)
    ]],
        catchan
--- a/test/functional/vimscript/api_functions_spec.lua
+++ b/test/functional/vimscript/api_functions_spec.lua
@ -181,8 +181,8 @@ describe('eval-API', function()
    eq('Vim(call):E117: Unknown function: buffer_get_line', err)

    -- some api functions are only useful from a msgpack-rpc channel
-    err = exc_exec('call nvim_subscribe("fancyevent")')
-    eq('Vim(call):E117: Unknown function: nvim_subscribe', err)
+    err = exc_exec('call nvim_set_client_info()')
+    eq('Vim(call):E117: Unknown function: nvim_set_client_info', err)
  end)

  it('have metadata accessible with api_info()', function()