Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] %val{window_range} returns wrong values during WinCreate #4975

Open
Screwtapello opened this issue Sep 9, 2023 · 2 comments
Open

[BUG] %val{window_range} returns wrong values during WinCreate #4975

Screwtapello opened this issue Sep 9, 2023 · 2 comments
Labels

Comments

@Screwtapello
Copy link
Contributor

Version of Kakoune

v2023.08.05-29-gbe1a61a2

Reproducer

Launch Kakoune, and get it to log %val{window_range} in a WinCreate hook:

kak -n -e "hook global WinCreate .* %{ echo -debug window_range: %val{window_range} }"

Switch to the *debug* buffer:

:buffer *debug*

Outcome

For me, building Kakoune with make, I get something like:

window_range: 32512 22058 2 0

If I build with debug=yes sanitize=undefined, I get:

window_range: 25964 486572031 918865728 0

If I build with debug=yes sanitize=address, I get:

window_range: -1094795586 -1094795586 -1094795586 0

Expectations

The four fields of window_range are "coord_y coord_x height width", so I would expect something like (based on a full-screen terminal):

window_range: 0 0 67 239

Additional information

If I run echo %val{window_range} manually after starting Kakoune, what I get is 0 0 67 0. I'm not sure if it's intentional for the width to be 0, but it happens even in v2023.08.05.

@mawww
Copy link
Owner

mawww commented Sep 10, 2023

I am wondering what the correct behaviour should be here, at the point we run WinCreate, the window has no highlighters, cursors, or even dimensions. At first I thought this was an issue introduced by 9787756 but it seems this never worked. I am tempted to just fix the uninitialized values and end-up with 0 0 0 0 there, but I wonder what the use case for accessing window range on WinCreate might be.

@Screwtapello
Copy link
Contributor Author

Screwtapello commented Sep 11, 2023

I encountered this with my fork of the scrollbar plugin, which uses the "coord_y" and "height" fields of %val{window_range}, alongside %val{buf_line_count}, to decide on the size and position of the scroll-thumb (and where to draw the flag-lines highlighter that displays the scrollbar).

Because I want a scrollbar in every window, I havehook global WinCreate .* %{ scrollbar-enable } in my autoload directory. After the recent "scrolling does not move the cursor" changes, Kakoune would get stuck at startup ("waiting for shell command to finish") while my memory-usage grew until I ran out of memory. It turns out that on that machine, the value of %val{window_range} in WinCreate typically had a "height" in the billions, and the poor shell command was trying to draw a scrollbar that big. I could reproduce the issue as above, so that's what I reported.

However, now I've looked a little closer, I see an additional problem. I had assumed my scrollbar-enable command would call update-scrollbar which would use the bogus window_range values, but it doesn't. Instead, it adds a WinResize hook, and surely by the time we call WinResize we have the new coordinates ready!

It turns out no, that's not how it works. WinResize has the old values in window_range, and passes the new values in hook_param. So the first WinResize event has bogus values in window_range, and thats why my scrollbar plugin was going crazy.

Given the initial values are visible in WinResize, I think it makes sense to initialise them, although I'd suggest "0 0 1 1" would be safer defaults to avoid potential divide-by-zero errors.

For the purposes of my scrollbar plugin, it'd also be nice to have a WinResizePost hook, run after the window_range expansion has been updated, but for the moment it just means I might have to hit <c-l> occasionally to make sure the scrollbar is showing the right thing, or find some way to conditionally merge the updated "window height" from the hook parameter with the existing "coord_y" from window_range.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants