From 53c47143fd3e51fba01c6ad019015e2531ecd797 Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Wed, 30 Mar 2022 08:06:45 -0700 Subject: [PATCH] term: scope the conpty resize quirks better to conpty on windows Previously, we'd just default the resize quirk on for all programs on Windows. That pessimizes the otherwise fine behavior of `wezterm ssh` or mux connections to unixy platforms. This commit moves the quirk out of the config trait and makes it a runtime property of the terminal, and then arranges for it to be set when we know that we set up a conpty for the terminal. refs: #1265 --- mux/src/domain.rs | 2 +- term/src/config.rs | 28 ---------------------------- term/src/screen.rs | 25 ++++++++++++++++++++++++- term/src/terminalstate/mod.rs | 19 ++++++++++++------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/mux/src/domain.rs b/mux/src/domain.rs index dd3a57e09..6f69cc233 100644 --- a/mux/src/domain.rs +++ b/mux/src/domain.rs @@ -284,7 +284,7 @@ impl Domain for LocalDomain { Box::new(writer), ); if self.is_conpty() { - terminal.set_supress_initial_title_change(); + terminal.enable_conpty_quirks(); } let pane: Rc = Rc::new(LocalPane::new( diff --git a/term/src/config.rs b/term/src/config.rs index f6fe5ccb8..d6e451f9e 100644 --- a/term/src/config.rs +++ b/term/src/config.rs @@ -158,34 +158,6 @@ pub trait TerminalConfiguration: std::fmt::Debug { /// defines the initial palette. fn color_palette(&self) -> ColorPalette; - /// Return true if a resize operation should consider rows that have - /// made it to scrollback as being immutable. - /// When immutable, the resize operation will pad out the screen height - /// with additional blank rows and due to implementation details means - /// that the user will need to scroll back the scrollbar post-resize - /// than they would otherwise. - /// - /// When mutable, resizing the window taller won't add extra rows; - /// instead the resize will tend to have "bottom gravity" meaning that - /// making the window taller will reveal more history than in the other - /// mode. - /// - /// mutable is generally speaking a nicer experience. - /// - /// On Windows, the PTY layer doesn't play well with a mutable scrollback, - /// frequently moving the cursor up to high and erasing portions of the - /// screen. - /// - /// This behavior only happens with the windows pty layer; it doesn't - /// manifest when using eg: ssh directly to a remote unix system. - /// - /// Ideally we'd have this return `true` only for the native windows - /// pty layer, but for the sake of simplicity, we make this conditional - /// on being a windows build. - fn resize_preserves_scrollback(&self) -> bool { - cfg!(windows) - } - fn canonicalize_pasted_newlines(&self) -> NewlineCanon { NewlineCanon::default() } diff --git a/term/src/screen.rs b/term/src/screen.rs index c25041dcf..9d24335a1 100644 --- a/term/src/screen.rs +++ b/term/src/screen.rs @@ -166,6 +166,7 @@ impl Screen { physical_cols: usize, cursor: CursorPosition, seqno: SequenceNo, + is_conpty: bool, ) -> CursorPosition { let physical_rows = physical_rows.max(1); let physical_cols = physical_cols.max(1); @@ -227,7 +228,29 @@ impl Screen { let new_cursor_y; - if self.config.resize_preserves_scrollback() { + // true if a resize operation should consider rows that have + // made it to scrollback as being immutable. + // When immutable, the resize operation will pad out the screen height + // with additional blank rows and due to implementation details means + // that the user will need to scroll back the scrollbar post-resize + // than they would otherwise. + // + // When mutable, resizing the window taller won't add extra rows; + // instead the resize will tend to have "bottom gravity" meaning that + // making the window taller will reveal more history than in the other + // mode. + // + // mutable is generally speaking a nicer experience. + // + // On Windows, the PTY layer doesn't play well with a mutable scrollback, + // frequently moving the cursor up to high and erasing portions of the + // screen. + // + // This behavior only happens with the windows pty layer; it doesn't + // manifest when using eg: ssh directly to a remote unix system. + let resize_preserves_scrollback = is_conpty; + + if resize_preserves_scrollback { new_cursor_y = cursor .y .saturating_add(cursor_y as i64) diff --git a/term/src/terminalstate/mod.rs b/term/src/terminalstate/mod.rs index 1d73e278c..1b8bef0d7 100644 --- a/term/src/terminalstate/mod.rs +++ b/term/src/terminalstate/mod.rs @@ -201,13 +201,14 @@ impl ScreenOrAlt { cursor_main: CursorPosition, cursor_alt: CursorPosition, seqno: SequenceNo, + is_conpty: bool, ) -> (CursorPosition, CursorPosition) { - let cursor_main = self - .screen - .resize(physical_rows, physical_cols, cursor_main, seqno); - let cursor_alt = self - .alt_screen - .resize(physical_rows, physical_cols, cursor_alt, seqno); + let cursor_main = + self.screen + .resize(physical_rows, physical_cols, cursor_main, seqno, is_conpty); + let cursor_alt = + self.alt_screen + .resize(physical_rows, physical_cols, cursor_alt, seqno, is_conpty); (cursor_main, cursor_alt) } @@ -368,6 +369,7 @@ pub struct TerminalState { unicode_version: UnicodeVersion, unicode_version_stack: Vec, + enable_conpty_quirks: bool, /// On Windows, the ConPTY layer emits an OSC sequence to /// set the title shortly after it starts up. /// We don't want that, so we use this flag to remember @@ -556,6 +558,7 @@ impl TerminalState { unicode_version, unicode_version_stack: vec![], suppress_initial_title_change: false, + enable_conpty_quirks: false, accumulating_title: None, lost_focus_seqno: seqno, focused: true, @@ -564,7 +567,8 @@ impl TerminalState { } } - pub fn set_supress_initial_title_change(&mut self) { + pub fn enable_conpty_quirks(&mut self) { + self.enable_conpty_quirks = true; self.suppress_initial_title_change = true; } @@ -819,6 +823,7 @@ impl TerminalState { cursor_main, cursor_alt, self.seqno, + self.enable_conpty_quirks, ); self.top_and_bottom_margins = 0..physical_rows as i64; self.left_and_right_margins = 0..physical_cols;