Skip to content
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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justinmk
Copy link
Member

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

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()`.
@github-actions github-actions bot added the api libnvim, Nvim RPC API label Apr 24, 2024
@justinmk justinmk added this to the 0.11 milestone Apr 24, 2024
@justinmk justinmk mentioned this pull request May 12, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api libnvim, Nvim RPC API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant