The previous commit ensures that we can never flush the buffer in a
state where toggling cursor visibility can corrupt other escape codes.
Thus, we can remove the workaround added as part of e838452, simplyfing
the code and hiding the cursor on more occasions.
e83845285 fixed an issue where long (true color) escape sequences got
interrupted by the cursor visibility toggling caused by buffer flushes.
cdfaecb25 introduces a new issue which causes similar problems: While
the old buffer flushing code appended the cursor visibility escapes to
the buffer before/after flushing, the new code effectively prepends the
sequences.
Assume the following sequence of events occurs:
- A long escape code is issued using unibi_out when the buffer is
almost full
- out() gets called for a prefix of that escape code, causing the
buffer to fill up
- flush_buf(ui, false) is called and (correctly) does not insert any
cursor toggling escapes
- The rest of the escape code is written into the now empty buffer
- At some later point, some other part of nvim calls flush_buf(ui,
true), which then toggles the cursor, corrupting the escape code
This could possibly also be fixed by tracking the state of the buffer
(i.e. does it contain a partially output escape code?), but this seems
fragile in the same way e83845285 turned out to be.
The root cause for all these problems is the mismatch between nvim's
(implicit) assumption that the buffer is flushable at any point in time
and the non-atomicity of unibilium's character based callback interface.
The proper fix (without modifying unibilium) is to ensure nvim's
assumption about the buffer state holds at all times.
To that end, add a "cork" flag which ensures one unibi_out-call never
splits its output across a buffer flush; if an escape code does not fit
into the current buffer, flush it without any part of the escape code in
it and insert the whole escape code in the emptied buffer. This is a
little more complex because it modifies the buffer in place rather than
printing into another buffer, checking the remaining space in the
terminal buffer and then memcpy'ing it.
Get terminal debugging info by starting Nvim with 'verbose' level 3:
nvim -V3log
This is like Vim's `:set termcap`, which was removed in Nvim (and would
be very awkward to restore because of the decoupled UI).
filter('v:_null_list, 'v:val') should return v:_null_list and a similar
statement should hold for map.
Changes after review
* Test inserted in legacy test suite has been removed by reverting the commit
adding it.
* Change the fix to tv_copy the argument before returning.
* Readd the two tests on crashes, and modified their expected return value.
* Move the test from 'incorrect behaviour' section to 'correct behaviour section'
* Add analogous tests for v:_null_dict
Always copy list or dictionary to return variable
If the type of input is correct (i.e. either a list or a dictionary), this
should also be returned.
ref #7649
ref #766427f9b1c7b0 caused a regression: it uses loop_schedule_deferred() to
defer emitting the "enable focus reporting" termcode. tui_main() never
processes `tui_loop.events` (which loop_schedule_deferred() depends on),
so the event was never actually processed.
But fixing that (by processing `tui_loop.events`) would bring back the
problem 27f9b1c7b0 tried to fix: it still emits the event too soon.
Instead, do a little dance: schedule the event on `main_loop` and then
forward it to `tui_loop`.
NOTE: after this commit, in tmux 2.3 with `focus-events` enabled,
FocusGained is fired on startup and when resuming from suspend.
Using `script` to record the terminal session (and `vterm-dump` to
post-process the result):
BEFORE:
{DECSM 1049}{DECSM 1}{ESC =}
{CUP *}{ED *}{DECSM 2004}{DECSM 1004}{CSI 1,43 r}
{CUP 1,1}
{CUP *}{ED *}{SM 34}{DECSM 25}
{DECRM 25}{CSI 2 q}{CSI 2 q}
{CUP *}{ED *}{LF}
{SGR *}{LS1}{SGR 94}~
...
AFTER:
{CUP *}{ED *}{CSI 1,43 r}
{CUP 1,1}
{CUP *}{ED *}{SM 34}{DECSM 25}
{DECRM 25}{CSI 2 q}{CSI 2 q}
{CUP *}{ED *}{DECSM 2004}{DECSM 1004}{LF}
{SGR *}{LS1}{SGR 94}~
...
Note: there are three changes to ascii_isident. Reverting first two (in
find_special_key and first in get_special_key_code) normally fails the new test
with empty &isident, but reverting the third does not. Hence adding `>` to
&isident.
Ref vim/vim#2389.
- channels: vim-patch:8.0.0018
- GUI: vim-patch:8.0.0021
- Different recursive function implementation: vim-patch:8.0.0141
- JSON handling: vim-patch:8.0.0166, vim-patch:8.0.0169, vim-patch:8.0.0170,
vim-patch:8.0.0171, vim-patch:8.0.0180
Mark vim-patch:8.0.0096 applied, since it was added in
860ecd7055.
[ci skip]
Since "builtin" terminfo definitions were implemented (7cbf52db1b),
the decisions made by tui.c and terminfo.c are more relevant. Exposing
that decision in the 'term' option helps with troubleshooting.
Also: remove code that allowed setting t_Co. `:set t_Co=…` has never
worked; the highlight_spec test asserting that nvim_set_option('t_Co')
_does_ work makes no sense, and should not have worked.
If an EOF is returned from `fgets`, `vim_fgets` might spin forever, as
it tries to consume the current line.
A `NULL` return value from `fgets` should break out of the function
(unless `errno` is `EINTR`), and then `feof` should be used to check for
the EOF condition on the stream.
The calls to `fgets` in `src/nvim/if_cscope.c` (and elsewhere) can show
communication errors to the user if a signal is delivered during its
system calls. For plugins that proxy subprocess output into cscope
requests, a `SIGCHLD` might *always* interfere with calls into `fgets`.
To see this in a debugger, put a breakpoint on `cs_reading_emsg` and
watch signals come in (with lldb, using `process handle --notify true
--pass true`). Next, run a subcommand from neovim that calls through
cscope when it returns. A tag picker plugin, like vim-picker and fzy,
with `cscopetag` and `cscopetagorder=0` set, reproduced this reliably.
The breakpoint will hit after a `SIGCHLD` is delivered, and `errno` will
be set to 4, `EINTR`.
The caller of `fgets` should retry when `NULL` is returned with `errno`
set to `EINTR`.
On startup, if running in a terminal, save the termios properties.
Use the saved termios for `:terminal` and `jobstart()` pty jobs.
This won't affect nvim spawned outside of a terminal.
questions:
- This affects `:terminal` and `jobstart({'pty'✌️true})`.
Should we be more conservative for `jobstart({'pty'✌️true})` (e.g.
pass NULL to forkpty() and let the OS defaults prevail)?
- Note: `iutf8` would not be set in that case.
