[vim/vim] feat: support winid as argument for foldclosed*() (PR #21133)
Mao-Yining
fixes: #21130
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/21133
Commit Summary
- 863fbdf feat: support winid as argument for foldclosed*()
File Changes
(4 files)
- M runtime/doc/builtin.txt (17)
- M src/evalfunc.c (4)
- M src/fold.c (20)
- M src/testdir/test_fold.vim (70)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#21133)Thanks. I think this is reasonable. But please note, that using tv_get_lnum resolves '.' and '$' in the context of the current buffer which may return unexpected results. Have a look at how f_line() handles this.
There are a few other functions, that could be handled that way, e.g. foldlevel(), foldtextresult().
Please also add tests for v9 type checking, similar to what Test_line() does in test_vim9_builtin.vim.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#21133)x
Thanks. All Done.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
h_east
h-east left a comment (vim/vim#21133)Two things go wrong here regardless of the interface question.
In foldclosed_both(), restore_win() runs even when switch_win() was
never reached: with an unknown window id wp is NULL, the && chain stops
before switch_win(), and switchwin is uninitialized. restore_win() also
calls unblock_autocmds() with nothing to match it, and --autocmd_blocked
has no lower bound (autocmd.c:2640), so after one foldclosed(1, 99999) the
next block_autocmds() stops blocking anything.
In f_foldtextresult(), the if (wp == NULL || tp == NULL) return; comes
after entered = TRUE and nothing puts it back, so one call with an unknown
window id leaves foldtextresult() returning an empty string for the rest of
the session.
Both point at the same open question: what should an unknown window id do?
foldclosed() returning -1 and foldlevel() returning 0 are answers a valid
window gives too, so a script cannot tell them apart. An error would be
better, and the documentation should say which it is.
foldclosed_both() hands the window to hasFoldingWin(), while foldlevel()
and foldtextresult() switch to it, which is heavier and blocks autocommands
while it lasts. Is there a reason not to pass the window down in all three?
Moving foldLevel() elsewhere in the file, reindenting the parameter list of
foldclosed_both(), and the blank line in the new else block are unrelated
to the feature and make the diff harder to read.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Mao-Yining
mao-yining left a comment (vim/vim#21133)Two things go wrong here regardless of the interface question.
In
foldclosed_both(),restore_win()runs even whenswitch_win()was never reached: with an unknown window idwpis NULL, the&&chain stops beforeswitch_win(), andswitchwinis uninitialized.restore_win()also callsunblock_autocmds()with nothing to match it, and--autocmd_blockedhas no lower bound (autocmd.c:2640), so after onefoldclosed(1, 99999)the nextblock_autocmds()stops blocking anything.In
f_foldtextresult(), theif (wp == NULL || tp == NULL) return;comes afterentered = TRUEand nothing puts it back, so one call with an unknown window id leavesfoldtextresult()returning an empty string for the rest of the session.Both point at the same open question: what should an unknown window id do?
foldclosed()returning -1 andfoldlevel()returning 0 are answers a valid window gives too, so a script cannot tell them apart. An error would be better, and the documentation should say which it is.
foldclosed_both()hands the window tohasFoldingWin(), whilefoldlevel()andfoldtextresult()switch to it, which is heavier and blocks autocommands while it lasts. Is there a reason not to pass the window down in all three?Moving
foldLevel()elsewhere in the file, reindenting the parameter list offoldclosed_both(), and the blank line in the newelseblock are unrelated to the feature and make the diff harder to read.
Thank you for your reply. The motivation for this issue came from developing redraw_listener — if there is an inadvertent error, it throws an error on every redraw, which is very difficult to deal with. What are your thoughts on this situation?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#21133)thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.