[vim/vim] Add 'stickybuf' support (PR #13903)
Colin Kennedy
Closes: #6445
Summary
This PR adds a new "local to window" option called 'stickybuf'. When a window has :set stickybuf applied, any command, mapping, etc like :edit that would normally change the window to look at another buffer will fail or have some other alternate behavior.
This PR impacts many Vim commands and mappings. To make reviewing easier, I've grouped affected commands and mappings into groups which I'll explain below.
New Error
For any command / mapping that must fail, E922 + a message is printed to the user.
All 'stickybuf' Cases
Fail Or Allow With [!] (forceit)
Any command in this list may potentially change a window's current buffer. When 'stickybuf' is present, these :foo commands will fail with an error message. Users can still call :foo! to force the command.
:Next:argdo:argedit:args- Note: This jumps for the first args when called. Maybe
:argsshould be
in the "Allow Command But Don't Swap" section, instead?
- Note: This jumps for the first args when called. Maybe
:bNext:bfirst:blast:bmodified:bnext:bprevious:brewind:bufdo:buffer:djump:edit:enew:ex:find:first:ijump:lNext:lNfile:last:ldo:ll:llast:lnext:lnfile:lpfile:lprevious:lrewind:ltag:next:pop:previous:rewind:tNext:tag:tfirst:tjump:tlast:tnext:tprevious:trewind- Note: No unittests:view:visual:wNext:wnext:wprevious
Disabled Commands
These commands are completely disallowed and have no [!] alternative.
:drop- @maintainers - Would you ...
- like to add [!] to this command so 'stickybuf' can be forced?
- or split a new window?
Disabled Mappings
These mappings are completely disallowed and have no alternative.
:normal \<C-]>:normal \<C-^>:normal \<C-t>:normal ]\<C-d>:normal ]\<C-i>:normal g\<C-]>:normal g]:normal gt
Fully Allowed Commands
These commands are "Commands that I originally thought could be affected by 'stickybuf' but aren't so they've been left alone".
:badd:balt:windo
Auto-Find/Create 'nostickybuf' Window
These commands are always allowed, even with 'stickybuf'. When a window with 'stickybuf' would be affected by the command, either a nearby 'nostickybuf' window is chosen instead or a brand new split window is created and that window is used. So no matter what, these commands never touch a 'stickybuf' window's current buffer.
:cNext:cNfile:cc / <Enter>:cdo:cfirst:clast:cnext:cprevious:cnfile:cpfile:crewind:tabdo
Allowed Because It Makes A New Window
These commands are always allowed because they always operate on a new window. A 'stickybuf' window that splits into a new window defaults to 'nostickybuf' so these commands don't need to consider 'stickybuf' at all.
:normal \<C-w>]:pedit:sblast:sbprevious
Special
Allow Command But Don't Swap
The commands below generally are "do X and then jump to the first error". For example, see :help :cexpr. The commands have been modified to be "do X and then jump to the first error unless the window is 'stickybuf'. Users can still jump to the first error if they include [!].
:cbuffer:cfile:cexpr:lbuffer:lfile:lvimgrep:lvimgrepadd:vimgrep:vimgrepadd
:browse
:browse calls another command, like :browse edit. This command fails if the delegating command would change the current buffer. But :browse edit! is okay.
I wrote 2 tests for :browse while in Neovim, which passed, but when I copied them to Vim, it errors with:
Caught exception in Test_browse_edit_fail(): Vim(edit):E338: Sorry, no file browser in console mode @ command line..script /home/selecaoone/repositories/vim/src/testdir/runtest.vim[607]..function RunTheTest[57]..Test_browse_edit_fail[10]..<SNR>6_execute_try_catch, line 4
So I removed those tests for Vim. If curious, these were the tests.
Click to see the tests" Fail :browse edit but :browse edit! is allowed func Test_browse_edit_fail() call s:reset_all_buffers() let l:other = s:make_buffer_pairs() let l:current = bufnr() let l:caught = s:execute_try_catch("browse edit other") call assert_equal(1, l:caught) call assert_equal(l:current, bufnr()) let l:caught = s:execute_try_catch("browse edit! other") call assert_equal(0, l:caught) call assert_equal(l:other, bufnr()) endfunc " Allow :browse w because it doesn't change the buffer in the current file func Test_browse_edit_pass() call s:reset_all_buffers() let l:other = s:make_buffer_pairs() let l:current = bufnr() let l:caught = s:execute_try_catch("browse write other") call assert_equal(0, l:caught) call delete("other") endfunc
:vimgrep vs :grep
The [!] has different meanings for :vimgrep! and :grep! which leads to some odd results when we take into account 'stickybuf'.
:vimgrep- Do search + swap to the results:vimgrep!- Do search + swap to the results:grep- Do search + swap to the results:grep!- Do search + do not swap to the results
With these commands, 'stickybuf' currently modifies the commands like this:
:vimgrep- Do search + do not swap to the results:vimgrep!- Unchanged from the original:grep- Do search + do not swap to the results:grep!- Unchanged from the original
It's a bit weird that, with 'stickybuf' enabled, :grep and :grep! don't swap to the found results ever but :vimgrep! does. Maybe that's a sign that these should be splitting a window, instead?
Allow Intra-Buffer Moves
If the command / mapping does not change the current buffer, then it is allowed. For example, moving from one Vim mark a buffer to another Vim mark in the same buffer is okay. But if the current 'stickybuf' window would have its buffer changed, the command / mapping is disabled and prints an error message to the user.
- Vim marks (e.g. `M / 'M)
:normal \<C-i>:normal \<C-o>:normal gF:normal gf:normal [f:normal ]f:windo
Unaffected Commands / Mappings
Every command / mapping in this section was considered at one point while implementing 'stickybuf' but was ignored because the command / mapping either doesn't affect the window's current buffer or some other characteristic that made it harmless. If the reason is special, each line will say why it was ignored.
:pedit- This is not by stickybuf because it edits in a new window
- Split windows are
'nostickybuf'by default so it's okay
- Commands that do not change the current buffer
- Quickfix / Location List
:cabove:cafter:cbefore:cbelow:cbottom:cgetbuffer:cgetexpr:cnewer:colder:ctop:labove:lafter:lbefore:lbelow:lbottom:lgetbuffer:lgetexpr:lnewer:lolder:normal gD:normal gd
- Error-related
:caddbuffer:caddfile:cgetfile:laddbuffer:laddfile:lgetfile
- Miscellaneous
:goto
- Quickfix / Location List
- Mappings that spawn a new window / split
:ball:dsplit:help v_K:normal K:normal \<C-W>\<C-F>:normal \<C-W>\<C-]>:normal gK
- Commands that spawn a new window / split
:sbNext:sbfirst:sblast:sbmodified:sbnext:sbprevious:sbrewind:sbuffer:stjump:vertical
- Tags (in a Preview window)
:ptNext:ptfirst:ptjump:ptlast:ptnext:ptprevious:ptrewind:ptselect
- Commands that close windows (and don't attempt to change a window's buffer)
:bdelete:cclose:close
- Commands that show a previously-hidden stickybuf window
- (hidden with
:hide) :unhide:sunhide
- (hidden with
- Mappings that keep the window-buffer pairs the same
:normal gT:normal gt
No Unittests (Do You Want Them?)
These commands / mappings seemed harmless so I skipped writing unittests for them. I'm just mentioning it here for completeness. Do you want unittests for these or are we okay to skip them?
:caddexpr:cfdo- This seemed to be the same as :cdo so I figured those unittests cover it:laddexpr:lexpr:lfdo- This seemed to be the same as :ldo so I figured those unittests cover it:normal [\<C-D>- No unittests because a unittest for:normal ]\<C-D>exists:normal [\<C-I>- No unittests because a unittest for:normal ]\<C-I>exists:normal \<C-RightMouse>:normal g\<RightMouse>:popv_CTRL-]
Important Questions For Reviewers
- What should this window attribute be called? I called it
'stickybuf'when I first created the branch because originally the option was a "local to buffer" option but'stickybuf'is "local to window" now. Should we go with @bfrg suggestion and use'winfixbuf'? - Would you like any section to merge into any other section? For example, should the items under "Allow Command But Don't Swap" go into "Auto-Find/Create 'nostickybuf' Window"?
- I read through
:help quickrefwhen deciding what commands / mappings to change. I'm not super confident that I caught every mapping so please, if you think I missed any mapping, please let me know so we can add any needed changes and unittests for it to this PR - For every command / mapping that creates a split window in order to avoid
stopping with an error, e.g. the "Auto-Find/Create 'nostickybuf' Window"
section ...- Currently I just split the window horizontally. Do we want this split behavior to be customizable? Maybe with
'switchbuf'or'splitkeep'or something similar?
- Currently I just split the window horizontally. Do we want this split behavior to be customizable? Maybe with
:drophas no [!] which means'stickybuf'would disable that command. Would you like to add [!] for it?- When
'stickybuf'is applied, several mappings are completely disabled. Such as<C-^>- Maybe these should split a new window, instead? Or something else?
- When a user makes a split of a window with 'stickybuf' applied, do they want the copy to also have stickybuf? I assume no, right? After all if they're making a split, they probably want to navigate away from the current buffer that they're looking at.
- A continuation from 7. When creating a new tab based on the current window, such as
:tabedit, should'stickybuf'be copied to the new tab? - What should I do about the
ex_makefamily of commands such as:make,:lmake,:grep,:grepadd, etc? Their current logic is "If:foo, do X and jump to the first error". If:foo!, don't jump to the first error. So should I error on:grepaddbut allow:grepadd!or would you rather I- make a split window on
:grepaddand not split - allow
:grepadd!? - Or something completely different?
- make a split window on
- If a command like :buffer targets the same buffer with 'stickybuf' applied,
should Vim...- Pass silently?
- If yes then the "Fail Or Allow With [!] (forceit)" Section will need
to double their unittests for the pass case.
- If yes then the "Fail Or Allow With [!] (forceit)" Section will need
- Or error with E922 anyway
- Note: This is currently what this PR is doing
- Pass silently?
- What should happen if a user calls
:nextwith an args list like[first] first second thirdand the window is'stickybuf'? Should it successfully do the action and change the args list tofirst [first] second thirdsince the 0th an 1st indices of the arg list are the same buffer? Or should this call error?
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/13903
Commit Summary
- ca1b865 Added initial files
- 62222fc Removed newlines
- 63ca61f Added more 'stickybuf' related code
- a5e8876 Added WIP test_stickybuf.vim
- a85b8f7 Fixed if condition whitespace
- 94af302 Added more 'stickybuf' tests, for quickfix and location lists
- 9b849b0 Got compile to complete
- a7ac37a Updated test_stickybuf.vim - Test_grep now doesn't block make test
- 46b55c0 Changed exarg_T call to be C-style
- 6f4caf9 Changed exarg_T call to be C-style
- 848d47b Changed the 'stickybuf' error code from E969 to E922
- 04137a5 Moved curly to the next line
- c4a161b Added E922 to runtime/doc/tags
- e7a631e Moved curlies to the next line
- 24d00ad Fixed test_options unittest
File Changes
(29 files)
- M runtime/doc/message.txt (6)
- M runtime/doc/options.txt (9)
- M runtime/doc/quickref.txt (1)
- M runtime/doc/tags (2)
- M runtime/doc/tagsrch.txt (29)
- M runtime/optwin.vim (3)
- M src/arglist.c (12)
- M src/buffer.c (9)
- M src/errors.h (1)
- M src/ex_cmds.c (6)
- M src/ex_cmds2.c (27)
- M src/ex_docmd.c (28)
- M src/insexpand.c (2)
- M src/normal.c (15)
- M src/option.c (1)
- M src/option.h (1)
- M src/optiondefs.h (4)
- M src/po/cs.cp1250.po (2)
- M src/po/pl.cp1250.po (2)
- M src/po/ru.cp1251.po (592)
- M src/po/sk.cp1250.po (2)
- M src/proto/search.pro (2)
- M src/quickfix.c (46)
- M src/search.c (7)
- M src/structs.h (2)
- M src/tag.c (12)
- M src/testdir/Make_all.mak (1)
- A src/testdir/test_stickybuf.vim (2615)
- M src/window.c (2)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
> @@ -13,7 +13,7 @@ msgstr "" "Last-Translator: Ji�� Pavlovsk� <jpavl...@mbox.vol.cz>\n" "Language-Team: Czech <c...@li.org>\n" "MIME-Version: 1.0\n" -"Content-Type: text/plain; charset=cp1250\n" +"Content-Type: text/plain; charset=CP1250\n"
Note to reviewers: I have not touched any of the .po files, the changes were all auto-generated. I'm happy to change or revert anything in the .po files.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
That is ... quite a change.
One should/would think twice and weigh the benefits of stickybuf.
Why gt is disallowed but :tabnew/:tabnext/g<tab> etc are ok?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
There is also <C-w>T that takes current window into a new tab.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Why gt is disallowed but :tabnew/:tabnext/g etc are ok?
My bad, that's a documentation problem. gt is allowed. It's actually <C-]>, which changes the window to look at a tag, that is subject to 'stickybuf'. I'll update the PR description and code to reflect it.
There is also T that takes current window into a new tab.
I think <C-w>T is fine because it's moving the window and buffer together into a new tab, not forcing the window to look at a new buffer. But I could be wrong. I don't use that mapping much.
@habamax I'm not sure if you've been looking at the discussion on vim-dev but it sounds like we'll be moving in a more "Make a split, don't fail" direction overall for this PR. But I don't want to speak for @chrisbra.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- 0db053b Fixed test_stickybuf.vim function docstring
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/arglist.c:
> @@ -683,6 +683,12 @@ do_argfile(exarg_T *eap, int argn)
char_u *p;
int old_arg_idx = curwin->w_arg_idx;
+ if (!eap->forceit && curwin->w_p_stb)
if (!eap->forceit && curwin->w_p_stb)
This part with the error message should be in a seperat function so you do not duplicate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,33 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_stb)
+ {
+ if (eap->cmdidx == CMD_ldo && !eap->forceit)
+ {
+ // Disallow :ldo if 'stickybuf' is applied
+ semsg(_("E922: Cannot edit buffer. 'switchbuf' is enabled. Use ! to force it."));
Use a different error number here please, or unify the error messages to be always the same
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -13,7 +13,7 @@ msgstr "" "Last-Translator: Ji�� Pavlovsk� <jpavl...@mbox.vol.cz>\n" "Language-Team: Czech <c...@li.org>\n" "MIME-Version: 1.0\n" -"Content-Type: text/plain; charset=cp1250\n" +"Content-Type: text/plain; charset=CP1250\n"
you can reset that file (or copy it back from the main branch).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/arglist.c:
> @@ -683,6 +683,12 @@ do_argfile(exarg_T *eap, int argn)
char_u *p;
int old_arg_idx = curwin->w_arg_idx;
+ if (!eap->forceit && curwin->w_p_stb)
To make sure I understand your suggestion, do you want a common function that contains both if-logic + an error message that prints to Vim on-fail and change the code like ...
int is_allowed_to_go_to_buffer(exarg_T *eap) {
if (!eap->forceit && curwin->w_p_stb
)
{
semsg(_("E922: Cannot go to buffer. 'switchbuf' is enabled. Use ! to force it."))
return FALSE;
}
return TRUE;
}Then in callers ...
if (!is_allowed_to_go_to_buffer(eap) return;
Is this along the lines of what you wanted or did you want something different? If we change the signature to int is_allowed_to_go_to_buffer(int forceit) we could also probably use the same function in other .c files too
Related to this, I was also wondering if you'd like any of the currently hard-coded messages to be put into src/errors.h
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/arglist.c:
> @@ -683,6 +683,12 @@ do_argfile(exarg_T *eap, int argn)
char_u *p;
int old_arg_idx = curwin->w_arg_idx;
+ if (!eap->forceit && curwin->w_p_stb)
yes to both please :)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 4 commits.
- 2797298 Added is_allowed_to_go_to_buffer
- dcb4cd7 Reverted the accidentally changed .po files
- f6e4d9d Removed a hard-coded error message
- 7b1c1db Got compile to work again
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/arglist.c:
> @@ -683,6 +683,12 @@ do_argfile(exarg_T *eap, int argn)
char_u *p;
int old_arg_idx = curwin->w_arg_idx;
+ if (!eap->forceit && curwin->w_p_stb)
This is now done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,33 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_stb)
+ {
+ if (eap->cmdidx == CMD_ldo && !eap->forceit)
+ {
+ // Disallow :ldo if 'stickybuf' is applied
+ semsg(_("E922: Cannot edit buffer. 'switchbuf' is enabled. Use ! to force it."));
I don't know the history or conventions of Vim error codes but if E1513+ is okay, I'll append to those.
I tried to find an existing gap in the error codes but the only 3 contiguous error codes that were open are E1045-E1046 and E1412-E1499, which appear to be reserved for Vim9 related things. And E56-E59 too, I suppose
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)⬇️ Suggested change
-int is_allowed_to_go_to_buffer(int forceit) +int check_can_go_to_buffer(int forceit)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)
Since goto is used a lot in this repository and this function doesn't have much to do with that, I'd like to avoid potentially confusing people. How about check_can_change_buffer?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)
Ah, it doesn't accept a buffer argument. check_can_switch_buffer then.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)
Do you think check_can_switch_buffer is too close to 'switchbuf'? I'm happy to go with check_can_switch_buffer if that works for you
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)
Maybe check_can_set_curbuf?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -156,6 +156,22 @@ log_frame_layout(frame_T *frame) } #endif +/* + * Check if the current window is allowed to move to a different buffer. + * If the window has 'stickybuf', then forceit must be TRUE or this function + * will return FALSE. + */ +int is_allowed_to_go_to_buffer(int forceit)
I dig it, let's go with check_can_set_curbuf
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- abf8753 Renamed is_allowed_to_go_to_buffer to check_can_set_curbuf
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/errors.h:
> @@ -2397,7 +2397,6 @@ EXTERN char e_io_file_requires_name_to_be_set[]
#ifdef FEAT_EVAL
EXTERN char e_invalid_callback_argument[]
INIT(= N_("E921: Invalid callback argument"));
-// E922 unused
// E922 unused
It is still (or again) unused :)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,33 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_stb)
+ {
+ if (eap->cmdidx == CMD_ldo && !eap->forceit)
+ {
+ // Disallow :ldo if 'stickybuf' is applied
+ semsg("%s", e_stickybuf_cannot_go_to_buffer_forceit);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ {
you can remove the curly braces for single statements after the if
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_docmd.c:
> @@ -7245,6 +7248,13 @@ ex_open(exarg_T *eap)
static void
ex_edit(exarg_T *eap)
{
+ if (// Exclude commands which keep the window's current buffer
move comment to a separate line above the if, looks strange
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
I think it looks quite good already. I left a few comments, mainly style related. I'll check your detailed questions for Reviewers later.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- 6cfdea7 Removed unneeded {}s from .c files
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Okay, the following:
:drop
https://github.com/maintainers - Would you ...
like to add [!] to this command so 'stickybuf' can be forced?
or split a new window?
I think :drop already split the window if the current buffer can't be abandoned. So it should behave the same for 'stickybuf' windows.
Disabled Mappings
(plus a few other mappings like [_CTRL-D, [_CTRL-I, etc
It's not really mappings but default keybindings. I suppose that failing is fine. I guess we already do this, if hidden is not set and trying to jump to another buffer.
Auto-Find/Create 'nostickybuf' Window
That should be fine
Allowed Because It Makes A New Window
That is fine
Allow Command But Don't Swap
Mostly fine. Do we get an error message if those commands do not swap? Or perhaps just a warning message instead, so that one knows, why the command did not jump to an error message from the quickfix window.
:browse
should be fine
Caught exception in Test_browse_edit_fail(): Vim(edit):E338: Sorry, no file browser in console mode @ command line..script /home/selecaoone/repositories/vim/src/testdir/runtest.vim[607]..function RunTheTest[57]..Test_browse_edit_fail[10]..<SNR>6_execute_try_catch, line 4
That is interesting, it should open in netrw I guess. Did you disable netrw plugin?
:vimgrep vs :grep
Here I am not sure. the ! does not seem to be documented, but it seems to mean, that force switching to the buffer, even if it would mean to abandon any current changes (see :h E37). So I would not like to change the meaning of ! and just make it fail the command, for sticky buffers unless the ! was given (in which case it should switch).
Allow Intra-Buffer Moves
That should be fine
Unaffected Commands / Mappings
That seems fine
No Unittests (Do You Want Them?)
if possible, yes please
What should this window attribute be called? I called it 'stickybuf' when I first created the branch because originally the option was a "local to buffer" option but 'stickybuf' is "local to window" now. Should we go with @bfrg suggestion and use 'winfixbuf'?
I thought about it. stickybuf sounds a bit sloppy, and I thought about persistentbuf or similar, but not sure if that is any better. 🤷♀️
Would you like any section to merge into any other section? For example, should the items under "Allow Command But Don't Swap" go into "Auto-Find/Create 'nostickybuf' Window"?
Not that is fine.
I read through :help quickref when deciding what commands / mappings to change. I'm not super confident that I caught every mapping so please, if you think I missed any mapping, please let me know so we can add any needed changes and unittests for it to this PR
Yeah, if we missed anything, we can do it in a followup PR then. Don't worry, I see you spend quite a bit of time abut all existing commands.
Currently I just split the window horizontally. Do we want this split behavior to be customizable? Maybe with 'switchbuf' or 'splitkeep' or something similar?
I think it is fine for now. This can be done in a followup PR, if people want to futher define it.
:drop has no [!] which means 'stickybuf' would disable that command. Would you like to add [!] for it?
Yes I think that is fine
When 'stickybuf' is applied, several mappings are completely disabled. Such as <C-^>
Maybe these should split a new window, instead? Or something else?
Yes, I think this is fine.
When a user makes a split of a window with 'stickybuf' applied, do they want the copy to also have stickybuf? I assume no, right? After all if they're making a split, they probably want to navigate away from the current buffer that they're looking at.
That is tricky. By default new windows inherit the options from the old window (except for e.g. filetype). I would think we do not want this option to be inherited for new windows.
A continuation from 7. When creating a new tab based on the current window, such as :tabedit, should 'stickybuf' be copied to the new tab?
Yeah, probably not.
What should I do about the ex_make family of commands such as :make, :lmake, :grep, :grepadd, etc? Their current logic is "If :foo, do X and jump to the first error". If :foo!, don't jump to the first error. So should I error on :grepadd but allow :grepadd! or would you rather I
make a split window on :grepadd and not split
allow :grepadd!?
Or something completely different?
I would think whatever happens when you have a single buffer open which has been modified and the hidden option is not set when you issue :make that should also for stickybuffer windows. (I would guess, it errors out).
If a command like :buffer targets the same buffer with 'stickybuf' applied,
should Vim...
Pass silently?
If yes then the "Fail Or Allow With [!] (forceit)" Section will need
to double their unittests for the pass case.
Or error with E922 anyway
Note: This is currently what this PR is doing
That is tricky again. Isn't this currently a no-op? So I guess pass silently would be prefered.
What should happen if a user calls :next with an args list like [first] first second third and the window is 'stickybuf'? Should it successfully do the action and change the args list to first [first] second third since the 0th an 1st indices of the arg list are the same buffer? Or should this call error?
I guess it should allow it, as long as it doesn't move away from the buffer currently displayed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dkearns
What should this window attribute be called? I called it 'stickybuf' when I first created the branch because originally the option was a "local to buffer" option but 'stickybuf' is "local to window" now. Should we go with @bfrg suggestion and use 'winfixbuf'?
I thought about it. stickybuf sounds a bit sloppy, and I thought about persistentbuf or similar, but not sure if that is any better. 🤷♀️
I think @bfrg's name is good - there is already winfixheight and winfixwidth.
Maybe I'm showing my age but stickybuf made me think of the Rolling Stones' album cover.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
I think @bfrg's name is good - there is already winfixheight and winfixwidth.
Okay, then let's go with this please
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Maybe I'm showing my age but stickybuf made me think of the Rolling Stones' album cover.
I had the same thought, haha! Just about any name is good for me. Ideally a cool name with impact but practical, consistent names are probably the more responsible thing. It'll probably be at least a week before I've gone through all of the notes. I appreciate everyone's quick review and patience.
By the way, what is the PR etiquette for "Resolve conversation"? I always ran with
- If the reviewee does the reviewer's suggestion exactly, the reviewee resolves the conversation
- If the reviewee wants to do something different than the suggestion, the reviewee/reviewer talk the details and then the reviewer resolves
Some existing conversations here can probably close. I'd like to do the closing if that's fine
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/quickfix.c:
> @@ -3261,8 +3261,37 @@ qf_jump_edit_buffer(
prev_winid == curwin->w_id ? curwin : NULL);
}
else
+ {
+ if (!forceit && curwin->w_p_stb)
+ {
+ if (qi->qfl_type == QFLT_LOCATION)
+ {
+ // Location lists cannot split or reassign their window
+ // so 'stickybuf' windows must fail
+ semsg("%s", e_stickybuf_cannot_go_to_buffer_forceit);
+ return QF_ABORT;
+ }
+
+ if (win_valid(prevwin))
+ {
remove curly braces please
In src/quickfix.c:
> @@ -4982,6 +5011,13 @@ qf_jump_first(qf_info_T *qi, int_u save_qfid, int forceit)
if (qf_restore_list(qi, save_qfid) == FAIL)
return;
+
+ if (!forceit && curwin->w_p_stb)
+ {
remove curly braces please
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/quickfix.c:
> @@ -3261,8 +3261,37 @@ qf_jump_edit_buffer(
prev_winid == curwin->w_id ? curwin : NULL);
}
else
+ {
+ if (!forceit && curwin->w_p_stb)
+ {
+ if (qi->qfl_type == QFLT_LOCATION)
+ {
+ // Location lists cannot split or reassign their window
+ // so 'stickybuf' windows must fail
+ semsg("%s", e_stickybuf_cannot_go_to_buffer_forceit);
+ return QF_ABORT;
+ }
+
+ if (win_valid(prevwin))
+ {
Agreed, it's removed already!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
> @@ -4982,6 +5011,13 @@ qf_jump_first(qf_info_T *qi, int_u save_qfid, int forceit)
if (qf_restore_list(qi, save_qfid) == FAIL)
return;
+
+ if (!forceit && curwin->w_p_stb)
+ {
Removed!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
I had to take a step back from this PR to do some personal things but I plan to resume it tomorrow. I hope to have an update, with all existing comments that have been made so far applied
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
I've done most feedback except for a couple and will push it when I can. In the meantime I'd like to get some clarification on something.
What should I do about the ex_make family of commands such as :make, :lmake, :grep, :grepadd, etc? Their current logic is "If :foo, do X and jump to the first error".
I would think whatever happens when you have a single buffer open which has been modified and the hidden option is not set when you issue :make that should also for stickybuffer windows. (I would guess, it errors out).
Allow Command But Don't Swap
Mostly fine. Do we get an error message if those commands do not swap? Or perhaps just a warning message instead, so that one knows, why the command did not jump to an error message from the quickfix window.
The first note says to do what :make does (which as you correctly guessed errors out). Which in effect means 'Let's move all/most of the "Allow Command But Don't Swap" commands to instead be "Fail Or Allow With [!] (forceit)".
The second note, to add a warning, in effect means 'Let's keep "Allow Command But Don't Swap" but add a warning'
We could make individual exceptions for certain commands but most of the "Allow Command But Don't Swap" commands all go through the same one or two functions. So a change to one command there would basically mean changing them most/all of them. Should "Allow Command But Don't Swap" be a warning or should we move them to "Fail Or Allow With [!] (forceit)"?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
The first note says to do what :make does (which as you correctly guessed errors out). Which in effect means 'Let's move all/most of the "Allow Command But Don't Swap" commands to instead be "Fail Or Allow With [!] (forceit)".
Okay, so let's do Fail Or Allow With [!] (forceit) this then.
No hurries, just ping me when you are ready for the next review round :)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
7to...@gmail.com
Also I think this changes behavior of switchbuf=uselast. Is there any explanation added on this?
Colin Kennedy
Does :cc still honor the switchbuf=vsplit setting after this change?
It’s nested in the PR replies currently but I think I asked whether we need this now in this PR and the consensus was to handle it as a horizontal split. If people want customizations, that can come in another revision. It was a thread with me and @chrisbra if I recall correctly.
I think this changes behavior of switchbuf=uselast
The logic for 'switchbuf' is untouched in this PR. However windows that have 'stickybuf' (soon to be renamed to 'winfixbuf') applied, those windows are excluded as candidates so set switchbuf=uselast won’t accidentally switch to them.
Tom M
Does :cc still honor the switchbuf=vsplit setting after this change?
It’s nested in the PR replies currently but I think I asked whether we need this now in this PR and the consensus was to handle it as a horizontal split. If people want customizations, that can come in another revision. It was a thread with me and @chrisbra if I recall correctly.
I think this changes behavior of switchbuf=uselast
The logic for 'switchbuf' is untouched in this PR. However windows that have 'stickybuf' (soon to be renamed to 'winfixbuf') applied, those windows are excluded as candidates so set switchbuf=uselast won’t accidentally switch to them.
Colin Kennedy
Yes, we should add that to the documentation for 'switchbuf'. We can always remove the documentation later if/when 'switchbuf' is incorporated into the split behavior. By the way though, I think only uselast is affected. I don’t use
vsplit
personally but I think vsplit should still work just the same as it has, I could be wrong.
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to a topic in the Google Groups "vim_dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/vim_dev/wGoAd6n832w/unsubscribe.
To unsubscribe from this group and all its topics, send an email to vim_dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/CAD_dSXjh2CKoO8%3DRGbkD0tjbQ%3DLsxTBpk%2B%2BE4egO%3DNi_eg67QA%40mail.gmail.com.
--
Lead Pipeline Developer
Website : colinkennedy.github.io
LinkedIn : https://www.linkedin.com/in/colinvfx
GitHub : github.com/ColinKennedy
Colin Kennedy
@ColinKennedy pushed 1 commit.
- 8d0e8d9 Added 'stickybuf' window-local option
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- 11af9c6 Renamed 'stickybuf' to 'winfixbuf'
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- c00fe74 Renamed 'stickybuf' to 'winfixbuf'
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
I think I've addressed everything and the PR is ready for another round of review. Unittests failed for me because of test_terminal2.vim but it doesn't always happen and I think it's unrelated to this PR.
Change Summary
'stickybuf'was renamed to'winfixbuf'- E1514 has been removed. Now there is only E1513
- The "Allow Command But Don't Swap" is removed and its commands are now "Fail Or Allow With [!] (forceit)"
- W23 has been added for "Allow Command But Don't Swap"
- [!] was added to
:dropto force-allow even if'winfixbuf'is enabled 'switchbuf'now has a disclaimer about'winfixbuf'- The placeholder
W1001warning has been removed because "Allow Command But Don't Swap" doesn't exist - Most tag-related unittests were accidentally broken and are now fixed
s:execute_try_catchwas replaced withassert_fails:next,:previous, etc now passes if it would change to the same buffer- Added more unittests
- Known, missing tests were added. e.g.
:normal g\<RightMouse> - New tests (e.g.
:mksession, short'wfb'option, etc)
- Known, missing tests were added. e.g.
Topics / PR Replies
Click to expand:vimgrep vs :grep
So I would not like to change the meaning of ! and just make it fail the command, for sticky buffers unless the ! was given (in which case it should switch).
I've changed both the behavior of :vimgrep :grep to fail. Both [!] are left unchanged. I wasn't sure if you meant to change both of just :grep, please let me know if that's the case and I'll edit it.
With 'winfixbuf' enabled...
- `:vimgrep - Fail
- `:vimgrep! - Unchanged from the original
- `:grep - Fail
- `:grep! - Unchanged from the original
Missing, Known Unittests
No Unittests (Do You Want Them?)
if possible, yes please
All are added now
Click to expand:caddexpr:laddexpr:lexpr:cfdo:normal [\<C-D>- No unittests because a unittest for:normal ]\<C-D>exists:pop:lfdo- This seemed to be the same as :ldo so I figured those unittests cover it:normal [\<C-I>- No unittests because a unittest for:normal ]\<C-I>exists:normal \<C-RightMouse>:normal g\<RightMouse>v_CTRL-]v_g_CTRL-]
'sticybuf''s New Name - 'winfixbuf'
I think @bfrg's name is good - there is already winfixheight and winfixwidth.
Okay, then let's go with this please
Done!
:drop! Support
:drop has no [!] which means 'winfixbuf' would disable that command. Would you like to add [!] for it?
Yes I think that is fine
Done!
Window Split Behavior
When a user makes a split of a window with 'winfixbuf' applied, do they want the copy to also have winfixbuf?
That is tricky. By default new windows inherit the options from the old window (except for e.g. filetype). I would think we do not want this option to be inherited for new windows.
Alright, that's the current behavior so I'll stick with that unless we have a reason later to change it.
Calling :buffer on the same file while 'winfixbuf'
:buffer on the same file
That is tricky again. Isn't this currently a no-op? So I guess pass silently would be prefered.
Silently-passing is now done.
Calling args list commands on the same file while 'winfixbuf'
What should happen if a user calls :next with an args list like [first] first second third and the window is 'winfixbuf'?
That is tricky again. Isn't this currently a no-op? So I guess pass silently would be prefered.
This is now done. I've also added a test for ":snext", just in case, to make sure split windows always are allowed.
Open Concerns
Calling :browse from unittests
I was still unable to get this working locally but it does work in Neovim and also on Vim's GitHub action tests. So I'm guessing it's an issue with my host machine or how I chose to compile Vim. It's not a big deal because as mentioned I'm keeping parity in Neovim and can test there but it would be nice to get to the bottom of it if anyone knows the likely cause. I'm happy to split off a separate chat for that so this PR stays on focus.
make distclean
./configure --with-features=huge --enable-gui=gnome2 --with-x
make
(cd ./src/testdir && make test_winfixbuf)
configure: creating cache auto/config.cache
checking whether make sets $(MAKE)... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether the compiler supports GNU C... yes
checking whether gcc accepts -g... yes
checking for gcc option to enable C11 features... none needed
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for fgrep... /usr/bin/grep -F
checking for gawk... gawk
checking for strip... strip
checking for sys/wait.h that is POSIX.1 compatible... yes
checking for unsigned long long int... yes
checking for long long int... yes
checking if the compiler supports trailing commas... yes
checking if the compiler supports C++ comments... yes
checking --enable-fail-if-missing argument... no
checking for clang version... N/A
checking for buggy tools... - sh is 'GNU bash, version 4.2.46(2)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.' - probably OK.
checking uname... Linux
checking uname -r... 3.10.0-1160.11.1.el7.x86_64
checking uname -m... x86_64
checking for Haiku... no
checking for QNX... no
checking for Darwin (Mac OS X)... no
checking for stdio.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for strings.h... yes
checking for sys/stat.h... yes
checking for sys/types.h... yes
checking for unistd.h... yes
checking for sys/time.h... yes
checking for sys/select.h... yes
checking for sys/socket.h... yes
checking for AvailabilityMacros.h... no
checking for dispatch/dispatch.h... no
checking --with-local-dir argument... Defaulting to /usr/local
checking --with-vim-name argument... Defaulting to vim
checking --with-ex-name argument... Defaulting to ex
checking --with-view-name argument... Defaulting to view
checking --with-global-runtime argument... no
checking --with-modified-by argument... no
checking if character set is EBCDIC... no
checking --disable-smack argument... no
checking for linux/xattr.h... yes
checking for sys/xattr.h... yes
checking for XATTR_NAME_SMACKEXEC in linux/xattr.h... no
checking --disable-selinux argument... no
checking for is_selinux_enabled in -lselinux... yes
checking for selinux/selinux.h... yes
checking --enable-xattr argument... yes
checking for sys/xattr.h... (cached) yes
checking --with-features argument... huge
checking --with-compiledby argument... no
checking --disable-xsmp argument... no
checking --disable-xsmp-interact argument... no
checking diff feature... enabled
checking --enable-luainterp argument... no
checking --enable-mzschemeinterp argument... no
checking --enable-perlinterp argument... no
checking --enable-pythoninterp argument... no
checking --enable-python3interp argument... no
checking --enable-tclinterp argument... no
checking --enable-rubyinterp argument... no
checking --enable-cscope argument... no
checking --disable-netbeans argument... no
checking --disable-channel argument... no
checking for socket in -lsocket... no
checking whether compiling with IPv6 networking is possible... yes
checking for inet_ntop... yes
checking --enable-terminal argument... defaulting to yes
checking --enable-autoservername argument... no
checking --enable-multibyte argument... yes
checking --disable-rightleft argument... no
checking --disable-arabic argument... no
checking --enable-xim argument... defaulting to auto
checking --enable-fontset argument... no
checking for xmkmf... no
checking for X... libraries , headers
checking for gethostbyname... yes
checking for connect... yes
checking for remove... yes
checking for shmat... yes
checking for IceConnectionNumber in -lICE... yes
checking if X11 header files can be found... yes
checking for _XdmcpAuthDoIt in -lXdmcp... no
checking for IceOpenConnection in -lICE... yes
checking for XpmCreatePixmapFromData in -lXpm... no
checking if X11 header files implicitly declare return values... no
checking size of wchar_t... 4
checking --enable-gui argument... GNOME 2.x GUI support
checking for pkg-config... /usr/bin/pkg-config
checking --disable-gtktest argument... gtk test enabled
checking for GTK - version >= 2.2.0... yes; found version 2.24.31
checking for libgnomeui-2.0... not found
checking version of Gdk-Pixbuf... OK.
checking for glib-compile-resources... /usr/bin/glib-compile-resources
checking glib-compile-resources... usable.
checking --disable-icon-cache-update argument... not set
checking for gtk-update-icon-cache... /usr/bin/gtk-update-icon-cache
checking --disable-desktop-database-update argument... not set
checking for update-desktop-database... /usr/bin/update-desktop-database
checking for X11/SM/SMlib.h... yes
checking for X11/xpm.h... no
checking for X11/Sunkeysym.h... yes
checking for XIMText in X11/Xlib.h... yes
X GUI selected; xim has been enabled
checking for /proc link to executable... /proc/self/exe
checking for CYGWIN or MSYS environment... no
checking whether toupper is broken... no
checking whether __DATE__ and __TIME__ work... yes
checking whether __attribute__((unused)) is allowed... yes
checking for elf.h... yes
checking for main in -lelf... yes
checking for dirent.h that defines DIR... yes
checking for library containing opendir... none required
checking for stdint.h... (cached) yes
checking for stdlib.h... (cached) yes
checking for string.h... (cached) yes
checking for sys/select.h... (cached) yes
checking for sys/utsname.h... yes
checking for termcap.h... yes
checking for fcntl.h... yes
checking for sgtty.h... yes
checking for sys/ioctl.h... yes
checking for sys/time.h... (cached) yes
checking for sys/types.h... (cached) yes
checking for termio.h... yes
checking for iconv.h... yes
checking for inttypes.h... (cached) yes
checking for langinfo.h... yes
checking for math.h... yes
checking for unistd.h... (cached) yes
checking for stropts.h... no
checking for errno.h... yes
checking for sys/resource.h... yes
checking for sys/systeminfo.h... no
checking for locale.h... yes
checking for sys/stream.h... no
checking for termios.h... yes
checking for libc.h... no
checking for sys/statfs.h... yes
checking for poll.h... yes
checking for sys/poll.h... yes
checking for pwd.h... yes
checking for utime.h... yes
checking for sys/param.h... yes
checking for sys/ptms.h... no
checking for libintl.h... yes
checking for libgen.h... yes
checking for util/debug.h... no
checking for util/msg18n.h... no
checking for frame.h... no
checking for sys/acl.h... no
checking for sys/access.h... no
checking for sys/sysinfo.h... yes
checking for wchar.h... yes
checking for wctype.h... yes
checking for sys/ptem.h... no
checking for sys/sysctl.h... yes
checking for pthread_np.h... no
checking for strings.h... (cached) yes
checking if strings.h can be included after string.h... yes
checking whether gcc needs -traditional... no
checking for an ANSI C-conforming const... yes
checking for working volatile... yes
checking for mode_t... yes
checking for off_t... yes
checking for pid_t... yes
checking for size_t... yes
checking for uid_t in sys/types.h... yes
checking for uint32_t... yes
checking for ino_t... yes
checking for dev_t... yes
checking whether byte ordering is bigendian... no
checking for inline... inline
checking for rlim_t... yes
checking for stack_t... yes
checking whether stack_t has an ss_base field... no
checking --with-tlib argument... empty: automatic terminal library selection
checking for tgetent in -ltinfo... yes
checking whether we talk terminfo... yes
checking what tgetent() returns for an unknown terminal... zero
checking whether termcap.h contains ospeed... yes
checking whether termcap.h contains UP, BC and PC... yes
checking whether tputs() uses outfuntype... no
checking whether del_curterm() can be used... yes
checking whether sys/select.h and sys/time.h may both be included... yes
checking for /dev/ptc... no
checking for SVR4 ptys... yes
checking for ptyranges... don't know
checking for struct sigcontext... yes
checking getcwd implementation is broken... no
checking for fchdir... yes
checking for fchown... yes
checking for fchmod... yes
checking for fsync... yes
checking for getcwd... yes
checking for getpseudotty... no
checking for getpwent... yes
checking for getpwnam... yes
checking for getpwuid... yes
checking for getrlimit... yes
checking for gettimeofday... yes
checking for localtime_r... yes
checking for lstat... yes
checking for memset... yes
checking for mkdtemp... yes
checking for nanosleep... yes
checking for opendir... yes
checking for putenv... yes
checking for qsort... yes
checking for readlink... yes
checking for select... yes
checking for setenv... yes
checking for getpgid... yes
checking for setpgid... yes
checking for setsid... yes
checking for sigaltstack... yes
checking for sigstack... yes
checking for sigset... yes
checking for sigsetjmp... no
checking for sigaction... yes
checking for sigprocmask... yes
checking for sigvec... yes
checking for strcasecmp... yes
checking for strcoll... yes
checking for strerror... yes
checking for strftime... yes
checking for stricmp... no
checking for strncasecmp... yes
checking for strnicmp... no
checking for strpbrk... yes
checking for strptime... yes
checking for strtol... yes
checking for tgetent... yes
checking for towlower... yes
checking for towupper... yes
checking for iswupper... yes
checking for tzset... yes
checking for usleep... yes
checking for utime... yes
checking for utimes... yes
checking for mblen... yes
checking for ftruncate... yes
checking for unsetenv... yes
checking for posix_openpt... yes
checking for clock_gettime... yes
checking types of arguments for select... int,fd_set *,struct timeval *
checking for _LARGEFILE_SOURCE value needed for large files... no
checking for special C compiler options needed for large files... no
checking for _FILE_OFFSET_BITS value needed for large files... no
checking --enable-canberra argument... Defaulting to yes
checking for libcanberra... no; try installing libcanberra-dev
checking --enable-libsodium argument... Defaulting to yes
checking for libsodium... no; try installing libsodium-dev
checking for st_blksize... yes
checking for timer_create without -lrt... no
checking for timer_create with -lrt... yes
checking whether stat() ignores a trailing slash... no
checking for nanoseconds field of struct stat... st_mtim.tv_nsec
checking for iconv_open()... yes
checking for nl_langinfo(CODESET)... yes
checking for strtod in -lm... yes
checking for isinf()... yes
checking for isnan()... yes
checking --disable-acl argument... no
checking for acl_get_file in -lposix1e... no
checking for acl_get_file in -lacl... no
checking for POSIX ACL support... no
checking for acl_get in -lsec... no
checking for Solaris ACL support... no
checking for AIX ACL support... no
checking for pango_shape_full... yes
checking --enable-gpm argument... yes
checking for gpm... no
checking --disable-sysmouse argument... no
checking for sysmouse... no
checking for FD_CLOEXEC... yes
checking for rename... yes
checking for dirfd... yes
checking for flock... yes
checking for sysctl... not usable
checking for sysinfo... yes
checking for sysinfo.mem_unit... yes
checking for sysinfo.uptime... yes
checking for sysconf... yes
checking for _SC_SIGSTKSZ via sysconf()... not usable
checking size of int... 4
checking size of long... 8
checking size of time_t... 8
checking size of off_t... 8
checking uint32_t is 32 bits... ok
checking whether memmove handles overlaps... yes
checking whether X_LOCALE needed... no
checking whether Xutf8SetWMProperties() can be used... yes
checking for _xpg4_setrunelocale in -lxpg4... no
checking how to create tags... ctags -I INIT+,INIT2+,INIT3+,INIT4+,INIT5+ --fields=+S
checking how to run man with a section nr... man
checking --disable-nls argument... no
checking for msgfmt... msgfmt
checking for NLS... gettext() works
checking for bind_textdomain_codeset... yes
checking for _nl_msg_cat_cntr... yes
checking if msgfmt supports --desktop... yes
checking for dlfcn.h... yes
checking for dlopen()... no
checking for dlopen() in -ldl... yes
checking for dlsym()... yes
checking for setjmp.h... yes
checking for GCC 3 or later... yes
checking whether we need -D_FORTIFY_SOURCE=1... yes
checking whether we need to force -D_FILE_OFFSET_BITS=64... no
checking linker --as-needed support... yes
configure: updating cache auto/config.cache
configure: creating auto/config.status
config.status: creating auto/config.mk
config.status: creating auto/config.h
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/arglist.c:
> @@ -697,13 +698,18 @@ do_argfile(exarg_T *eap, int argn)
return;
}
+ if (!is_split_cmd
+ && (&ARGLIST[argn])->ae_fnum != curbuf->b_fnum
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
can you add a test for this? The following should test it:
argadd foobar
next
:set winfixbuf
:rewind "should abort
In src/arglist.c:
> @@ -830,6 +836,9 @@ ex_argedit(exarg_T *eap)
// Whether curbuf will be reused, curbuf->b_ffname will be set.
int curbuf_is_reusable = curbuf_reusable();
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
same here, can you please add a test, using :argedit when the buffer is win-fixed?
In src/buffer.c:
> @@ -1370,6 +1370,13 @@ do_buffer_ext(
if ((flags & DOBUF_NOPOPUP) && bt_popup(buf) && !bt_terminal(buf))
return OK;
#endif
+ if (
+ action == DOBUF_GOTO
+ && buf != curbuf
+ && !check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) ? TRUE : FALSE))
+ // disallow navigating to another buffer when 'winfixbuf' is applied
+ return FAIL;
same here; :b to try to jump to a different buffer.
In src/ex_cmds.c:
> @@ -2428,6 +2428,9 @@ getfile(
int retval;
char_u *free_me = NULL;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return GETFILE_ERROR;
is it possible to get a test here as well?
In src/ex_cmds.c:
> @@ -5445,6 +5448,9 @@ ex_drop(exarg_T *eap)
buf_T *buf;
tabpage_T *tp;
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
same here using :drop command
In src/ex_cmds2.c:
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
How about a test here please?
In src/ex_cmds2.c:
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
not sure if a test here is possible
In src/ex_cmds2.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
can we have a test here?
In src/ex_docmd.c:
> @@ -7245,6 +7248,14 @@ ex_open(exarg_T *eap)
static void
ex_edit(exarg_T *eap)
{
+ // Exclude commands which keep the window's current buffer
+ if (
+ eap->cmdidx != CMD_badd
+ && eap->cmdidx != CMD_balt
+ // All other commands must obey 'winfixbuf' / ! rules
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
Can we have a test here? should be just :e when winfixbuf is set
In src/ex_docmd.c:
> @@ -9173,6 +9184,9 @@ ex_stag(exarg_T *eap)
static void
ex_tag(exarg_T *eap)
{
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
same here please
In src/normal.c:
> @@ -3323,6 +3323,12 @@ nv_ctrlo(cmdarg_T *cap)
static void
nv_hat(cmdarg_T *cap)
{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
and here please
In src/normal.c:
> @@ -4074,6 +4080,12 @@ nv_gotofile(cmdarg_T *cap)
return;
#endif
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
and here please
In src/quickfix.c:
> @@ -3261,8 +3261,35 @@ qf_jump_edit_buffer(
prev_winid == curwin->w_id ? curwin : NULL);
}
else
+ {
+ if (!forceit && curwin->w_p_wfb)
+ {
+ if (qi->qfl_type == QFLT_LOCATION)
+ {
+ // Location lists cannot split or reassign their window
+ // so 'winfixbuf' windows must fail
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return QF_ABORT;
and here please
In src/quickfix.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return QF_ABORT;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
same here please
In src/quickfix.c:
> @@ -4991,6 +5018,11 @@ qf_jump_first(qf_info_T *qi, int_u save_qfid, int forceit)
if (qf_restore_list(qi, save_qfid) == FAIL)
return;
+
+ if (!check_can_set_curbuf_forceit(forceit))
+ return;
and here
In src/quickfix.c:
> @@ -6514,6 +6549,9 @@ ex_vimgrep(exarg_T *eap)
if (vgr_process_args(eap, &args) == FAIL)
goto theend;
+ if (eap->cmdidx == CMD_vimgrep && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
and here please
In src/tag.c:
> @@ -289,6 +289,9 @@ do_tag(
static char_u **matches = NULL;
static int flags;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return FALSE;
and here please
In src/tag.c:
> @@ -3705,6 +3708,9 @@ jumpto_tag(
size_t len;
char_u *lbuf;
+ if (postponed_split == 0 && !check_can_set_curbuf_forceit(forceit))
+ return FAIL;
and here please
In src/window.c:
> @@ -157,6 +157,37 @@ log_frame_layout(frame_T *frame)
}
#endif
+/*
+ * Check if the current window is allowed to move to a different buffer.
+ * If the window has 'winfixbuf', this function will return FALSE.
+ */
+int check_can_set_curbuf_disabled(void)
+{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return FALSE;
and here please
In src/window.c:
> + }
+
+ return TRUE;
+}
+
+/*
+ * Check if the current window is allowed to move to a different buffer.
+ * If the window has 'winfixbuf', then forceit must be TRUE or this function
+ * will return FALSE.
+ */
+int check_can_set_curbuf_forceit(int forceit)
+{
+ if (!forceit && curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return FALSE;
and here please
In src/window.c:
> @@ -572,6 +603,9 @@ do_window(
// FALLTHROUGH
case ']':
case Ctrl_RSB:
+ if (!check_can_set_curbuf_disabled())
+ break;
and here please
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
There are couple of checks for 'winfixbuf' that are not tested. Can you please try to address those if feasible?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/arglist.c:
> @@ -830,6 +836,9 @@ ex_argedit(exarg_T *eap)
// Whether curbuf will be reused, curbuf->b_ffname will be set.
int curbuf_is_reusable = curbuf_reusable();
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
We could probably remove this check actually. Since the check in do_argfile catches it in any case. Would you like me to do consolidate?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/buffer.c:
> @@ -1370,6 +1370,13 @@ do_buffer_ext(
if ((flags & DOBUF_NOPOPUP) && bt_popup(buf) && !bt_terminal(buf))
return OK;
#endif
+ if (
+ action == DOBUF_GOTO
+ && buf != curbuf
+ && !check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) ? TRUE : FALSE))
+ // disallow navigating to another buffer when 'winfixbuf' is applied
+ return FAIL;
I'm not sure how coverage is computed but several tests rely on this code already and should be working. There's Test_buffer already but then there's also
- Test_bNext
- Test_bfirst
- Test_blast
- Test_bmodified
- Test_bnext
- Test_bprevious
- Test_brewind
- Test_windo
Each of them cross through this code
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/arglist.c:
> @@ -697,13 +698,18 @@ do_argfile(exarg_T *eap, int argn)
return;
}
+ if (!is_split_cmd
+ && (&ARGLIST[argn])->ae_fnum != curbuf->b_fnum
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
I think Test_rewind already does this, no? s:make_args_list() creates an argslist, calls set winfixbuf, then I call next! to force it to the next element of the args list, then call rewind and it fails with E1513 but allows rewind!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Maybe I did something wrong (this is my first time contributing to Vim beyond runtime files) but in general my process was
- Write failing unittests in Neovim
- Debug + add breakpoints to figure out the best place to put a E1513 error message
- Tests now pass
- Move test to Vim (this PR/branch)
There could be something lost in translation but in general if there's a code block, there should already be a test that uses it. But I'll double check and reply to each of your comments just in case so we know which test is meant for what
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds.c:
> @@ -2428,6 +2428,9 @@ getfile(
int retval;
char_u *free_me = NULL;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return GETFILE_ERROR;
Test_djump()
Test_ijump()
Test_marks_mappings_fail()
Test_normal_ctrl_o_fail()
Test_normal_square_bracket_left_ctrl_d()
Test_normal_square_bracket_left_ctrl_i()
Test_normal_square_bracket_right_ctrl_d()
Test_normal_square_bracket_right_ctrl_i()
Use this currently
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
I believe currently these tests use this side of the "if" block
Test_argdo_choose_available_window
Test_bufdo_choose_available_window
Test_tabdo_choose_available_window
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_docmd.c:
> @@ -7245,6 +7248,14 @@ ex_open(exarg_T *eap)
static void
ex_edit(exarg_T *eap)
{
+ // Exclude commands which keep the window's current buffer
+ if (
+ eap->cmdidx != CMD_badd
+ && eap->cmdidx != CMD_balt
+ // All other commands must obey 'winfixbuf' / ! rules
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
There's
Test_edit()
Test_enew()
Test_ex()
Test_short_option()
Test_view()
Test_visual()
If those work for you
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
> @@ -9173,6 +9184,9 @@ ex_stag(exarg_T *eap)
static void
ex_tag(exarg_T *eap)
{
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
We can probably remove this one because we have the same check in do_tag already. Would you like to remove this so we consolidate?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/normal.c:
> @@ -3323,6 +3323,12 @@ nv_ctrlo(cmdarg_T *cap)
static void
nv_hat(cmdarg_T *cap)
{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
In retrospect, we can probably remove this because of the check in getfile and consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
> @@ -4074,6 +4080,12 @@ nv_gotofile(cmdarg_T *cap)
return;
#endif
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
These tests currently use this code (e.g. if you remove this block, the following tests immediately fail)
Test_normal_gF()
Test_normal_gf()
Test_normal_square_bracket_left_f()
Test_normal_square_bracket_right_f()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/quickfix.c:
> @@ -3261,8 +3261,35 @@ qf_jump_edit_buffer(
prev_winid == curwin->w_id ? curwin : NULL);
}
else
+ {
+ if (!forceit && curwin->w_p_wfb)
+ {
+ if (qi->qfl_type == QFLT_LOCATION)
+ {
+ // Location lists cannot split or reassign their window
+ // so 'winfixbuf' windows must fail
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return QF_ABORT;
This block is used in these tests
Test_lNext()
Test_lNfile()
Test_ll()
Test_llast()
Test_lnext()
Test_lnfile()
Test_lpfile()
Test_lprevious()
Test_lrewind()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Still in the middle of replying to all notes but I'm a bit surprised since some blocks have multiple tests covering it but code coverage is saying that no test uses it. I'd been using (cd src/testdir && make test_winfixbuf) to run and verify tests this whole time. Is that not the right way to do it? If that's valid then I'm inclined to think that many of the code coverage reports are false negatives...
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
> @@ -4991,6 +5018,11 @@ qf_jump_first(qf_info_T *qi, int_u save_qfid, int forceit)
if (qf_restore_list(qi, save_qfid) == FAIL)
return;
+
+ if (!check_can_set_curbuf_forceit(forceit))
+ return;
Should be used already in
Test_cbuffer()
Test_cexpr()
Test_cfile()
Test_grep()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
> @@ -6514,6 +6549,9 @@ ex_vimgrep(exarg_T *eap)
if (vgr_process_args(eap, &args) == FAIL)
goto theend;
+ if (eap->cmdidx == CMD_vimgrep && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
We might be able to remove this one. Need to double-check
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> + }
+
+ return TRUE;
+}
+
+/*
+ * Check if the current window is allowed to move to a different buffer.
+ * If the window has 'winfixbuf', then forceit must be TRUE or this function
+ * will return FALSE.
+ */
+int check_can_set_curbuf_forceit(int forceit)
+{
+ if (!forceit && curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return FALSE;
A lot of tests should be using this block of code already
All testsTest_Next
Test_argedit
Test_argglobal
Test_arglocal
Test_args
Test_bNext
Test_bfirst
Test_blast
Test_bmodified
Test_bnext
Test_bprevious
Test_brewind
Test_buffer
Test_cbuffer
Test_cexpr
Test_cfile
Test_djump
Test_drop
Test_edit
Test_enew
Test_ex
Test_find
Test_first
Test_grep
Test_ijump
Test_last
Test_ltag
Test_marks_mappings_fail
Test_next
Test_next_same_buffer
Test_normal_ctrl_o_fail
Test_normal_ctrl_rightmouse
Test_normal_ctrl_square_bracket_right
Test_normal_ctrl_t
Test_normal_ctrl_w_ctrl_square_bracket_right
Test_normal_ctrl_w_g_ctrl_square_bracket_right
Test_normal_g_ctrl_square_bracket_right
Test_normal_g_rightmouse
Test_normal_g_square_bracket_right
Test_normal_gt
Test_normal_square_bracket_left_ctrl_d
Test_normal_square_bracket_left_ctrl_i
Test_normal_square_bracket_right_ctrl_d
Test_normal_square_bracket_right_ctrl_i
Test_normal_v_ctrl_square_bracket_right
Test_normal_v_g_ctrl_square_bracket_right
Test_pop
Test_previous
Test_rewind
Test_short_option
Test_tNext
Test_tag
Test_tfirst
Test_tjump
Test_tlast
Test_tnext
Test_tprevious
Test_view
Test_vimgrep
Test_vimgrepadd
Test_visual
Test_wNext
Test_windo
Test_wnext
Test_wprevious
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds.c:
> @@ -5445,6 +5448,9 @@ ex_drop(exarg_T *eap)
buf_T *buf;
tabpage_T *tp;
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
Possibly can remove since do_argfile has the same check. Unless we want to exit early, probably better to consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
There should be these tests already
Test_ldo
Test_lfdo
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 1 commit.
- 87a28a1 Removed unneeded code
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/tag.c:
> @@ -289,6 +289,9 @@ do_tag(
static char_u **matches = NULL;
static int flags;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return FALSE;
This note and #13903 (comment) are interrelated. If the other block and this block are removed then the following tests will fail
Test_normal_ctrl_w_g_ctrl_square_bracket_right
Test_normal_g_square_bracket_right
Test_normal_v_ctrl_square_bracket_right
Test_normal_v_g_ctrl_square_bracket_right
Test_tNext
Test_tprevious
However if we add this block back in then the above tests pass again. Which further makes me think that we can remove #13903 (comment) and keep this block. That said, this block is used in all of the above tests. Not sure why the automated coverage is saying otherwise.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -572,6 +603,9 @@ do_window(
// FALLTHROUGH
case ']':
case Ctrl_RSB:
+ if (!check_can_set_curbuf_disabled())
+ break;
Odd. I thought Test_normal_ctrl_square_bracket_right should cross this code but that test passes even without this block. Might be a difference between Neovim and Vim. I'll follow up on this
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -572,6 +603,9 @@ do_window(
// FALLTHROUGH
case ']':
case Ctrl_RSB:
+ if (!check_can_set_curbuf_disabled())
+ break;
Neovim behaves the same as this branch. Removed unneeded code in both locations.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/ex_cmds2.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
I could be wrong but I believe these tests already use this section
Test_argdo_make_new_window
Test_bufdo_make_new_window
Test_ldo
Test_lfdo
Test_tabdo_make_new_window
Without the split, those tests fail. Though checking again, Test_tabdo_make_new_window is borked and needs fixing.
If you'd like to be absolutely certain that the split is happening as we expect, I could do what I do in make_new_window and count the number of window splits before and after each test, if you'd like
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/quickfix.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return QF_ABORT;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
Should already be used as a part of Test_cnext_make_new_window. We call s:get_windows_count(), then call cnext and confirm that a new window (let l:expected = l:windows + 1) was created. And then confirm that cnext! does not create a new window because we reuse the previously created split to subsequent cnext calls
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/tag.c:
> @@ -3705,6 +3708,9 @@ jumpto_tag(
size_t len;
char_u *lbuf;
+ if (postponed_split == 0 && !check_can_set_curbuf_forceit(forceit))
+ return FAIL;
We may be already to remove this block since we now have a check inside of getfile but I think I would need add a test to ensure splits are still happening for tag commands. We do this already in certain other places like the cnext and xdo unittests but I don't believe I added any "did it successfully split" unittests for tag-related commands. Please let me know if you'd like those
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy pushed 3 commits.
- 781a792 Removed unused winfixbuf check
- 68a1e58 Added check_can_set_curbuf_disabled to missing parts of the code
- f87e050 Removed curly braces
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
I've addressed every point that I could but still unsure about the role of coverage or if somehow what I've written isn't right. @chrisbra please let me know. Thank you!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
@ColinKennedy commented on this pull request.
In src/window.c:
> @@ -157,6 +157,37 @@ log_frame_layout(frame_T *frame)
}
#endif
+/*
+ * Check if the current window is allowed to move to a different buffer.
+ * If the window has 'winfixbuf', this function will return FALSE.
+ */
+int check_can_set_curbuf_disabled(void)
+{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return FALSE;
This is done
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
okay, thanks no worries. In that case it is probably fine. Not sure what problem codecov has. I'll check it once more time later today and will probably merge it then. Thanks for all your patience and effort you put into this.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_docmd.c:
> @@ -9173,6 +9184,9 @@ ex_stag(exarg_T *eap)
static void
ex_tag(exarg_T *eap)
{
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
yes please, consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/quickfix.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return QF_ABORT;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
okay thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -4991,6 +5018,11 @@ qf_jump_first(qf_info_T *qi, int_u save_qfid, int forceit)
if (qf_restore_list(qi, save_qfid) == FAIL)
return;
+
+ if (!check_can_set_curbuf_forceit(forceit))
+ return;
okay
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -6514,6 +6549,9 @@ ex_vimgrep(exarg_T *eap)
if (vgr_process_args(eap, &args) == FAIL)
goto theend;
+ if (eap->cmdidx == CMD_vimgrep && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
okay thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/normal.c:
> @@ -4074,6 +4080,12 @@ nv_gotofile(cmdarg_T *cap)
return;
#endif
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
okay thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -3323,6 +3323,12 @@ nv_ctrlo(cmdarg_T *cap)
static void
nv_hat(cmdarg_T *cap)
{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
yes, please consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_docmd.c:
> @@ -7245,6 +7248,14 @@ ex_open(exarg_T *eap)
static void
ex_edit(exarg_T *eap)
{
+ // Exclude commands which keep the window's current buffer
+ if (
+ eap->cmdidx != CMD_badd
+ && eap->cmdidx != CMD_balt
+ // All other commands must obey 'winfixbuf' / ! rules
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
yes thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_cmds2.c:
> + semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
+ else
+ {
+ // Split the window, which will be 'nowinfixbuf', and set curwin to that
+ exarg_T new_eap;
+ CLEAR_FIELD(new_eap);
+ new_eap.cmdidx = CMD_split;
+ new_eap.cmd = (char_u *)"split";
+ new_eap.arg = (char_u *)"";
+ ex_splitview(&new_eap);
If you could just fix the Test_tabdo_make_new_window then?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
+ }
+
+ if (win_valid(prevwin))
+ // Change the current window to another because 'winfixbuf' is enabled
+ curwin = prevwin;
okay thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_cmds2.c:
> @@ -457,6 +457,31 @@ ex_listdo(exarg_T *eap)
tabpage_T *tp;
buf_T *buf = curbuf;
int next_fnum = 0;
+
+ if (curwin->w_p_wfb)
+ {
+ if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit)
+ {
+ // Disallow :ldo if 'winfixbuf' is applied
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return;
thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/ex_cmds.c:
> @@ -5445,6 +5448,9 @@ ex_drop(exarg_T *eap)
buf_T *buf;
tabpage_T *tp;
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
yes please consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -2428,6 +2428,9 @@ getfile(
int retval;
char_u *free_me = NULL;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return GETFILE_ERROR;
thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/buffer.c:
> @@ -1370,6 +1370,13 @@ do_buffer_ext(
if ((flags & DOBUF_NOPOPUP) && bt_popup(buf) && !bt_terminal(buf))
return OK;
#endif
+ if (
+ action == DOBUF_GOTO
+ && buf != curbuf
+ && !check_can_set_curbuf_forceit((flags & DOBUF_FORCEIT) ? TRUE : FALSE))
+ // disallow navigating to another buffer when 'winfixbuf' is applied
+ return FAIL;
thanks, understood
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/arglist.c:
> @@ -830,6 +836,9 @@ ex_argedit(exarg_T *eap)
// Whether curbuf will be reused, curbuf->b_ffname will be set.
int curbuf_is_reusable = curbuf_reusable();
+ if (!check_can_set_curbuf_forceit(eap->forceit))
+ return;
yes please consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -697,13 +698,18 @@ do_argfile(exarg_T *eap, int argn)
return;
}
+ if (!is_split_cmd
+ && (&ARGLIST[argn])->ae_fnum != curbuf->b_fnum
+ && !check_can_set_curbuf_forceit(eap->forceit))
+ return;
okay fine then.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/tag.c:
> @@ -289,6 +289,9 @@ do_tag(
static char_u **matches = NULL;
static int flags;
+ if (!check_can_set_curbuf_forceit(forceit))
+ return FALSE;
okay, then please do so and consolidate
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -3705,6 +3708,9 @@ jumpto_tag(
size_t len;
char_u *lbuf;
+ if (postponed_split == 0 && !check_can_set_curbuf_forceit(forceit))
+ return FAIL;
okay, if you could please add that additional test here then?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
In src/window.c:
> @@ -157,6 +157,37 @@ log_frame_layout(frame_T *frame)
}
#endif
+/*
+ * Check if the current window is allowed to move to a different buffer.
+ * If the window has 'winfixbuf', this function will return FALSE.
+ */
+int check_can_set_curbuf_disabled(void)
+{
+ if (curwin->w_p_wfb)
+ {
+ semsg("%s", e_winfixbuf_cannot_go_to_buffer);
+ return FALSE;
okay thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
> @@ -572,6 +603,9 @@ do_window(
// FALLTHROUGH
case ']':
case Ctrl_RSB:
+ if (!check_can_set_curbuf_disabled())
+ break;
okay thanks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Ben Jackson
Sorry I didn't read all the comments, but how does this affect programmatic changes to buffer such as py3 vim.current.buffer = other_buffer ?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Please just consolidate the checks and I'll check it once more time later today and will probably merge it then. Thanks for all your patience and effort you put into this.
It's my pleasure, the last thing I wanted to reiterate just in case you missed it was this earlier point (#13903 (comment))
Several commands such as :normal gf are set to be ...
"allowed to run if the result would stay in the same buffer but error if the buffer would change".
I'm wondering if instead we should change the behavior to be ...
"allowed to run if the result would stay in the same buffer or [select another window or do a split window]" like how we have it for quickfix commands such as :cnext.
So that way a user can press gf while in normal mode of a terminal buffer with 'winfixbuf' enabled and Vim will open the file in a nearby window or create a new one. What do you all think?
Also this has been pretty fun and painless, it wasn't always easy but then things worth doing are rarely 100% easy :)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
Sorry I didn't read all the comments, but how does this affect programmatic changes to buffer such as
py3 vim.current.buffer = other_buffer?
Truthfully I completely forgot about Vim's other language bindings. In Python's case I should add a unittest to make sure it raises an exception. Vim also has Lua but I haven't looked into that. This other issue (#11181) suggests that Vim doesn't handle loft errors into Lua in the same way that I'd be used to with Neovim's Lua API. But I'll give that a shot to see what can be done there.
@puremourning If you have a reference of any Python API that shows the different ways to change buffers apart from py3 vim.current.buffer = other_buffer, I can add unittests for these cases
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Ben Jackson
my concern is that this is sort of a 'breaking' change for scripts. Sometimes plugins just have to switch the current buffer temporarily because of vims curbuf being used internally etc. This change means that mind a user set this option for a given window then a script might start producing errors/tracebacks.
You can see :help python for the python bjndings
There are also lua, TCL and ruby at least. I don't know them.
I take it that internal things which temporarily switch curbuf aren't going to be problematic either?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
my concern is that this is sort of a 'breaking' change for scripts.
'winfixbuf' is an opt in feature so it wouldn't break backwards compatibility with anything, if that's your concern.
Sometimes plugins just have to switch the current buffer temporarily because of vims curbuf being used internally etc.
Yes which is why most commands allow [!] to force-change. And if that's not possible, plugin authors looking to take advantage of 'winfixbuf' could always temporarily set 'nowinfixbuf' and set it back once they're done.
This change means that mind a user set this option for a given window then a script might start producing errors/tracebacks.
I think this is a fair point but the same can be said for 'nobuflisted', 'readonly', 'nomodifiable', set bufhidden=delete, or any other option that restricts a buffer. If a plugin author adds 'nomodifiable' to a buffer but changes a buffer's lines with vim.current.buffer[:] = ["foo"] and Vim errors, is the fault with 'nomodifiable' or is it a bug in the plugin? Since the default is to allow buffer modification and one has to opt in to that option for it to error, I'd argue that it's always a bug in the plugin. The same logic applies to 'winfixbuf'.
You can see :help python for the python bindings. Example any window object can have its buffer changed with assigning.
There are also lua, TCL and ruby at least. I don't know them.
Thank you for reminding me, I'll see if I can add coverage for those!
I take it that internal things which temporarily switch curbuf aren't going to be problematic either?
I haven't encountered any issues with this while implementing the tests. The added E1513 error that 'winfixbuf' produces is only emitted at the last possible point, ideally just before a command would complete that changes a buffer. And where possible, "intra-moves" such as marks that jump to a different point in the same file are allowed. It's only when the final buffer result would change that there should be any apparent error. That said if you see any gaps in the unittesting, please let me know and I'm happy to add it.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Ben Jackson
I mean an existing (currently working) script or plugin suddenly starts throwing errors because a timer callback or channel data handler needs to temporarily switch buffers and the user happens to have set this new option in their buffer. I bring this up because my plugins have to do this.
When this happens inevitably they raise a bug against my plugin and I have to spend a bunch of time trying to understand what broke. But my plugin worked before and now doesn't.
And now I have to put some hacky 'if this then that' wrapper around every handler based on vim version and/or whatever othervim ends up implementing. This is a lot of work created by this feature IMO, which is arguably not justified by the linked feature request.
The difference between this being added and 'nomodifiable' is that older option predates timers and channels and many other things and for sure is older than the minimum supported vim version of, let's say, 'most' plugins affected by these sorts of changes.
I don't even think that doing all work in 'SafeState' and 'SafeStateAgain' auto commands would allow a plugin to safely temporarily switch buffers now.
I know that sounds quite negative but I do think we should consider the impact of this feature. I have a personal opinion that the underlying reason for the change is essentially spurious (oil/vinegar etc.) but besides that I think the change carries meaningful risk.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Colin Kennedy
and the user happens to have set this new option in their buffer. I bring this up because my plugins have to do this.
This could still occur today with any of the existing options that I previously mentioned. Any plugin that modifies a buffer where a user sets 'nomodifiable' could incorrectly assume fault in a tool and make a GitHub issue. IMO whether a Vim option predates a plugin existing is besides the point. The point to me is "what are Vim's capabilities and what is Vim not (currently) capable of doing?"
I understand that as a plugin author you are cautious about this feature and that's your choice. However I want to point out the original issue (neovim/neovim#12517) was nearly 4 years old and within 15 minutes of my update in December, it already had 8 GitHub reactions. And again on a couple of other progress updates. The original post has over 30, which in my experience is an unusually high amount of interest.
Vim currently doesn't have the ability to express "purposeful" windows. "This window is meant to show this specific thing". 'winfixbuf' effectively provides that capability. I don't use or oil/vinegar but perhaps 'winfixbuf' would help those. That is for a plugin author to individually decide. Our push for winfixbuf is to improve Vim's overall capabilities and thereby improving the core Vim experience.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.