Track OSC 133 Prompts and Clear on Resize - #8961
#8961Track OSC 133 Prompts and Clear on Resize#8961nixpulvis wants to merge 1 commit into
Conversation
Wire up OSC 133 ;A and ;C through to Term. ;A records the prompt's cursor position as an active mark; ;C clears it. Before resizing the grid, erase from the active mark to the end of the viewport and damage the affected lines. Shells (e.g. fish) draw right-prompts with absolute cursor positioning, producing a sparse line of [left prompt][gap][right prompt]. shrink_columns reflows that detached tail onto a stray continuation row on horizontal shrink. The grid has no structural way to distinguish that gap from typed interior whitespace (see the shrink_reflow_empty_cell_inside_line test which deliberately reflows the latter), so any pure-grid fix would change that contract. Erasing the OSC 133 marked region instead and letting the shell repaint sidesteps the ambiguity and matches what kitty and ghostty already do. Fish, for example has a fish_handle_reflow variable to control this behavior. ;B and ;D are wired as default no-ops; Alacritty doesn't need them for resize. Existing shrink_columns reflow behavior is unchanged for content without an active prompt mark.
|
I assume when opening this PR you just deleted the PR template that asked about no LLM being involved in the creation of this PR? Or was that not prefilled as part of the PR template at all? |
chrisduerr
left a comment
There was a problem hiding this comment.
This approach says, if you emit OSC 133, you agree to redraw your prompt completely. Simple as that.
Says who? This is just winging it for the result, isn't it?
From what I can tell, the entirety of this PR seems to be based around introducing some new proprietary escape sequence for the sole purpose of avoiding artifacts when resizing the window, which notably is not even the intended purpose of that escape sequence? Am I understanding this right?
It should be noted that bash only redraws the last line, so users who implement OSC 133 for themselves in bash and have multi-line prompts, will see their top line of the prompt cleared on resize, but not redrawn. I believe this is a valid tradeoff though, since most bash users are A) unlikely to be implementing OSC 133 I'd imagine, and B) also unlikely to be using multi-line prompts. I don't believe there's any other way for bash to show a regression.
What? So bash, the most common shell of them all is broken and somehow that's fine? Why are bash users any less likely to use this than OSC 133? Just because it's broken on bash? If we break it on all shells, then we just don't have to implement it, right?
Also why was this not brought up in #5850 before implementation?
Was almost certainly my fault when I copy and pasted the description from my editor, sorry about that. No LLM written code here.
I wouldn't say it's completely broken, it just breaks multiline prompts when resizing. That said, it's really because I didn't feel you would be open to the hoops other terminals jump through to support this. For example ghostty injects custom OSC 133 escapes with a custom parameter of last=true on bash to only clear the last row to avoid this. Kitty does something I actually like a bit, but is much more work to save the state of the prompt, then clear it, reflow, then restore it. That could be what we agree on, since it's a complete solution I believe, but more machinery than this.
I guess I wanted a PoC to actually make sure I could solve my issue, and that issue seems more focused on the general case of tracking prompts, commands, and outputs. This doesn't expose any of that to users in any way. The VTE patch would absolutely make that possible, and I'd be in favor of implementing an action like "jump to next/last command" etc. Sorry for kinda jumping into this, I was just feeling crafty recently. |
If this escape is not suitable to solve the problem you're trying to solve and instead requires hacks anyway, then isn't this a pretty clear indicator that this escape is not useful? I have no interest in supporting an escape just because it allows implementing a weird hack to work around a perceived issue.
That's fair, though I'm really not a fan of this escape, which I assume you could have guessed. I don't like forcing a decision by opening up a PR, so while I know you've been annoyed by the underlying issue for a long time, I have no interest in rushing the implementation of a proprietary escape we'll never be able to remove again. As long as that's okay with you, I have no problem with just opening up a PR. |
|
@perfbot It's been a while, but this might alter scroll region performance. |
|
Yea, I was by no means trying to force or rush anything. If anything I knew this wasn't going to be merged as-is, so I wanted to spark the discussion with something a bit more meaningful, and at least for my own understanding of the issue. I wouldn't say the escape isn't meaningful here, it gives us exactly the region where the prompt lives, which is core to what could be a complete solution which handles bash properly as well. The issue at the moment is the simple "clear the viewport after the prompt" assumes shells redraw the whole prompt, which is true for zsh and fish for example, but not bash. |
Shells that know Alacritty is resizing them can already work around this issue by taking this into account anyway though, right? Like even without any support from Alacritty. |
| *cell = bg.into(); | ||
| } | ||
| if mark.line.0 + 1 < screen_lines { | ||
| self.grid.reset_region((mark.line + 1)..); |
There was a problem hiding this comment.
I was going to ask if it made sense to create a function for resetting a region from row,col to row,col like this need or if this is fine as is.
There was a problem hiding this comment.
It's probably fine, we do the exact same thing for ansi::ClearMode::Below:
if (cursor.line.0 as usize) < screen_lines - 1 {
self.grid.reset_region((cursor.line + 1)..);
}There was a problem hiding this comment.
(though to be fair, this means there's at least 2 places where this is needed, so it could also be taken as an argument for introducing a new function)
|
|
|
Hard to tell from just a single run, but that does look like a performance regression in region scrolling. |
|
@perfbot Let's give it another shot with latest arch and rust. |
|
|
|
Now I wish I had run it twice before updating. |
|
Remind me, where are the SHAs coming from for these benches? Also what are the version numbers? |
SHAs are master (purple) and PR (green). By version I'd assume you mean the |
We should keep conversations about this specific implementation in this patch. I'm definitely not putting this into flags, it should go into |
|
Yea, good call about the I'm looking into where we actually need to use this extra information now so it's not dropped on reflows and maybe other places. |
|
OK, I've been playing around and this is quickly ballooning into a larger changeset than I anticipated. Some of it is expected tracking through linewraps and widechar edge cases... but then there's some things I'm still trying to understand about how Fish or other OSC 133 providers send these codes. One issue is that if we store the markers on the cells directly, then when those cells are cleared so are the marks. At first I thought this was desired, but one case specifically is making me rethink this. When the I think I'll try to read over how other terminals implement this now. I know ghostty does some extra stuff to support right prompts with an additional argument to the I'd love some basic support for this, so I'm not really advocating for going above and beyond the initial set of OSC 133 features. I'm not even 100% sure why the additional arguments are needed for the prompt escape. I would think just sending prompt start after command start would say the same thing... but I could be missing something. |
Wire
OSC 133 ;Aand;Cthrough toTerm.;Arecords the prompt's cursor position as an active mark,;Cclears it. Before resizing the grid, erase from the active mark to the end of the viewport and damage the affected lines. This is not active in the alt-grid.Why
In shells like Fish, we can now enable
fish_handle_reflowand avoid this:Fish disabled reflow for alacritty, which causes right prompts to wrap incorrectly on resize.
Details
Shells (notably fish) draw right-prompts with absolute cursor positioning, producing a sparse line of
[left prompt][gap][right prompt].shrink_columnsreflows that detached tail onto a stray continuation row on horizontal shrink. The grid has no structural way to distinguish that gap from typed interior whitespace (shrink_reflow_empty_cell_inside_linedeliberately reflows the latter), so any pure-grid fix would change that contract.Erasing the grid after a OSC 133 mark instead and letting the shell repaint sidesteps the ambiguity and follows from what kitty (
screen.c'sprevent_current_prompt_from_rewrappingplus its blank-and-repaint on resize) and ghostty (Terminal.resizecheckingcursor.semantic_content != .output) already do to some degree. I looked a bit at various implementations of this and there are major differences across the board, but clearing after the prompt and relying on shells to redrawn onSIGWINCHis a theme. Some terminals wrap the shell to help facilitate the OSC 133 integration, which I don't think is a good idea for Alacritty.This approach says, if you emit OSC 133, you agree to redraw your prompt completely. Simple as that.
Technically, kitty's approach of using the OSC 133 regions to save the prompt and reflow around it without clearing it in a visible draw frame avoids some flickering, but I'm personally not bothered by the flicker whie redrawing, and it's a lot more complexity to do things that way. The goal here is just to allow the end state to be consistent after a resize.
Shell Cooperation
Fish sets
fish_handle_reflow=0when$TERMmatchesalacritty*, predating this work. It can be overridden withset --global fish_handle_reflow 1until fish drops the alacritty exclusion upstream. Withfish_handle_reflow 1onSIGWINCHthe prompt will be redrawn completely and now correctly.Users of bash or zsh don't get OSC 133 built in, but can be enabled by writing the escape sequence themselves.
It should be noted that bash only redraws the last line, so users who implement OSC 133 for themselves in bash and have multi-line prompts, will see their top line of the prompt cleared on resize, but not redrawn. I believe this is a valid tradeoff though, since most bash users are A) unlikely to be implementing OSC 133 I'd imagine, and B) also unlikely to be using multi-line prompts. I don't believe there's any other way for bash to show a regression.
Depends on alacritty/vte#152 for the new
Handlermethods. If/when that's released, this PR can be updated to use the released version, however the best way to do that is.