Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix positioning of terminal inline assist after clearing the screen (#34465)

Browse source 
Closes #33945. Here's my attempt to describe what's going on in that
issue and what this fix is doing:

We always render the terminal inline assistant starting on the line
after the cursor, with a height of 4 lines. When deploying it, we scroll
the viewport to the bottom of the terminal so that the assistant will be
in view.

When scrolling while the assistant is deployed (including in that case),
we need to make an adjustment that "pushes up" the terminal content by
the height of the assistant, so that we can scroll to see all the normal
content plus the assistant itself. That quantity is `scroll_top`, which
represents _how much height in the current viewport is occupied by the
assistant that would otherwise be occupied by terminal content_. So when
you scroll up and a line of the assistant's height goes out of view,
`scroll_top` decreases by 1, etc.

When we scroll to the bottom after deploying the assistant, we set
`scroll_top` to the result of calling `max_scroll_top`, which computes
it this way:

```
block.height.saturating_sub(viewport_lines.saturating_sub(terminal_lines))
```

Which, being interpreted, is "the height of the assistant, minus any
viewport lines that are not occupied by terminal content", i.e. the
assistant is allowed to eat up vertical space below the last line of
terminal content without increasing `scroll_top`.

The problem comes when we clear the screen---this adds a full screen to
`terminal_lines`, but the cursor is positioned at the top of the
viewport with blank lines below, just like at the beginning of a session
when `terminal_lines == 1`. Those blank lines should be available to the
assistant, but the `scroll_top` calculation doesn't reflect that.

I've tried to fix this by basing the `max_scroll_top` calculation on the
position of the cursor instead of the raw `terminal_lines` value. There
was also a special case for `viewport_lines == terminal_lines` that I
think can now be removed.

Release Notes:

- Fixed the positioning of the terminal inline assistant when it's
deployed after clearing the terminal.
This commit is contained in:
Cole Miller 2025-07-15 15:16:48 -04:00 • committed by GitHub
parent b398935081
commit af0031ae8b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 8 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

27
crates/terminal_view/src/terminal_view.rs
View file

@ -25,11 +25,11 @@ use terminal::{
TaskStatus, Terminal, TerminalBounds, ToggleViMode,
alacritty_terminal::{
index::Point,
term::{TermMode, search::RegexSearch},
term::{TermMode, point_to_viewport, search::RegexSearch},
},
terminal_settings::{self, CursorShape, TerminalBlink, TerminalSettings, WorkingDirectory},
};
use terminal_element::{TerminalElement, is_blank};
use terminal_element::TerminalElement;
use terminal_panel::TerminalPanel;
use terminal_scrollbar::TerminalScrollHandle;
use terminal_slash_command::TerminalSlashCommand;
@ -497,25 +497,14 @@ impl TerminalView {
};
let line_height = terminal.last_content().terminal_bounds.line_height;
let mut terminal_lines = terminal.total_lines();
let viewport_lines = terminal.viewport_lines();
if terminal.total_lines() == terminal.viewport_lines() {
let mut last_line = None;
for cell in terminal.last_content.cells.iter().rev() {
if !is_blank(cell) {
break;
}
let last_line = last_line.get_or_insert(cell.point.line);
if *last_line != cell.point.line {
terminal_lines -= 1;
}
*last_line = cell.point.line;
}
}
let cursor = point_to_viewport(
terminal.last_content.display_offset,
terminal.last_content.cursor.point,
)
.unwrap_or_default();
let max_scroll_top_in_lines =
(block.height as usize).saturating_sub(viewport_lines.saturating_sub(terminal_lines));
(block.height as usize).saturating_sub(viewport_lines.saturating_sub(cursor.line + 1));
max_scroll_top_in_lines as f32 * line_height
}