[vim/vim] `WinScrolled` is not emitted during inital phase of `incsearch` (Issue #11628)
Evgeni Chasnovski
Steps to reproduce
- Create a config file (called 'init_winscrolled-incsearch.vim' in this example') with the following content:
let g:n = 0 au WinScrolled * let g:n = g:n + 1 set incsearch
vim -u init_winscrolled-incsearch.vim:help:echo g:nand remember its value (should be 1).- Press
/and typejumped. During that it should scroll window twice: atjuand afterjumpe. - Type
<C-c>to abort search. It should scroll back to initial position. :echo g:nstill shows the same value as before, however, it should be increased by 3.
Expected behaviour
WinScrolled is emitted on every window scroll.
Version of Vim
9.0.954
Environment
OS: EndeavourOS Linux x86_64, 6.0.10-arch2-1
Terminal: st
$TERM: st-256color
Shell: zsh 5.9
Logs and stack traces
No response
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
Evgeni Chasnovski wrote
> ### Steps to reproduce
>
> 1. Create a config file (called 'init_winscrolled-incsearch.vim' in this example') with the following content:
> ```vim
> let g:n = 0
> au WinScrolled * let g:n = g:n + 1
> set incsearch
> 2. `vim -u init_winscrolled-incsearch.vim`
> 3. `:help`
> 4. `:echo g:n` and remember its value (should be 1).
> 5. Press `/` and type `jumped`. During that it should scroll window twice: at `ju` and after `jumpe`.
> 6. Type `<C-c>` to abort search. It should scroll back to initial position.
> 7. `:echo g:n` still shows the same value as before, however, it should be increased by 3.
>
> ### Expected behaviour
>
> `WinScrolled` is emitted on every window scroll.
what WinScrolled is used for.
The basic idea of 'incsearch' is that you can see where the search will
take you. There is no intention to completely update the whole display,
especially skip "decorations", since those may take time and the jumping
around that 'incsearch' causes should be quick.
What would WinScrolled events do while the user is typing the search
pattern? Would the same apply to WinResized?
I checked that when the search is executed then the one WinScrolled
event is triggered if the topline changed. That is actually what I
expect.
--
hundred-and-one symptoms of being an internet addict:
154. You fondle your mouse.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Evgeni Chasnovski
What would WinScrolled events do while the user is typing the search pattern? Would the same apply to WinResized?
My use case is animating any scroll. Right now WinScrolled is triggered only after accepting the search with <CR> (if top line has changed).
The expectation here is that WinScrolled is triggered after any scroll, i.e. either top line, left column, or skip column change in any window (at least within current tab). Same goes for WinResized - triggered after any window resize, i.e. width or height change (not sure if it can be applied in this issue, though).
I checked that when the search is executed then the one WinScrolled event is triggered if the topline changed. That is actually what I expect.
As far as I can tell from original example in this issue, WinScrolled is triggered only and after accepting search, not when topline changed. Whereas scroll can happen multiple times during typing search word and even after cancelling it with <C-c>, while no WinScrolled seem to be triggered.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
> > What would WinScrolled events do while the user is typing the search
> > pattern? Would the same apply to WinResized?
>
> My use case is animating any scroll. Right now `WinScrolled` is
> triggered only after accepting the search with `<CR>` (if top line has
> changed).
not by using autocommand events. I don't see how it can work anyway,
since the event is triggered after the screen is updated. The text
would jump around or flicker.
> The expectation here is that `WinScrolled` is triggered after any
> scroll, i.e. either top line, left column, or skip column change in
> any window (at least within current tab). Same goes for `WinResized` -
> triggered after any window resize, i.e. width or height change (not
> sure if it can be applied in this issue, though).
The change to do otherwise must be properly justified, there must be a
good reason for it. "expecting" isn't a reason by itself.
> >I checked that when the search is executed then the one WinScrolled
> >event is triggered if the topline changed. That is actually what I
> >expect.
>
> As far as I can tell from original example in this issue,
> `WinScrolled` is triggered only and after accepting search, not when
> topline changed. Whereas scroll can happen multiple times during
> typing search word and even after cancelling it with `<C-c>`, while no
> `WinScrolled` seem to be triggered.
reason to change it.
--
hundred-and-one symptoms of being an internet addict:
you want.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Evgeni Chasnovski
I don't really intend to support "animating" scrolling, and certainly not by using autocommand events. I don't see how it can work anyway, since the event is triggered after the screen is updated. The text would jump around or flicker.
Sure, I don't expect you to support it. I was just answering the "What would WinScrolled events do while the user is typing the search pattern?".
At the moment I have pretty good implementation based on WinScrolled event without flickering. It doesn't seem that event is triggered after the screen is updated (at least in Neovim). And I really hope it will stay that way.
As mentioned, this won't happen when jumping around for 'incsearch'. The change to do otherwise must be properly justified, there must be a good reason for it. "expecting" isn't a reason by itself.
Probably, more explicit wording is that it was my understanding of WinScrolled documentation. Particularly, "After any window in the current tab page scrolled the text (horizontally or vertically)". I provided an example when this doesn't happen (a window scrolled but WinScrolled was not triggered): during initial phase of search with incsearch enabled.
Correct. And it will probably stay that way, since I currently see no reason to change it.
Sure, although I disagree, I respect you decisions. If you think this is final, then please feel free to close this.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
bfrg
What would WinScrolled events do while the user is typing the search
pattern? Would the same apply to WinResized?
I could think of a use case. Imagine someone wrote a plugin that provides some external syntax highlighting (LSP, tree-sitter etc.). For performance reasons the syntax highlighting might first be applied to the current view port only. I.e. text-properties are added only to the currently visible lines, and the rest is added later as the window is scrolled. This is certainly faster than adding all at once (but also more complicated), especially when editing a very large file with several thousand lines. If that's the case I would expect the plugin to add the remaining syntax highlighting as the window is scrolled, either through inscearch, or when the window is resized, or any other command that initiates scrolling.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
> > What would WinScrolled events do while the user is typing the search
> > pattern? Would the same apply to WinResized?
>
> I could think of a use case. Imagine someone wrote a plugin that
> provides some external syntax highlighting (LSP, tree-sitter etc.). For
> performance reasons the syntax highlighting might first be applied to
> the current view port only. I.e. text-properties are added only to the
> currently visible lines, and the rest is added later as the window is
> scrolled. This is certainly faster than adding all at once (but also
> more complicated), especially when editing a very large file with
> several thousand lines. If that's the case I would expect the plugin to
> add the remaining syntax highlighting as the window is scrolled, either
> that initiates scrolling.
I would say that is exactly the reason the event should not be triggered
when jumping around for 'incsearch', because that should be really fast.
You don't want to wait a second for updated highlighting with every
character you type.
Slow highlight updating would work better when the moving around rests
for a bit. And that is not just for 'incsearch' but also for other
commands that make the text move. E.g. typing "n" several times, or
using "cnext" to go through a sequence of positions. Therefore it's
better to separate this.
An alternative would be trigger WinScrolled on every little change, and
provide some way for the consumer of the event to decide to postpone the
effect. The problem is that we can't really define a rule when to
postpone. For me when using 'incsearch' I don't really care if the
highlighting is correct, it just want to see where the pattern matches.
But for others it might be different, e.g. when search for a piece of
text that is highlighted in a particular way. This may lead to endless
tuning of config parameters.
--
If you feel lonely, try schizophrenia.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
bfrg
So it's consistency vs performance. Well, I would have said that users who are bothered by a plugin that significantly slows down scrolling should just stop using that plugin (or improve it). IMO, the current behavior is unexpected. WinScrolled is not triggered during inscsearch nor when pressing <C-g> or <C-t>, but on the other hand it is triggered when opening a vsplit and when closing the same split. I don't know if that's a bug but I wouldn't have expected it.
I'm curious what other users think.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Nick Jensen
An alternative would be trigger WinScrolled on every little change, and provide some way for the consumer of the event to decide to postpone the effect.
This sounds pretty sensible to me. What about adding another value to the v:event? e.g.:
{
all: {width: 0, height: 2, leftcol: 0, topline: 1, skipcol: 0},
1003: {width: 0, height: -1, leftcol: 0, topline: 0, skipcol: 0},
1006: {width: 0, height: 1, leftcol: 0, topline: 1, skipcol: 0},
trigger: 'search'
}
Where 'trigger' is one of e.g. 'search', 'resize', 'split' etc.?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
> > An alternative would be trigger WinScrolled on every little change,
> > and provide some way for the consumer of the event to decide to
> > postpone the effect.
>
> This sounds pretty sensible to me. What about adding another value to
> the v:event? e.g.:
>
> ```
> {
> all: {width: 0, height: 2, leftcol: 0, topline: 1, skipcol: 0},
> 1003: {width: 0, height: -1, leftcol: 0, topline: 0, skipcol: 0},
> 1006: {width: 0, height: 1, leftcol: 0, topline: 1, skipcol: 0},
> trigger: 'search'
> }
> ```
>
> Where 'trigger' is one of e.g. 'search', 'resize', 'split' etc.?
It would require much more detailed information, and even then the
scrolling can be caused by more than one thing (command triggering an
autocommand that triggers a plugin that splits a window, that...).
I think that only some clever debouncing mechanism would work. Knowing
what action/command/whatever happened before is going to lead to endless
tuning that will never work properly, and needs re-tuning every time a
new feature is added.
--
Microsoft says that MS-Windows is much better for you than Linux.
That's like the Pope saying that catholicism is much better for
you than protestantism.
/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Nick Jensen
How can any plugin base a decision on that?
Well you're suggesting that highlighting updates shouldn't necessarily happen during incsearch operations, so a plugin can very easily make a decision to not update when v.event.trigger == 'search' (or 'incsearch', or whatever).
It's also possible to let the plugin handle debouncing, which gives all control to the plugin to do as it will - sometimes debouncing may be appropriate, perhaps not always.
I'm approaching this from a plugin-author perspective, not an implementer perspective, it may not be feasible. Really, this was just an idea for how to do what you suggested in your comment to "trigger WinScrolled on every little change, and provide some way for the consumer of the event to decide to postpone the effect", which does sound like the way I'd like to be able to handle it on the plugin side.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dezza
dezza left a comment (vim/vim#11628)As always I appreciate @brammool good questions solely for narrowing the issue and @chrisbra the now maintainer certainly lives up to!
I'm not opposed to the idea of WinScrolled triggering during search, but I think @nickspoons idea of having a source field or trigger on the v:event dict is a good idea.
—
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.