-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
remote TUI: refactor TUI from thread to separate process #18375
Conversation
d7a2d08
to
8dfe26e
Compare
Problem: Piping NodeJS output into Neovim makes the editor unusable. This happens because NodeJS changes the tty state on exit after Nvim calls uv_tty_set_mode(). (May not always happen due to race condition.) This should have been fixed by 4ba5b4a #13084. But some commands and functions (:sleep, system(), …) call ui_flush() internally, in particular the first tui_mode_change() is called before the end of startup. Steps to reproduce: 1. node -e "setTimeout(()=>{console.log('test')}, 1000)" | nvim -u NORC +"sleep 500m" - 2. The cursor key letters just overwrite the editor screen, and CTRL+C exits. Solution: Skip pending_mode_update during startup. Note: Delaying ui_flush() entirely could be a more general solution (emit a new UI event on VimEnter?). But "remote/coprocess TUI" #18375 could make all of this moot anyway. Fixes #18470
Problem: Piping NodeJS output into Neovim makes the editor unusable. This happens because NodeJS changes the tty state on exit after Nvim calls uv_tty_set_mode(). (May not always happen due to race condition.) This should have been fixed by 4ba5b4a neovim#13084. But some commands and functions (:sleep, system(), …) call ui_flush() internally, in particular the first tui_mode_change() is called before the end of startup. Steps to reproduce: 1. node -e "setTimeout(()=>{console.log('test')}, 1000)" | nvim -u NORC +"sleep 500m" - 2. The cursor key letters just overwrite the editor screen, and CTRL+C exits. Solution: Skip pending_mode_update during startup. Note: Delaying ui_flush() entirely could be a more general solution (emit a new UI event on VimEnter?). But "remote/coprocess TUI" neovim#18375 could make all of this moot anyway. Fixes neovim#18470
runtime/doc/ui.txt
Outdated
@@ -55,8 +55,8 @@ with these (optional) keys: | |||
`stdin_fd` Read buffer from `fd` as if it was a stdin pipe | |||
This option can only used by |--embed| ui, | |||
see |ui-startup-stdin|. | |||
|
|||
|
|||
`term_ttyin` Tells if `stdin` is a `tty` or not. |
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.
for parallel form with stdin_fd
this could be named stdin_tty
, stdout_tty
c5ab361
to
a750faf
Compare
95008a1
to
d77dcde
Compare
followup: #21675 |
@bfredl can I ask for a brief overview of how the embedded nvim process is being created? Don't need you to describe the whole thing. Some high-level overview would be fine. I can do some research on my own then. |
@3N4N we use the same internal API as |
Of course GUI detection wasn't fucked up enough in neovim already (see c2603e4 for the last rant about this topic). Instead of requiring special handling just for neovim, we now also need special treatment just for neovim 0.9, because of course they don't adhere to their own documentation anymore and v:event['chan'] may now also return 1, as a sideeffect of running the internal TUI in a separate process [0]. So to this day there is still no sane way to detect TUI in neovim, instead we have to add a hacky workaround to check nvim_list_uis() for ext_termcolors, which I am 100% confident will break in the future too. Vim had sane API for this since forever and it is as simple as checking for has('gui_running') at any point of time, but of course neovim can't have this set at startup because we have to make everything convoluted as fuck by sourcing plugins before the UI has a chance to attach. Why the UI is not allowed to set a flag as the very first thing in the startup sequence (and then attach later), is beyond stupid. This is also not the first time that neovim's weird startup sequence causes problems [1]. Fixes #46 [0] neovim/neovim#18375 [1] neovim/neovim#19362
It would be good to update the outdated documentation
Note that GUIs as well as the internal TUI may now return 1, so as of now there is no sane way to detect TUI in neovim (but adding official API for this would be a separate issue). To reproduce this sideeffect of this patch, run the following which will output 1 after this patch and 0 before this patch: nvim --clean -Nu <(cat<<EOF
au UIEnter * echom v:event['chan']
EOF) Also see vimpostor/vim-tpipeline#46 (comment) |
It looks like finally neovim got sane GUI detection support, like vim [0]. This means we can remove the temporary workaround from 95a6ccb completely. Nightly neovim users who used a version of neovim released after the broken 2022-12-01 version [1], but before the fix in 2023-02-28, should update their version of neovim as soon as possible, as the root of the problem is now properly fixed for neovim 0.9. Fixes #46. [0] neovim/neovim#22382 [1] neovim/neovim#18375
Maybe a stupid question, but how should I kill or detach the TUI without killing the terminal I am in? |
Eeerm... |
That would also kill the headless neovim instance. I just want to detach/kill the TUI and leave the headless session running.
|
@egolep There is no real user-level functionality yet.. You could use A direct way for the user to say "detach the current ui" (the one last sending input to nvim), would be a useful addition to core, for sure. |
Rebase of most of #10071
Very rough draft. "builtin TUI" replacement sorta works, but not remote attachment yet (actually input works, but not redrawing..)
Also needs a lot of cleanup, obviously.