Problem Description: In ex_mode, the default_grid.chars are not allocated, and subsequently,
the w_grid.target in curwin is not allocated to default_grid in update_screen. This leads to
a null pointer crash when the completion function is executed in ex_mode.
Solution: Set full_screen when in ex_mode to ensure that default_grid is allocated.
Problem: :close may cause Nvim to quit if an autocommand triggered when
closing the buffer closes all other non-floating windows and
there are floating windows.
Solution: Correct the check for the only non-floating window.
Its corresponding test in Vim is in test_popupwin.win, so having it in
the middle of test_window_cmd.vim is strange, and it is now covered by
tests in ui/float_spec.lua anyway.
Problem: nvim_win_set_config does not update statuslines after removing a split.
Solution: call last_status.
Didn't realize this was missing in the original nvim_win_set_config for splits
PR.
As it can only be done for the current tabpage, do it if win_tp == curtab;
enter_tabpage will eventually call last_status anyway when the user enters
another tabpage.
Problem: there are new ways to escape textlock or break the cmdwin in
nvim_win_set_config and nvim_tabpage_set_win.
Solution: fix them. Use win_goto to check it in nvim_tabpage_set_win and use the
try_start/end pattern like with similar functions such as nvim_set_current_win
(which uses the existing msg_list, if set).
Careful not to use `wp->handle` when printing the window ID in the error message
for nvim_tabpage_set_win, as win_goto autocommands may have freed the window.
On a related note, I have a feeling some API functions ought to be checking
curbuf_locked...
Problem: nvim_win_set_config does not handle failure in win_split_ins properly
yet, which can cause all sorts of issues. Also nvim_open_win and
nvim_win_set_config do not set the error message to the one from win_split_ins.
Solution: handle failure by undoing winframe_remove, like in win_splitmove.
Make sure autocommands from switching to the altwin fire within a valid window,
and ensure they don't screw things up. Set the error message to that of
win_split_ins, if any.
Also change a few other small things, including:
- adjust win_append to take a tabpage_T * argument, which is more consistent
with win_remove (and also allows us to undo a call to win_remove).
- allow winframe_restore to restore window positions. Useful if `wp` was in a
different tabpage, as a call to win_comp_pos (which only works for the current
tabpage) after winframe_restore should no longer be needed.
Though enter_tabpage calls win_comp_pos anyway, this has the advantage of
ensuring w_winrow/col remains accurate even before entering the tabpage
(useful for stuff like win_screenpos, if used on a window in another tabpage).
(This change should probably also be PR'd to Vim later, even though it doesn't
use winframe_restore for a `wp` in a different tabpage yet).
Problem: heap-use-after-free in win_splitmove if Enter/Leave
autocommands from win_split_ins immediately closes "wp".
Solution: check that "wp" is valid after win_split_ins.
(Sean Dewar)
abf7030a5c
Problem: win_gotoid() checks for textlock and other things when switching
to a window that is already current (after v9.1.0119)
Solution: return early with success when attempting to switch to curwin
(Sean Dewar)
2a65e73944
Problem: infinite loop in win_update with 'smoothscroll' set when
window width is equal to textoff, or signed integer overflow
if smaller.
Solution: don't revalidate wp->w_skipcol in that case, as no buffer text
is being shown. (Sean Dewar)
02fcae02a9
Test_window_split_no_room changes were already cherry-picked earlier.
Problem: can switch windows while textlocked via f_win_gotoid and
f_win_splitmove (which also allows switching in the cmdwin).
Solution: Check text_or_buf_locked in f_win_splitmove()
(Sean Dewar)
While at it, call text_or_buf_locked() in f_win_gotoid() instead of testing for
cmdwin_type() (which text_buf_locked() does and in addition will also verify
that the buffer is not locked).
f865895c87
Problem: saving and restoring all frames to split-move is overkill now
that WinNewPre is not fired when split-moving.
Solution: defer the flattening of frames until win_split_ins begins
reorganising them, and attempt to restore the layout by
undoing our changes. (Sean Dewar)
704966c254
Adjust winframe_restore to account for Nvim's horizontal separators when the
global statusline is in use. Add a test.
Problem: win_splitmove fires WinNewPre and possibly WinNew when moving
windows, even though no new windows are created.
Solution: don't fire WinNew and WinNewPre when inserting an existing
window, even if it isn't the current window. Improve the
accuracy of related documentation. (Sean Dewar)
96cc4aef3d
Partial as WinNewPre has not been ported yet (it currently has problems anyway).
Problem: win_split_ins has no check for E36 when moving an existing
window
Solution: check for room and fix the issues in f_win_splitmove()
(Sean Dewar)
0fd44a5ad8
Omit WSP_FORCE_ROOM, as it's not needed for Nvim's autocmd window, which is
floating. Shouldn't be difficult to port later if it's used for anything else.
Make win_splitmove continue working for turning floating windows into splits.
Move the logic for "unfloating" a float to win_split_ins; unlike splits, no
changes to the window layout are needed before calling it, as floats take no
room in the window layout and cannot affect the e_noroom check.
Add missing tp_curwin-fixing logic for turning external windows into splits, and
add a test.
NOTE: there are other issues with the way "tabpage independence" is implemented
for external windows; namely, some things assume that tp_curwin is indeed a
window within that tabpage, and as such, functions like tabpage_winnr and
nvim_tabpage_get_win currently don't always work for external windows (with the
latter aborting!)
Use last_status over frame_add_statusline, as Nvim's last_status already does
this for all windows in the current tabpage. Adjust restore_full_snapshot_rec to
handle this.
This "restore everything" approach is changed in a future commit anyway, so only
ensure it's robust enough to just pass tests.
Keep check_split_disallowed's current doc comment, as it's actually a bit more
accurate here. (I should probably PR Vim to use this one)
Allow f_win_splitmove to move a floating "wp" into a split; Nvim supports this.
Continue to disallow it from moving the autocommand window into a split (funnily
enough, the check wasn't reachable before, as moving a float was disallowed),
but now return -1 in that case (win_splitmove also returns FAIL for this, but
handling it in f_win_splitmove avoids us needing to switch windows first).
Cherry-pick Test_window_split_no_room fix from v9.1.0121.
Update nvim_win_set_config to handle win_split_ins failure in later commits.
Problem: nvim_open_win blocking all win_set_buf autocommands when !enter &&
!noautocmd is too aggressive.
Solution: temporarily block WinEnter/Leave and BufEnter/Leave events when
setting the buffer. Delegate the firing of BufWinEnter back to win_set_buf,
which also has the advantage of keeping the timing consistent (e.g: before the
epilogue in enter_buffer, which also handles restoring the cursor position if
autocommands didn't change it, among other things). Reword the documentation for
noautocmd a bit.
I pondered modifying do_buffer and callees to allow for BufEnter/Leave being
conditionally disabled, but it seems too invasive (and potentially error-prone,
especially if new code paths to BufEnter/Leave are added in the future).
Unfortunately, doing this has the drawback of blocking ALL such events for the
duration, which also means blocking unrelated such events; like if window
switching occurs in a ++nested autocmd fired by win_set_buf. If this turns out
to be a problem in practice, a different solution specialized for nvim_open_win
could be considered. :-)
Problem: currently, for splits, nvim_win_set_config accepts win without any of
split or vertical set, which has little effect and seems error-prone.
Solution: require at least one of split or vertical to also be set for splits.
Also, update nvim_win_set_config docs, as it's no longer limited to just
floating and external windows.
Problem: splitting is disallowed in some cases to prevent the window layout
changes while a window is closing, but it's not checked for.
Solution: check for this, and set the API error message directly.
(Also sneak in a change to tui.c that got lost from #27352; it's a char* buf,
and the memset is assuming one byte each anyway)
Problem: WinNew and win_enter autocommands can delete the target buffer to
switch to, causing a heap-use-after-free.
Solution: store a bufref to the buffer, check it before attempting to switch.
Problem: if switch_win{_noblock} fails to restore the old curwin after WinNew
(e.g: it was closed), wp will become the new curwin, but win_set_buf enter
events would still be blocked if !enter && !noautocmd.
Solution: fire them, as we've actually entered the new window.
Note: there's a problem of switch_win{_noblock} failing to restore the old
curwin, leaving us in wp without triggering WinEnter/WinLeave, but this affects
all callers of switch_win{_noblock} anyways. (It's also not clear how WinLeave
can be called if the old curwin was closed already).
Problem: BufWinEnter is not fired when not entering a new window, even when a
different buffer is specified and buffer-related autocommands are unblocked
(!noautocmd).
Solution: fire it in the context of the new window and buffer. Do not do it if
the buffer is unchanged, like :{s}buffer.
Be wary of autocommands! For example, it's possible for nvim_win_set_config to
be used in an autocommand to move a window to a different tabpage (in contrast,
things like wincmd T actually create a *new* window, so it may not have been
possible before, meaning other parts of Nvim could assume windows can't do
this... I'd be especially cautious of logic that restores curwin and curtab
without checking if curwin is still valid in curtab, if any such logic exists).
Also, bail early from win_set_buf if setting the temp curwin fails; this
shouldn't be possible, as the callers check that wp is valid, but in case that's
not true, win_set_buf will no longer continue setting a buffer for the wrong
window.
Note that pum_create_float_preview also uses win_set_buf, but from a glance,
doesn't look like it properly checks for autocmds screwing things up (win_enter,
nvim_create_buf...). I haven't addressed that here.
Also adds some test coverage for nvim_open_win autocommands.
Closes#27121.
Problem: win_set_config should have the observable effect of moving an existing
window to another place, but instead fires autocommands as if a new window was
created and entered (and does not fire autocommands reflecting a "return" to the
original window).
Solution: do not fire win_enter-related autocommands when splitting the window,
but continue to fire them when entering the window that fills the new space when
moving a window to a different tabpage, as the new curwin changes.
Also, remove "++once" from the WinEnter autocmd in the other test, as omitting
it also crashed Nvim before this fix.
Problem: win_enter autocommands can close new_curwin, crashing if it was the
last window in its tabpage after removing win, or can close parent, crashing
when attempting to split it later.
Solution: remove win first, check that parent is valid after win_enter.
NOTE: This isn't actually quite right, as this means win is not in the window
list or even has a frame when triggering enter autocommands (so it's not
considered valid in the tabpage). This is addressed in later commits.
`_FORTIFY_SOURCE` on Ubuntu caught this, resulting in:
[OLDTEST] Running test_rename
Failed: test_rename :: Nvim exited with non-zero code
Job exited with code 134
Screen (23 lines)
================================================================================
"test_rename.vim" "test_rename.vim" 120L, 3623B
Executing Test_rename_copy()
Executing Test_rename_dir_to_dir()
Executing Test_rename_fails()
Error detected while processing command line..script /<<BUILDDIR>>/neovim-0.9.5/test/old/testdir/runtest.vim[437]..function RunTheTest[44]..Test_rename_fails:
line 17:
E730: using List as a String
line 18:
E976: using Blob as a String
Executing Test_rename_file_ignore_case()*** buffer overflow detected ***: terminated
`snprintf`'s second parameter should be no greater than the number of
remaining bytes in the allocated object. We can see that this was off
by one, because in the simple case where `tail == tempname` (for a file
in the current directory), `rename_with_tmp` was passing `MAXPATHL + 2`
for an object allocated with a size of only `MAXPATHL + 1`.
Introduced in 5f1a153831.
Setting the label `ci:skip-news` will skip the job. This is useful for
maintainers to indicate to contributors that a feature isn't big enough
to warrant a news entry, or for contributors who dislike red CI even if
there's nothing wrong.
Also change label `ci-s390x` to `ci:s390x`; this way it'll be easier to
see that `ci:` are a subcategory of labels that affect CI in some way.
Just some basic spring cleaning.
In the distant past, not all UI:s where remote UI:s. They still aren't,
but both of the "UI" and "UIData" structs are now only for remote UI:s.
Thus join them as "RemoteUI".
Problem: Various warnings from clang on MS-Windows.
Solution: Avoid the warnings. (Yegappan Lakshmanan, closesvim/vim#10553)
a34b4460c2
Co-authored-by: Yegappan Lakshmanan <yegappan@yahoo.com>
On exit, pty_process_close() may be called after pty_process_finish1()
but before start_wait_eof_timer(), in which case the timer shouldn't be
started because pty_process_close() has already closed it.
Problem: 'shortmess' "F" flag doesn't work properly with 'autoread'
(after 9.1.0154)
Solution: Hide the file info message instead of the warning dialog
(zeertzjq)
closes: vim/vim#14159closes: vim/vim#141588a01744c56
Problem: Duplicate assignment in f_getregion().
Solution: Remove the duplicate assignment. Also improve getregion()
docs wording and fix an unrelated typo (zeertzjq)
closes: vim/vim#141540df8f93bda
Before, we needed to always pack an entire msgpack_rpc Object to
a continous memory buffer before sending it out to a channel.
But this is generally wasteful. it is better to just flush
whatever is in the buffer and then continue packing to a new buffer.
This is also done for the UI event packer where there are some extra logic
to "finish" of an existing batch of nevents/ncalls. This doesn't really
stop us from flushing the buffer, just that we need to update the state
machine accordingly so the next call to prepare_call() always will
start with a new event (even though the buffer might contain overflow
data from a large event).
Problem: can only call getregion() for current buffer
Solution: Allow to retrieve selections from different buffers
(Shougo Matsushita)
closes: vim/vim#1413184bf6e658d
Co-authored-by: Shougo Matsushita <Shougo.Matsu@gmail.com>
