-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
[RPC] Add methods to enumerate channels and clients to identify themselves #6743
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
Conversation
If the client sends this dict then "major" should be required (if "minor" were provided without "major" it could be a mistake).
Perhaps Informal description could live in a
@teto did some work towards this. We discussed a generic "channel created" event. Which reminds me, the thing holding us back from adding new events like this (instead of autocommands) is that we don't have guidelines for nvim events vs plugin events. Currently it's ad-hoc and any plugin could broadcast an event to all channels, which could cause bugs that are difficult to track. I have been thinking we may want to reserve a prefix for nvim core events, or devise some other way of having a reserved channel that only nvim can publish to (but all clients can subscribe to). |
Bulitin events should start with A complication is that internal events can be generated with different levels of async-ness:
|
Perhaps there should be an extensible dict for "informal" stuff (github weblink, license, fancy SVG logo, etc) ? |
Updated with some actual code. You can listen for |
@justinmk Implemention-wise this does what I need from it (except for some validation). The info is incomplete, but naturally extendable later on (extend to non-RPC channels, info about the process, the socket address etc). Any comments about the design or should I just finish it off with tests? |
Another concern: In case of nvim-to-nvim connection later on the two instance should be able to do the rendez-vous automatically. Not implemented here, but we should make sure the design is forward compatible (and it must also support The Amazing Cat scenario). |
Note to self: for pty channels show the |
src/nvim/channel.c
Outdated
@@ -226,12 +195,13 @@ void channel_create_event(Channel *chan, char *ext_source) | |||
source = (char *)IObuff; | |||
} | |||
|
|||
ILOG("new channel %" PRIu64 " (%s%s): %s", chan->id, stream_desc, | |||
mode_desc, source); | |||
ILOG("new channel %" PRIu64 " (%s%s): %s", chan->id, source); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many %s
in the format string.
src/nvim/channel.c
Outdated
(void)ext_source; | ||
#endif | ||
|
||
channel_info_changed(chan); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used before declared?
9eb57cc
to
de98bbc
Compare
9c19e48
to
7420000
Compare
@b-r-o-c-k I wonder if the indeterminism is related to CPU/IO load somehow. But the changed code (firing of the autocmd) happens after the return code is determined by |
80c14db
to
8b2d837
Compare
I tested reverting all code changes with latest push and this simple test still fails on appveyor (no autocmds or anything), sometimes on the first and sometimes on the second line:
So some racyness in process creation with |
3fb2582
to
49adbe8
Compare
Failure looks unrelated:
|
Haven't seen that failure before, and |
@justinmk any reason not to have this in 0.3? |
@bfredl No reason, would like to take one more look at it later today, feel free to merge otherwise. |
04d8fce
to
fe3a2cd
Compare
This travis ASAN failure is a bit weird:
A 64-bit build shouldn't need multilib, no? |
Fire autocmd when channel opens or its info changes. Add a way for API clients can describe themselves.
FEATURES: 3cc7ebf #7234 built-in VimL expression parser 6a7c904 #4419 implement <Cmd> key to invoke command in any mode b836328 #7679 'startup: treat stdin as text instead of commands' 58b210e :digraphs : highlight with hl-SpecialKey #2690 7a13611 #8276 'startup: Let `-s -` read from stdin' 1e71978 events: VimSuspend, VimResume #8280 1e7d5e8 #6272 'stdpath()' f96d99a #8247 server: introduce --listen e8c39f7 #8226 insert-mode: interpret unmapped META as ESC 98e7112 msg: do not scroll entire screen (#8088) f72630b #8055 let negative 'writedelay' show all redraws 5d2dd2e win: has("wsl") on Windows Subsystem for Linux #7330 a4f6cec cmdline: CmdlineEnter and CmdlineLeave autocommands (#7422) 207b7ca #6844 channels: support buffered output and bytes sockets/stdio API: f85cbea #7917 API: buffer updates 418abfc #6743 API: list information about all channels/jobs. 36b2e3f #8375 API: nvim_get_commands 273d2cd #8329 API: Make nvim_set_option() update `:verbose set …` 8d40b36 #8371 API: more reliable/descriptive VimL errors ebb1acb #8353 API: nvim_call_dict_function 9f994bb #8004 API: nvim_list_uis 3405704 #7520 API/UI: forward option updates to UIs 911b1e4 #7821 API: improve nvim_command_output WINDOWS OS: 9cefd83 #8084, #8516 build/win: support MSVC ee4e1fd win: Fix reading content from stdin (#8267) TUI: ffb8904 #8309 TUI: add support for mouse release events in urxvt 8d5a46e #8081 TUI: implement "standout" attribute 6071637 TUI: support TERM=konsole-256color 67848c0 #7653 TUI: report TUI info with -V3 ('verbose' >= 3) 3d0ee17 TUI/rxvt: enable focus-reporting d109f56 #7640 TUI: 'term' option: reflect effective terminal behavior FIXES: ed6a113 #8273 'job-control: avoid kill-timer race' 4e02f1a #8107 'jobs: separate process-group' 451c48a terminal: flush vterm output buffer on pty output #8486 5d6732f :checkhealth fixes #8335 53f11dc #8218 'Fix errors reported by PVS' d05712f inccommand: pause :terminal redraws (#8307) 51af911 inccommand: do not execute trailing commands #8256 84359a4 terminal: resize to the max dimensions (#8249) d49c1dd #8228 Make vim_fgets() return the same values as in Vim 60e96a4 screen: winhl=Normal:Background should not override syntax (#8093) 0c59ac1 #5908 'shada: Also save numbered marks' ba87a2c cscope: ignore EINTR while reading the prompt (#8079) b1412dc #7971 ':terminal Enter/Leave should not increment jumplist' 3a5721e TUI: libtermkey: force CSI driver for mouse input #7948 6ff13d7 #7720 TUI: faster startup 1c6e956 #7862 TUI: fix resize-related segfaults a58c909 #7676 TUI: always hide cursor when flushing, never flush buffers during unibilium output 303e1df #7624 TUI: disable BCE almost always 249bdb0 #7761 mark: Make sure that jumplist item will not have zero lnum 6f41ce0 #7704 macOS: Set $LANG based on the system locale a043899 #7633 'Retry fgets on EINTR' CHANGES: ad60927 #8304 default to 'nofsync' f3f1970 #8035 defaults: 'fillchars' a6052c7 #7984 defaults: sidescroll=1 b69fa86 #7888 defaults: enable cscopeverbose 7c4bb23 defaults: do :filetype stuff unless explicitly "off" 2aa308c #5658 'Apply :lmap in macros' 8ce6393 terminal: Leave 'relativenumber' alone (#8360) e46534b #4486 refactor: Remove maxmem, maxmemtot options 131aad9 win: defaults: 'shellcmdflag', 'shellxquote' #7343 c57d315 #8031 jobwait(): return -2 on interrupt also with timeout 6452831 clipboard: macOS: fallback to tmux if pbcopy is broken #7940 300d365 #7919 Make 'langnoremap' apply directly after a map ada1956 #7880 'lua/executor: Remove lightuserdata' INTERNAL: de0a954 #7806 internal statistics for list impl dee78a4 #7708 rewrite internal list impl
Obviously if GUI vim is started outside of tmux, we always already disabled this plugin, since $TMUX would be empty then. But if someone starts GUI vim from the command line within tmux, then $TMUX would not be empty and the plugin would become enabled which would look very weird, because we then embed the GUI vim statusline in a remote tmux console window. For gVim fixing this is very easy, as we can simply check for has('gui_running'). Unfortunately, this variable will probably not work in neovim GUIs, since the neovim GUI only attaches after vimrc has already been sourced, so has('gui_running') will return 0 even though the GUI will attach later in the startup process. We do not want to use neovim-qt specific variables like g:GuiLoaded [1]. For example gonvim uses another variable [2]. This is where things get more complicated. It seems like the only reliable way to detect it is to use the UIEnter autocmd [3], which luckily exists since 2017 [4]. There is also a way to identify neovim clients over RPC [5], but using autocmds is probably much easier. Therefore to solve the problem, we disable the plugin on UIEnter, which we do with the new tpipeline#state#restore() function added in a95ccb6. Theoretically we could also reload the plugin again on UILeave, but in reality I don't see anyone ever needing to do this. We could also add an option to keep the plugin enabled for GUIs if someone wants to have that feature, but let's do that in the future if that usecase ever comes up, which it probably won't (yes I am jinxing it). At least we now have sane defaults for GUI vim. Fixes #29. [1] equalsraf/neovim-qt#219 [2] dzhou121/gonvim#17 [3] neovim/neovim#3646 [4] neovim/neovim#6917 [5] neovim/neovim#6743
To implement stuff like #6742 in a forward-compatible fashion, @justinmk suggested an api function where embedders and connected clients might identify themselves and their capabilities. Before implementing it would be good to discuss what it should contain. Some suggestions
name
version: a dicitionary with "major", "minor", "commit", "prerelease" etc keys (all optional)
role: informal description of the clients role, with some suggested values: host, plugin (for a single-plugin process), gui
methods: builtin methods (before loading plugins into the host), not sure how detailed, but at least argument count so methods can be extended.
There probably should be an autocommand or something so interested parties (like the rplugin infrastructure) could listen for identified clients. And a
nvim_list_clients()
function for introspection/debugging.