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

surface: add timer-based scrolling during selection #4422

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

moni-dz
Copy link
Contributor

@moni-dz moni-dz commented Jan 2, 2025

Adds a timer to continuously scroll during selection when outside the viewport, 15ms per line.

Currently the scrolling behavior requires you to jiggle the mouse to continuously scroll upwards/downwards when selecting text.

Before

before.mp4

After

after.mp4

@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch from 0574889 to 164bda4 Compare January 3, 2025 00:01
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on.

I do not like the idea that the renderer thread is controlling the viewport in this way or modifying the terminal at all. The renderer thread should, philosophically, be a read-only consumer of the terminal.

I like that by putting it in a thread with libxev you've made this cross-platform and just work, though.

I wonder if this should perhaps go into the IO thread instead, which spiritually owns the terminal anyways.

@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch from 164bda4 to 7114287 Compare January 3, 2025 01:39
@moni-dz
Copy link
Contributor Author

moni-dz commented Jan 3, 2025

I do not like the idea that the renderer thread is controlling the viewport in this way or modifying the terminal at all. The renderer thread should, philosophically, be a read-only consumer of the terminal.

I wonder if this should perhaps go into the IO thread instead, which spiritually owns the terminal anyways.

I have moved the logic to the Termio thread, that seems to be the right call as sending mailbox messages became cleaner.

@moni-dz moni-dz requested a review from mitchellh January 3, 2025 01:42
@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch from 7114287 to 6c32377 Compare January 3, 2025 01:51
@moni-dz
Copy link
Contributor Author

moni-dz commented Jan 3, 2025

Now the selection region keeps up with the scrolling.

@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch 5 times, most recently from 20ef0aa to dd7a578 Compare January 9, 2025 04:47
@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch 2 times, most recently from ba071f7 to 7ff6765 Compare January 11, 2025 06:12
@moni-dz moni-dz force-pushed the moni/zkxpmupovkpl branch 5 times, most recently from d9e0481 to 8f7da27 Compare January 24, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants