fix(grid): fix a problem that the default bottom line of the scrolling region (DECSTBM) is placed outside the terminal screen

[akinomyoga]

Summary

When Zellij receives the control sequence \e[1;r (DECSTBM with the parameter string 1;) from a terminal application, it uses "the terminal height" (self.height) for the second parameter (which is supposed to be the bottom-line 1-based index of the scrolling region).

zellij/zellij-server/src/panes/grid.rs

Line 1570 in c72f3a7

let bottom_line_index = bottom_line_index.unwrap_or(self.height);

However, the internal index of the bottom line should be "(the terminal height) - 1" (self.height - 1) because the internal index is 0-based.

Problem

• Steps to reproduce: See the following reduced case. You can run the command inside Zellij.

$ printf '\e[1;r'; seq 999; printf '\e[r'

• What I expect: Suppose the current terminal height is N. I expect that the last N-lines in the output of the seq command remain in the terminal screen.

• Behavior in main: However, in the current main branch of Zellij, the first N-lines in the seq output remain in the terminal screen.

This doesn't happen when the terminal height is directly specified to the parameter string of DECSTBM as

$ printf '\e[1;%dr' "$LINES"; seq 999; printf '\e[r'

because the index base is properly converted on the following line:

zellij/zellij-server/src/panes/grid.rs

Line 2733 in c72f3a7

.map(|bottom| bottom.saturating_sub(1));

cf In another location that sets the default scrolling region, the subtraction is properly performed:

zellij/zellij-server/src/panes/grid.rs

Lines 1579 to 1581 in c72f3a7

	pub fn set_scroll_region_to_viewport_size(&mut self) {
	self.scroll_region = Some((0, self.height.saturating_sub(1)));
	}

Note: The problem was found while investigating akinomyoga/ble.sh#450, (though this PR doesn't still fix the original problem).

Remarks

There should still be problems when the terminal window height is changed or when the terminal application explicitly specifies the bottom line to be outside the terminal window. However, solving them would require changing the internal representation of the scroll region, so I avoid performing the large change within this PR. I think they should be separately fixed if needed.

[imsnif]

Thanks for this - good catch!

I agree the internal representation of the scrolling region needs to change (specifically it shouldn't be an Option), and I also agree it's out of scope here.

[akinomyoga]

Thanks!