keep track of scrollback buffer size #881
Conversation
There was a problem hiding this comment.
Looks like this turned out to be like a fair bit of work! Happy to see this though.
I have not tried this and only glanced over the code. Left some comments. I trust that you went through the basic smoke tests to make sure this works... only thing I can add (if you didn't try) is to make sure this doesn't slow pane resizing too much when the scrollbuffer is very full.
About alternative grid: I think we can totally ignore that use case (as long as it doesn't break the main one). Ideally scrolling should be completely disabled when the alternative grid is set (both in the UI and functionality).
zellij-server/src/panes/grid.rs
Outdated
| @@ -170,6 +177,7 @@ fn transfer_rows_from_viewport_to_lines_above( | |||
| viewport.insert(0, row); | |||
| } | |||
| } | |||
| transferred_rows_height | |||
There was a problem hiding this comment.
We need to account for the bounded_push above somehow. There's a chance that it drops a line if it overflows the buffer... maybe also return a value from it?
zellij-server/src/panes/grid.rs
Outdated
| @@ -557,6 +575,8 @@ impl Grid { | |||
| pub fn scroll_down_one_line(&mut self) { | |||
| if !self.lines_below.is_empty() && self.viewport.len() == self.height { | |||
| let mut line_to_push_up = self.viewport.remove(0); | |||
| self.scrollback_buffer_lines += | |||
| (line_to_push_up.width() as f64 / self.width as f64).ceil() as usize; | |||
| if line_to_push_up.is_canonical { | |||
| bounded_push(&mut self.lines_above, line_to_push_up); | |||
There was a problem hiding this comment.
We also need to account for bounded_push here, it might drop lines
|
i should have addressed the bounded push cases. |
|
What happens right now with the alternative grid, in this current state of affairs? |
|
well i noticed something like this can happen. where you keep scrolling: this seems to happen regardless of these changes though. |
|
Yeah, this is what I meant. We should ideally not allow scrolling when in alternative grid mode. So if this change leaves things as they are, I think it's fine. |
|
@imsnif I've handled a couple more edge cases, and am seeing numbers that match what we have in main. |
tlinford
changed the title
initial attempt at solving #877
missing handling of when the viewport gets switched to alternative_viewport