-
-
Notifications
You must be signed in to change notification settings - Fork 681
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
Fix(scroll): Prevent scroll buffer overwrite #655
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool - happy to see you getting into these parts of Zellij as well. :)
A few things:
- I'm a little afraid of the buffer being uncapped. If I leave a pane scrolled up with input running overnight I might build up quite the memory utilization by morning... do you think we can cap it somehow and then if it exceeds that cap we reset the scroll? not the best ux, but I think it's better than now, no? Or do you maybe have a different idea?
- When we play the pending vte events, at least on my machine, there's quite a long period where things are stuck. I assume it's when the events are interpreted. Do you think we might be able to add some renders in the middle of this somehow? So the user sees that something is happening?
dec46e6
to
b732471
Compare
@imsnif I've incorporated your suggestions and updated the PR. For now, I've kept the buffer size to 7000 events per pane and the tab will render after processing every 100 pending events. I can update the numbers as per your (or anyone else's) suggestions. Also, this works at all times- inside scroll mode or outside (when scrolling with the mouse). |
b732471
to
1fdd7d1
Compare
@kunalmohan - I think this isn't working for me, but maybe I'm doing something wrong. What I did:
|
I've addressed the review comments and made the required updates. Things work for me as expected, therefore, merging this now. |
Fix #306
Currently, if a command is producing output (eg
tail
), then scrolling up causes the command to overwrite the scroll buffer.This PR fixes it. Now when a pane is scrolled up, new vte events are buffered and when we exit scroll, the events are played at once.