Conversation
src/curses.rs
Outdated
|
|
||
| prompt_start: if support_osc133 { Some(CString::new("\x1b]133;A\x1b\x5c").unwrap()) } else { None }, | ||
| prompt_end: if support_osc133 { Some(CString::new("\x1b]133;B\x1b\x5c").unwrap()) } else { None }, | ||
| command_start: if support_osc133 { Some(CString::new("\x1b]133;C\x1b\x5c").unwrap()) } else { None }, |
There was a problem hiding this comment.
no need to open a new pull request, you can update the old one, or at least cross-reference #10351 for future readers for the benefit of future readers
Looks good to me overall.
I think we don't even need to define prompt_start fields, can simply hardcode them at the callsite.
Also make sure to remove unused code.
There was a problem hiding this comment.
if the output is not a terminal, what will happen if it receives those escseq?
| zelf.write_mbs_if_some(&term.and_then(|term| term.prompt_start.as_ref())); | ||
| let mut start = 0; | ||
| for line_break in left_prompt_layout.line_breaks { | ||
| zelf.write_str(&left_prompt[start..line_break]); |
There was a problem hiding this comment.
run bind \eg 'echo; git status; git remote -v; echo; commandline -f repaint'
right, we offer bindings like \el and \w so this should definitely be supported
src/screen.rs
Outdated
| // Output the left prompt if it has changed. | ||
| if left_prompt != zelf.actual_left_prompt { | ||
| zelf.r#move(0, 0); | ||
| zelf.write_mbs_if_some(&term.and_then(|term| term.prompt_start.as_ref())); |
There was a problem hiding this comment.
How is this feature used by terminals?
If it's just about not overwriting output, we should do it for right prompts too.
If it's meant to allow jumping between prompts, then the right prompt shouldn't get it.
There was a problem hiding this comment.
as for kitty, it only needs marking of
- start of PS1
- start of PS2 (auto-complete i think)
- start of application output
https://sw.kovidgoyal.net/kitty/shell-integration/#notes-for-shell-developers
| /// capabilities we care about in the process. | ||
| fn new(db: terminfo::Database) -> Self { | ||
| // todo: how to do feature detection of OSC 133? | ||
| let support_osc133 = false; |
There was a problem hiding this comment.
We can probably enable it unconditionally.
I'm not aware of a relevant terminal that doesn't have basic xterm compatibility.
We can add a knob to specify xterm compatibility that or turn off OSC 133 specifically but I think that can be done later.
We do have a bug where we fail to turn off bracketed paste and focus reporting for commands launched from bindings,
like bind \eg sh for example.
But we can definitely fix that, and it's not relevant here.
src/curses.rs
Outdated
| pub prompt_end: Option<CString>, // "\x1b133;B\x07" | ||
| pub command_start: Option<CString>, // "\x1b133;C\x07" | ||
| // pub command_end_before_status: Option<CString>, // "\x1b133;D;<exitcode>;\x07" | ||
| // pub command_end_after_status: Option<CString>, |
There was a problem hiding this comment.
Might as well add command start/end too here.
Emit it next to where we emit fish_preexec/fish_postexec, passing eval_res.status.status_value().
If you choose to do that, make sure it plays well with fish's default prompt which already renders a failing status in bold red.
|
Why is pub fn tputs(&mut self, str: &CStr) {
self.begin_buffering();
let _ = self.write(str.to_bytes());
self.end_buffering();
}I will not use CString in this patch. |
|
I am not sure where to print the other escape sequences. Anyone can take over this PR now. I have marked what needs to be added with |
it's now included upstream fish-shell/fish-shell#10352
Description
Support marking prompts with OSC 133
Supercedes #10351.
TODOs: