Improve VT102 compliance #904
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,8 +93,10 @@ impl TabStop { | |
| *t = false; | ||
| } | ||
| } | ||
| // If we want to exactly match VT100/xterm behavior, then | ||
| // we cannot honor ClearCharacterTabStopsAtActiveLine. | ||
| TabulationClear::ClearAllCharacterTabStops => { | ||
| // | TabulationClear::ClearCharacterTabStopsAtActiveLine => { | ||
| for t in &mut self.tabs { | ||
| *t = false; | ||
| } | ||
|
|
@@ -290,6 +292,7 @@ pub struct TerminalState { | |
| g0_charset: CharSet, | ||
| g1_charset: CharSet, | ||
| shift_out: bool, | ||
| newline_mode: bool, | ||
|
|
||
| tabs: TabStop, | ||
|
|
||
|
|
@@ -495,6 +498,7 @@ impl TerminalState { | |
| g0_charset: CharSet::Ascii, | ||
| g1_charset: CharSet::DecLineDrawing, | ||
| shift_out: false, | ||
| newline_mode: false, | ||
| current_mouse_button: MouseButton::None, | ||
| tabs: TabStop::new(size.physical_cols, 8), | ||
| title: "wezterm".to_string(), | ||
|
|
@@ -1028,6 +1032,9 @@ impl TerminalState { | |
| buf.push(0x1b as char); | ||
| } | ||
| buf.push(c); | ||
| if self.newline_mode && key == Enter { | ||
| buf.push(0x0a as char); | ||
| } | ||
| } | ||
| buf.as_str() | ||
| } | ||
|
|
@@ -1789,6 +1796,7 @@ impl TerminalState { | |
| self.application_keypad = false; | ||
| self.top_and_bottom_margins = 0..self.screen().physical_rows as i64; | ||
| self.left_and_right_margins = 0..self.screen().physical_cols; | ||
| self.screen.reverse_video_mode = false; | ||
| self.screen.activate_alt_screen(); | ||
| self.screen.saved_cursor().take(); | ||
| self.screen.activate_primary_screen(); | ||
|
|
@@ -1819,6 +1827,12 @@ impl TerminalState { | |
| self.writer.write(ST.as_bytes()).ok(); | ||
| self.writer.flush().ok(); | ||
| } | ||
| Device::RequestTerminalParameters(a) => { | ||
| self.writer | ||
| .write(format!("\x1b[{};1;1;128;128;1;0x", a + 2).as_bytes()) | ||
| .ok(); | ||
| self.writer.flush().ok(); | ||
| } | ||
| Device::StatusReport => { | ||
| self.writer.write(b"\x1b[0n").ok(); | ||
| self.writer.flush().ok(); | ||
|
|
@@ -1949,10 +1963,26 @@ impl TerminalState { | |
| // We always output at our "best" rate | ||
| } | ||
|
|
||
| Mode::SetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::ReverseVideo)) => { | ||
| // Turn on reverse video for all of the lines on the | ||
| // display. | ||
| self.screen.reverse_video_mode = true; | ||
| for y in 0..self.screen.physical_rows as i64 { | ||
|
There was a problem hiding this comment. In rust, array dimension accesses are bounds-checked by default which means that there is some (small) overhead for C style iteration. It's preferable to iterate containers directly so that the bounds check happens just once and the individual steps are essentially equivalent to bumping a pointer with no other overhead: for line in &mut self.screen.lines {
line.set_reverse(true);
}There was a problem hiding this comment. This is For rendering purposes, we could still keep https://github.com/wez/wezterm/blob/main/mux/src/renderable.rs#L72-L93 /// Implements Pane::get_lines for Terminal
pub fn terminal_get_lines(
term: &mut Terminal,
lines: Range<StableRowIndex>,
) -> (StableRowIndex, Vec<Line>) {
let screen = term.screen_mut();
let reverse = term.get_reverse_video(); // added
let phys_range = screen.stable_range(&lines);
(
screen.phys_to_stable_row_index(phys_range.start),
screen
.lines
.iter_mut()
.skip(phys_range.start)
.take(phys_range.end - phys_range.start)
.map(|line| {
let cloned = line.clone();
line.clear_dirty();
cloned.set_reverse(reverse); // added
cloned
})
.collect(),
)
}If something like that makes sense, then There was a problem hiding this comment. A few thoughts:
There was a problem hiding this comment. A small bit of exploring shows that for xterm, konsole, and VTE, reverse video is a property of the screen only, and furthermore when set all lines rendered, including those in scrollback, are reversed. I will remove the line-level attribute and leave it on the There was a problem hiding this comment. OK, reverse is nicer, thank you! However, there is one gap remaining: on lines that do not stretch to the full physical pixels, we do not see the reversed background. Example:
I need to add a bit after render.rs:933, so that the selection and cursor are rendered, and then if there is more physical width remaining fill it with bg_color. Any suggestions? |
||
| let line_idx = self.screen.phys_row(VisibleRowIndex::from(y)); | ||
| let line = self.screen.line_mut(line_idx); | ||
| line.set_reverse(true); | ||
| } | ||
| } | ||
|
|
||
| Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::ReverseVideo)) => { | ||
| // Turn off reverse video for all of the lines on the | ||
| // display. | ||
| self.screen.reverse_video_mode = false; | ||
| for y in 0..self.screen.physical_rows as i64 { | ||
|
There was a problem hiding this comment. for line in &mut self.screen.lines {
line.set_reverse(false);
} |
||
| let line_idx = self.screen.phys_row(VisibleRowIndex::from(y)); | ||
| let line = self.screen.line_mut(line_idx); | ||
| line.set_reverse(false); | ||
| } | ||
| } | ||
|
|
||
| Mode::SetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::Select132Columns)) | ||
|
|
@@ -1977,6 +2007,13 @@ impl TerminalState { | |
| self.insert = false; | ||
| } | ||
|
|
||
| Mode::SetMode(TerminalMode::Code(TerminalModeCode::AutomaticNewline)) => { | ||
|
There was a problem hiding this comment. would you mind adding a unit test for this? |
||
| self.newline_mode = true; | ||
| } | ||
| Mode::ResetMode(TerminalMode::Code(TerminalModeCode::AutomaticNewline)) => { | ||
| self.newline_mode = false; | ||
| } | ||
|
|
||
| Mode::SetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::BracketedPaste)) => { | ||
| self.bracketed_paste = true; | ||
| } | ||
|
|
@@ -2824,6 +2861,7 @@ impl TerminalState { | |
| self.g0_charset = saved.g0_charset; | ||
| self.g1_charset = saved.g1_charset; | ||
| self.shift_out = false; | ||
| self.newline_mode = false; | ||
| } | ||
|
|
||
| fn perform_csi_sgr(&mut self, sgr: Sgr) { | ||
|
|
@@ -3189,6 +3227,9 @@ impl<'a> Performer<'a> { | |
| self.cursor.y = y; | ||
| self.wrap_next = false; | ||
| } | ||
| if self.newline_mode { | ||
| self.cursor.x = 0; | ||
| } | ||
| } | ||
| ControlCode::CarriageReturn => { | ||
| if self.cursor.x >= self.left_and_right_margins.start { | ||
|
|
@@ -3258,6 +3299,15 @@ impl<'a> Performer<'a> { | |
| ControlCode::ShiftOut => { | ||
| self.shift_out = true; | ||
| } | ||
|
|
||
| ControlCode::Enquiry => { | ||
| let response = self.config.enq_answerback(); | ||
| if response.len() > 0 { | ||
| write!(self.writer, "{}", response).ok(); | ||
| self.writer.flush().ok(); | ||
| } | ||
| } | ||
|
|
||
| _ => log::warn!("unhandled ControlCode {:?}", control), | ||
| } | ||
| } | ||
|
|
@@ -3318,6 +3368,23 @@ impl<'a> Performer<'a> { | |
| Esc::Code(EscCode::DecSaveCursorPosition) => self.dec_save_cursor(), | ||
| Esc::Code(EscCode::DecRestoreCursorPosition) => self.dec_restore_cursor(), | ||
|
|
||
| Esc::Code(EscCode::DecDoubleHeightTopHalfLine) => { | ||
| let idx = self.screen.phys_row(self.cursor.y); | ||
| self.screen.line_mut(idx).set_double_height_top(); | ||
|
There was a problem hiding this comment. it would be great to add a some test coverage for these attributes, especially because we're not rendering them :-) I don't think we have any existing tests or conveniences for inspecting |
||
| } | ||
| Esc::Code(EscCode::DecDoubleHeightBottomHalfLine) => { | ||
| let idx = self.screen.phys_row(self.cursor.y); | ||
| self.screen.line_mut(idx).set_double_height_bottom(); | ||
| } | ||
| Esc::Code(EscCode::DecDoubleWidthLine) => { | ||
| let idx = self.screen.phys_row(self.cursor.y); | ||
| self.screen.line_mut(idx).set_double_width(); | ||
| } | ||
| Esc::Code(EscCode::DecSingleWidthLine) => { | ||
| let idx = self.screen.phys_row(self.cursor.y); | ||
| self.screen.line_mut(idx).set_single_width(); | ||
| } | ||
|
|
||
| Esc::Code(EscCode::DecScreenAlignmentDisplay) => { | ||
| // This one is just to make vttest happy; | ||
| // its original purpose was for aligning the CRT. | ||
|
|
@@ -3351,6 +3418,7 @@ impl<'a> Performer<'a> { | |
| self.insert = false; | ||
| self.dec_auto_wrap = true; | ||
| self.reverse_wraparound_mode = false; | ||
| self.screen.reverse_video_mode = false; | ||
| self.dec_origin_mode = false; | ||
| self.use_private_color_registers_for_each_graphic = false; | ||
| self.color_map = default_color_map(); | ||
|
|
@@ -3369,6 +3437,7 @@ impl<'a> Performer<'a> { | |
| self.g0_charset = CharSet::Ascii; | ||
| self.g1_charset = CharSet::DecLineDrawing; | ||
| self.shift_out = false; | ||
| self.newline_mode = false; | ||
| self.tabs = TabStop::new(self.screen().physical_cols, 8); | ||
| self.palette.take(); | ||
| self.top_and_bottom_margins = 0..self.screen().physical_rows as VisibleRowIndex; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sometimes use brace matching in vim to navigate, so it would be good to avoid this unbalanced curly!