Problem: With 'smoothscroll' the scroll position of a window is lost when
its height changes.
Solution: Only reset the skipped columns when 'smoothscroll' is off, where
they just serve to keep the cursor visible.
closes: vim/vim#2088517f3923b8c
Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem: When the only window has 'winfixheight' set and 'laststatus'
is one, splitting it leaves one screen line unused. This
happens for example when jumping to an item from a maximized
quickfix window (rendcrx)
Solution: Do not subtract the height of the status line twice
(Hirohito Higashi)
fixes: vim/vim#20495closes: vim/vim#20871ab36bcc870
Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem:
The object subsystem has an intermediate representation for no real
reason. Besides the code cost, this also adds an extra (api <=> OptVal)
conversion step, which is a (small) perf cost.
Solution:
We already have `Object`, so use it instead.
- drop `OptVal`, `OptValData`, `OptValType`, and related boilerplate.
- add `kObjectTypeUnset`.
Problem:
w_float_is_info conflates a window's role with its floating layout,
which are orthogonal properties.
Solution:
Replace w_float_is_info with a w_kind enum for the window's role.
Problem: #40731 may still crash if close_buffer autocmds reinsert the float's
grid. Plus removing the grid (and posting win_close) is unneeded if
win_close_othertab refuses to close the window later, which is possible.
Solution: do the stuff before freeing the window, like win_close.
Problem: Closing a floating window from a non-current tab frees its grid
without removing it from the compositor's `layers` table, so the next
`ui_comp_put_grid()` walks a dangling pointer (UAF).
Solution: Call `ui_comp_remove_grid()` (and `ui_call_win_close()` for
multigrid UIs) before `win_free_mem()`, matching `win_close()` since
PR #21551.
Problem:
Insert-mode globals are scattered.
Solution:
Group them into one struct. Besides improving readability and
discoverability, this greatly helps with save/restore of an insert-mode
session.
Problem:
- `edit.c` is an unnecessarily misleading and less-discoverable name
than `insert.c`.
- Numerous insert-mode related things are named with a `ins_` prefix.
- There is already a `insexpand.c` module...
Solution:
Rename `edit.c` => `insert.c`.
Problem:
`getchar.c` is a quirky name and doesn't align with our existing
`os/input.c` and `tui/input.c` modules, which encompass the same topic
at different layers.
Solution:
Rename `getchar.c` => `input.c` for discoverability.
Introduce `dialog.c` for functionality related to showing confirm
prompts, dialogs, etc.
Problem:
Changing global 'winbar' only updates window layout state in the current
tabpage. This means existing hidden tabs can keep stale winbar height.
Solution:
Recompute winbar state for all tabpages on global `'winbar'` changes.
Ref #6645
Problem:
When a window is resized it takes space from the window right/below first,
and only falls back to the window left/above when there is no more room.
Sometimes a user wants the space to come from a specific direction.
Solution:
Add nvim_win_resize(win, width, height, {anchor}) which resizes a window
with a choosable anchor edge, letting a window grow leftwards or upwards
by taking space from the window to the left or above first. The default
anchor reproduces nvim_win_set_width()/nvim_win_set_height().
Problem:
cmdwin (the `:q` cmdline buffer) has various limitations which require
special-casing all over the codebase.
Besides complicating the code, it also breaks async plugins if they try
to create buffers/windows after some work is done, if the user happens
to open cmdwin at the wrong the moment:
Lua callback: …/guh.nvim/lua/guh/util.lua:531:
E11: Invalid in command-line window; <CR> executes, CTRL-C quits
stack traceback:
[C]: in function 'nvim_buf_delete'
…/guh.nvim/lua/guh/util.lua:531: in function <…/guh.nvim/lua/guh/util.lua:526>
Solution:
Just say no to "inception". Reimplement cmdwin as a normal buffer+window.
All of the cmdwin contortions (in both core, and innocent plugins) exist
literally only to support "inception": recursive
cmdwin-in-cmdline-things, like `<c-r>=`, `/`, search-during-substitute,
`:input()`, etc. So we just won't support that (though I have
a potential plan for that later, which I call "modal parking lot").
The benefit is that plugins, and core, no longer have to care about
cmdwin.
BONUS:
- mouse-drag on vertical separators works (it only worked for
horizontal/statusline before)
- inccommand-in-cmdwin now works correctly, for free (thus don't need
#40077).
POTENTIAL FOLLOWUPS
- Drop `CHECK_CMDWIN` ("E11: Invalid in command-line window"), allow chaos.
- Unify `BUFLOCK_OK` / `LOCK_OK` ?
DESIGN:
- Eliminate lots of C globals, `EX_CMDWIN`, etc.
- `text_locked()` no longer reports true for cmdwin.
- cmdwin = a normal window with 'winfixbuf', 'bufhidden=wipe',
'buftype=nofile'. Invariants come from those options rather than
special cases throughout the codebase.
- `nv_record` for q:/q//q? calls Lua
`nlua_call_vimfn("vim._core.cmdwin", …)`. No `K_CMDWIN`
/ cmdline-reader detour.
- `cedit_key` (`c_CTRL-F`) schedules a deferred event that calls
`vim._core.cmdwin.open(type, content, pos)` and returns `Ctrl_C` so
the in-flight cmdline cancels. Reader state is not serialized; instead
the captured `(type, line, col)` is replayed via
`nvim_feedkeys(type..line.."<CR>", "nt", …)` after user confirms.
- On confirm/cancel: `<CR>` / `<C-C>` calls into Lua which closes the
window and re-feeds the cmdline.
BREAKING CHANGES:
- Expression-register cmdline (`<C-R>=` from insert-mode) no longer
supports cmdwin. Same applies to `input()` / `inputlist()` (already
covered by `text_locked`).
- Usage of cmdwin in macros/mappings will probably break (assuming they
ever worked).
Problem:
- Current 'statuscolumn' click label caveat is restrictive.
- v:virtnum is not unique to a line if it has both above and
below virtual lines.
- 'statuscolumn' click handler may expect v:virt/lnum to be set.
Solution:
- Store per-row click definitions for the statuscolumn in a
(nested line/virt number) map.
- Implement strategy that gives each 'statuscolumn' row a unique
v:virtnum.
- Set v:virt/lnum when determining which line is clicked.
Problem: Coverity warns for using a NULL pointer.
Solution: Check for memory allocaion failure.
96cbbe29de
Co-authored-by: Bram Moolenaar <Bram@vim.org>
Problem: Coverity warns about NULL pointer when triggering WinResized
Solution: Add OOM checks for windows_list like for scroll_dict. Remove
void casts that are unnecessary after 9.1.1084 (zeertzjq).
closes: vim/vim#16959b42b9fc41f
Co-authored-by: zeertzjq <zeertzjq@outlook.com>
Problem: If got_int is true when win_close() is called, it unexpectedly
fails in the branch that detects failure in apply_autocmds().
This causes wingotofile in do_window() to duplicate current
window when do_ecmd() is aborted with got_int.
Solution: Fix do_window() to save the got_int value before trying to
close the split window (Yohei Kojima).
Steps to reproduce:
1. run `touch a && touch .a.swp && echo a > b && vim b`
2. Type `<C-w>f`
3. In the warning dialogue, type `a` to abort
4. Current window is duplicated
closes: vim/vim#20382113e507cdd
Co-authored-by: Yohei Kojima <yk@y-koj.net>
Problem: Redrawing in an autocommand that uses an autocommand window
(more common since 5181984d) can result in flickering.
Solution: Hide autocommand windows.
Problem:
Followup to #39858. close_windows returned a holder window just so
do_buffer_ext could retry there, but the non-float branch was dead code
and close_windows was called twice.
Solution:
Check one_window directly in do_buffer_ext, drop the holder, make
close_windows void again.
Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Problem:
(Followup to 54f22a8f01c0feb27a531b52aedf5cdbd5e51b24.)
Deleting another buffer from a floatwin could move focus into the holder
window and fire BufEnter for the buffer being deleted.
Solution:
Use switch_win_noblock() instead of buf_jump_open_win() before
recursing into do_buffer_ext().
Problem:
2d795face6 added support for tab-local options ('cmdheight')
to `nvim_get_option_value`, but not to:
nvim_get_option_info2()
nvim_set_option_value(…, { tab = … })
gettabwinvar()
Solution:
- Update `options.lua` to model tab-local options. Introduce `kOptScopeTab`.
- Handle tab scope in the options layer so it works for all options APIs.
Note:
- No change to `gettabvar()`. Not sure if needed/wanted.
fix https://github.com/neovim/neovim/issues/31140
Problem:
No way to handle a "tab moved" event.
Use-case: tabline plugins may cache tab labels, and need to know when to
invalidate their cache.
Solution:
Add a `TabMoved` event that triggers whenever tabs are reordered via `:tabmove`
or via mouse click-and-drag.
Problem: win_fix_scroll(true) is called before win_comp_pos() in
command_height().
Solution: Move win_fix_scroll(true) after win_comp_pos(), matching the
ordering used in win_drag_status_line() (Jesse Rosenstock).
Patch 9.2.0413 added win_fix_scroll(true) to command_height() to handle
splitkeep when cmdheight changes, but placed the call before win_comp_pos().
win_fix_scroll() reads w_winrow to detect window movement
(620557bd48/src/window.c (L7266)),
but w_winrow is not recomputed until win_comp_pos() runs
(620557bd48/src/window.c (L6516)).
This causes incorrect scroll adjustments and was breaking
Test_smoothscroll_incsearch on macOS CI.
closes: vim/vim#2013840fc78f0a1
Co-authored-by: Gemini
Co-authored-by: Jesse Rosenstock <jmr@google.com>
Problem: Cursor is not adjusted when 'cmdheight' is changed to cover
the cursor with 'splitkeep' ~= "cursor".
Solution: Handle window resize for 'splitkeep' after changing 'cmdheight'.
Ensure previous window height is set when changing 'splitkeep'
(Luuk van Baal).
closes: vim/vim#20043bd0f3e6da5
Co-authored-by: Luuk van Baal <luukvbaal@gmail.com>
Problem: Cannot apply 'scrolloff' context lines at end of file
Solution: Add the 'scrolloffpad' option to keep 'scrolloff' context even
when at the end of the file (McAuley Penney).
closes: vim/vim#19040a414630393
Co-authored-by: McAuley Penney <jacobmpenney@gmail.com>
Problem:
- Unable to "pin" a window to prevent closing without specifically
being targeted.
- :fclose closes hidden windows (even before visible windows).
Solution:
- Add 'winpinned' window-local option. When set, window is skipped by
:fclose and :only. Pin the ui2 cmdline window (which should always be
visible), so that it is not closed by :only/fclose.
- Skip over hidden (and pinned) windows with :fclose.
Co-authored-by: glepnir <glephunter@gmail.com>
Problem: When closing floating windows to close a tabpage, if the current
buffer will unload, buffers contained in those floating windows
will too (unexpectedly).
Solution: Don't pass along "free_buf" argument; check 'bufhidden' for
the buffer in the to be closed float.
Problem: When 'ruler' is in last line of the screen and the current
floating window is closed, the ruler is not cleared.
Solution: When closing the current floating window, redraw the cmdline
if that contained, and will no longer contain the 'ruler'.
Problem: w_locked can be bypassed when recursively set if not restored
to its prior value.
Solution: Rather than save/restore everywhere, just make it a count,
like other locks (Sean Dewar)
Requires the previous commit, otherwise b_nwindows will be wrong in
tests, which causes a bunch of weird failures.
closes: vim/vim#197287cb43f286e
Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Problem: close_buffer() callers incorrectly handle b_nwindows,
especially after nasty autocmds, allowing it to go
out-of-sync. May lead to buffers that can't be unloaded, or
buffers that are prematurely freed whilst displayed.
Solution: Modify close_buffer() and review its callers; let them
decrement b_nwindows if it didn't unload the buffer. Remove
some now unneeded workarounds like 8.2.2354, 9.1.0143,
9.1.0764, which didn't always work (Sean Dewar)
(endless yapping omitted)
related: vim/vim#19728bf21df1c7b
b_nwindows = 0 change for free_all_mem() was already ported.
Originally Nvim returned true when b_nwindows was decremented before the end was
reached (to better indicate the decrement). That's not needed anymore, so just
return true only at the end, like Vim. (retval isn't used anywhere now anyways)
Set textlock for dict watchers at the end of close_buffer() to prevent them from
switching windows, as that can leave a window with a NULL buffer. (possible
before this PR, but the new assert catches it; added a test)
Despite textlock, things still aren't ideal, as watchers may observe the buffer
as unloaded and hidden (b_nwindows was decremented), yet still in a window...
Likewise, for Nvim, wipe_qf_buffer()'s comment may not be entirely accurate;
autocmds are blocked, but on_detach callbacks (textlocked) and dict watchers may
still run. Might be problematic, but those aren't new issues.
Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Problem: possible crash with winminheight=0
(Emilien Breton)
Solution: Use <= instead of < when checking reserved room in
frame_setheight() to correctly handle the zero-height
boundary case (Hirohito Higashi).
In frame_setheight(), when shrinking the current window and the only
other window has 'winfixheight' with 'winminheight'=0, room_reserved
was not cleared because the condition used '<' instead of '<='.
The freed rows were discarded, leaving fr_height sum less than
topframe fr_height. Subsequent resize operations then computed a
wrong room_cmdline that expanded topframe beyond the screen, causing
a crash.
fixes: vim/vim#19706closes: vim/vim#19712a5d9654620
Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
- Cleanup, remove redundant comments, add more tests.
- Enhance win_new_tabpage rather than create a new function for !enter, and use
a different approach that minimizes side-effects. Return the tabpage_T * and
first win_T * it allocated.
- Disallow during textlock, like other APIs that open windows.
- Remove existing win_alloc_firstwin error handling from win_new_tabpage; it's
not needed, and looks incorrect. (enter_tabpage is called for curtab, which is
not the old tabpage! Plus newtp is not freed)
- Fix checks after creating the tabpage:
- Don't fail if buf wasn't set successfully; the tab page may still be valid
regardless. Set buffer like nvim_open_win, possibly blocking Enter/Leave
events. (except BufWinEnter)
- tp_curwin may not be the initial window opened by win_new_tabpage. Use the
win_T * it returns instead, which is the real first window it allocated,
regardless of autocmd shenanigans.
- Properly check whether tab page was freed; it may have also been freed
before win_set_buf. Plus, it may not be safe to read its handle!
Problem:
`K` in help files may fail in some noisy text. Example:
(`fun(config: vim.lsp.ClientConfig): boolean`)
^cursor
Solution:
- `:help!` (bang, no args) activates DWIM behavior: tries `<cWORD>`,
then trims punctuation until a valid tag is found.
- Set `keywordprg=:help!` by default.
- Does not affect `CTRL-]`, that is still fully "tags" based.
Problem: more cases where it may not be safe to move a window between tabpages.
Solution: check them.
Rather speculative... I haven't spend much time looking, but I didn't find
existing code that sets these locks to skip checking win_valid. (what I did find
called it anyway, like in win_close) Still, I think it's a good precaution for
what future code might do.
If the fact that nvim_win_set_config *actually* moves windows between tabpages
causes unforeseen issues, "faking" it like ":wincmd T" may be an alternative:
split a new window, close the old one, but instead also block autocmds, copy the
old window's config, and give it its handle?
Problem: nvim_win_set_config may merge configs despite failing to configure a
split, and without applying necessary side-effects (like setting style=minimal
options). Plus, autocommands may apply a different config after the merge,
causing side-effects to apply for an outdated config.
Solution: merge configs last, only on success. Include fields only relevant to
splits. Properly set _cmdline_offset for splits.
Maybe better to disallow _cmdline_offset for splits instead, as the pum is
relative to cmdline_row anyway? (I didn't want to change behaviour too much)
Also use expect_unchanged in an unrelated test to quash a warning.
Problem: 5943a81 skips saving window options in `buflist_altfpos` for
style=minimal windows, which also prevents the window from restoring its own
options when switching buffers.
Solution: revert the change. In `get_winopts`, don't restore options from the
`WinInfo` of style=minimal windows when reusing values for a different window.
In `win_free`, clear `wi_optset` for minimal windows.
