Use XTVERSION for terminal-specific workarounds

As mentioned in earlier commit
("Query terminal before reading config").

Closes #11812

doc_src/language.rst

@@ -1693,6 +1693,10 @@ Fish also provides additional information through the values of certain environm
 
    the process ID (PID) of the shell.
 
+.. envvar:: fish_terminal
+
+   the name and version of the terminal fish is running inside (for example as reported via :ref:`XTVERSION <term-compat-xtversion>`).
+
 .. envvar:: history
 
    a list containing the last commands that were entered.

doc_src/terminal-compatibility.rst

@@ -224,9 +224,12 @@ Optional Commands
      - Ss
      - Set cursor style (DECSCUSR); Ps is 2, 4 or 6 for block, underscore or line shape.
      - VT520
-   * - ``\e[ Ps q``
+   * - .. _term-compat-xtversion:
+
+       ``\e[ Ps q``
      - n/a
      - Request terminal name and version (XTVERSION).
+       This is only used for temporary workarounds for incompatible terminals.
      - XTerm
    * - ``\e[?25h``
      - cvvis

share/functions/__fish_config_interactive.fish

@@ -145,16 +145,14 @@ end" >$__fish_config_dir/config.fish
     if not set -q fish_handle_reflow
         # VTE reflows the text itself, so us doing it inevitably races against it.
         # Guidance from the VTE developers is to let them repaint.
-        if set -q VTE_VERSION
-            # Same for these terminals
+        # Konsole reflows since version 21.04. Konsole added XTVERSION
+        # in v22.03.80~7.
+        if string match -rq -- "$fish_terminal" '^(?:VTE\b|Konsole |WezTerm )'
+            or begin
+                set -q KONSOLE_VERSION
+                and test "$KONSOLE_VERSION" -ge 210400 2>/dev/null
+            end
             or string match -q -- 'alacritty*' $TERM
-            or test "$TERM_PROGRAM" = WezTerm
-            set -g fish_handle_reflow 0
-        else if set -q KONSOLE_VERSION
-            and test "$KONSOLE_VERSION" -ge 210400 2>/dev/null
-            # Konsole since version 21.04(.00)
-            # Note that this is optional, but since we have no way of detecting it
-            # we go with the default, which is true.
             set -g fish_handle_reflow 0
         else
             set -g fish_handle_reflow 1
@@ -171,6 +169,10 @@ end" >$__fish_config_dir/config.fish
     if not functions --query __fish_update_cwd_osc
         function __fish_update_cwd_osc --on-variable PWD --description 'Notify terminals when $PWD changes'
             set -l host $hostname
+            # if set -l konsole_version (string match -r -- '^Konsole (\d+)\..*' "$fish_terminal")[2]
+            #     # To-do: use a Konsole version where KF6_DEP_VERSION is >= 6.12
+            #     and $konsole_version -lt ???
+            # end
             if set -q KONSOLE_VERSION
                 set host ''
             end

src/bin/fish.rs

@@ -39,6 +39,7 @@
         environment::{env_init, EnvStack, Environment},
         EnvMode, Statuses,
     },
+    env_dispatch::guess_emoji_width,
     eprintf,
     event::{self, Event},
     flog::{self, activate_flog_categories_by_pattern, set_flog_file_fd, FLOG, FLOGF},
@@ -62,6 +63,7 @@
     signal::{signal_clear_cancel, signal_unblock_all},
     threads::{self},
     topic_monitor,
+    tty_handoff::xtversion,
     wchar::prelude::*,
     wutil::waccess,
 };
@@ -553,6 +555,12 @@ enum CommandSource {
             .pending_input
             .borrow_mut()
             .extend(std::mem::take(&mut input_data.queue));
+        parser.vars().set_one(
+            L!("fish_terminal"),
+            EnvMode::GLOBAL,
+            xtversion().unwrap().to_owned(),
+        );
+        guess_emoji_width(parser.vars());
     }
 
     if !opts.no_exec && !opts.no_config {

src/env/var.rs

@@ -249,6 +249,7 @@ pub struct ElectricVar {
     ElectricVar{name: L!("fish_kill_signal"), flags:electric::READONLY | electric::COMPUTED},
     ElectricVar{name: L!("fish_killring"), flags:electric::READONLY | electric::COMPUTED},
     ElectricVar{name: L!("fish_pid"), flags:electric::READONLY},
+    ElectricVar{name: L!("fish_terminal"), flags:electric::READONLY},
     ElectricVar{name: L!("history"), flags:electric::READONLY | electric::COMPUTED},
     ElectricVar{name: L!("hostname"), flags:electric::READONLY},
     ElectricVar{name: L!("pipestatus"), flags:electric::READONLY | electric::COMPUTED},

src/env_dispatch.rs

@@ -12,6 +12,7 @@
 };
 use crate::terminal::use_terminfo;
 use crate::terminal::ColorSupport;
+use crate::tty_handoff::xtversion;
 use crate::wchar::prelude::*;
 use crate::wutil::fish_wcstoi;
 use crate::{function, terminal};
@@ -153,7 +154,7 @@ fn handle_timezone(var_name: &wstr, vars: &EnvStack) {
 }
 
 /// Update the value of [`FISH_EMOJI_WIDTH`](crate::fallback::FISH_EMOJI_WIDTH).
-fn guess_emoji_width(vars: &EnvStack) {
+pub fn guess_emoji_width(vars: &EnvStack) {
     use crate::fallback::FISH_EMOJI_WIDTH;
 
     if let Some(width_str) = vars.get(L!("fish_emoji_width")) {
@@ -168,7 +169,7 @@ fn guess_emoji_width(vars: &EnvStack) {
         return;
     }
 
-    let term = vars
+    let term_program = vars
         .get(L!("TERM_PROGRAM"))
         .map(|v| v.as_string())
         .unwrap_or_else(WString::new);
@@ -189,14 +190,14 @@ fn guess_emoji_width(vars: &EnvStack) {
         })
         .unwrap_or(0.0);
 
-    if term == "Apple_Terminal" && version as i32 >= 400 {
-        // Apple Terminal on High Sierra
-        FISH_EMOJI_WIDTH.store(2, Ordering::Relaxed);
-        FLOG!(term_support, "default emoji width: 2 for", term);
-    } else if term == "iTerm.app" {
+    if xtversion().unwrap().starts_with(L!("iTerm2 ")) {
         // iTerm2 now defaults to Unicode 9 sizes for anything after macOS 10.12
         FISH_EMOJI_WIDTH.store(2, Ordering::Relaxed);
         FLOG!(term_support, "default emoji width 2 for iTerm2");
+    } else if term_program == "Apple_Terminal" && version as i32 >= 400 {
+        // Apple Terminal on High Sierra
+        FISH_EMOJI_WIDTH.store(2, Ordering::Relaxed);
+        FLOG!(term_support, "default emoji width: 2 for", term_program);
     } else {
         // Default to whatever the system's wcwidth gives for U+1F603, but only if it's at least
         // 1 and at most 2.
@@ -372,7 +373,6 @@ pub fn env_dispatch_init(vars: &EnvStack) {
 fn run_inits(vars: &EnvStack) {
     init_locale(vars);
     init_terminal(vars);
-    guess_emoji_width(vars);
     update_wait_on_escape_ms(vars);
     update_wait_on_sequence_key_ms(vars);
     handle_read_limit_change(vars);

src/input_common.rs

@@ -13,7 +13,9 @@
 use crate::reader::reader_test_and_clear_interrupted;
 use crate::terminal::{SCROLL_FORWARD_SUPPORTED, SCROLL_FORWARD_TERMINFO_CODE};
 use crate::threads::iothread_port;
-use crate::tty_handoff::{get_kitty_keyboard_capability, maybe_set_kitty_keyboard_capability};
+use crate::tty_handoff::{
+    get_kitty_keyboard_capability, maybe_set_kitty_keyboard_capability, XTVERSION,
+};
 use crate::universal_notifier::default_notifier;
 use crate::wchar::{encode_byte_to_char, prelude::*};
 use crate::wutil::encoding::{mbrtowc, mbstate_t, zero_mbstate};
@@ -1439,13 +1441,14 @@ fn parse_xtversion(&mut self, buffer: &mut Vec<u8>) -> Option<()> {
         if buffer.get(3)? != &b'|' {
             return None;
         }
-        FLOG!(
-            reader,
-            format!(
-                "Received XTVERSION response: {}",
-                str2wcstring(&buffer[4..buffer.len()])
-            )
-        );
+        XTVERSION.get_or_init(|| {
+            let xtversion = str2wcstring(&buffer[4..buffer.len()]);
+            FLOG!(
+                reader,
+                format!("Received XTVERSION response: {}", xtversion)
+            );
+            xtversion
+        });
         None
     }
 

src/reader.rs

@@ -146,6 +146,7 @@
     tok_command, MoveWordStateMachine, MoveWordStyle, TokenType, Tokenizer, TOK_ACCEPT_UNFINISHED,
     TOK_SHOW_COMMENTS,
 };
+use crate::tty_handoff::XTVERSION;
 use crate::tty_handoff::{
     get_tty_protocols_active, initialize_tty_metadata, maybe_set_kitty_keyboard_capability,
     safe_deactivate_tty_protocols, TtyHandoff,
@@ -272,7 +273,7 @@ pub fn terminal_init() -> InputEventQueue {
     let mut input_queue = InputEventQueue::new(STDIN_FILENO);
 
     let _init_tty_metadata = ScopeGuard::new((), |()| {
-        initialize_tty_metadata();
+        initialize_tty_metadata(XTVERSION.get_or_init(WString::new));
     });
 
     if !querying_allowed(STDIN_FILENO) {

src/tty_handoff.rs

@@ -14,13 +14,20 @@
 };
 use crate::terminal::{Output, Outputter};
 use crate::threads::assert_is_main_thread;
+use crate::wchar::prelude::*;
 use crate::wchar_ext::ToWString;
-use crate::wutil::perror;
+use crate::wutil::{perror, wcstoi};
 use libc::{EINVAL, ENOTTY, EPERM, STDIN_FILENO, WNOHANG};
 use once_cell::sync::OnceCell;
 use std::mem::MaybeUninit;
 use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
 
+pub static XTVERSION: OnceCell<WString> = OnceCell::new();
+
+pub fn xtversion() -> Option<&'static wstr> {
+    XTVERSION.get().as_ref().map(|s| s.as_utfstr())
+}
+
 // Facts about our environment, which inform how we handle the tty.
 #[derive(Debug, Copy, Clone)]
 pub struct TtyMetadata {
@@ -33,13 +40,11 @@ pub struct TtyMetadata {
 
 impl TtyMetadata {
     // Create a new TtyMetadata instance with the current environment.
-    fn detect() -> Self {
-        use std::env::var_os;
-
-        let in_tmux = var_os("TMUX").is_some();
-
+    fn detect(xtversion: &wstr) -> Self {
+        let in_tmux = xtversion.starts_with(L!("tmux "));
         // Detect iTerm2 before 3.5.12.
-        let pre_kitty_iterm2 = get_iterm2_version().is_some_and(|v| v < (3, 5, 12));
+        let pre_kitty_iterm2 = get_iterm2_version(xtversion).is_some_and(|v| v < (3, 5, 12));
+
         Self {
             in_tmux,
             pre_kitty_iterm2,
@@ -185,30 +190,21 @@ fn tty_protocols() -> Option<&'static TtyProtocolsSet> {
     unsafe { TTY_PROTOCOLS.load(Ordering::Acquire).as_ref() }
 }
 
-// Get the TTY protocols, initializing it if necessary.
+// Initialize TTY metadata.
 // This also initializes the terminal enable and disable serialized commands.
-// Note in practice this is only used from the main thread - races are very unlikely.
-fn get_or_init_tty_protocols() -> &'static TtyProtocolsSet {
+pub fn initialize_tty_metadata(xtversion: &wstr) {
     use std::sync::atomic::Ordering::{Acquire, Release};
     // Standard lazy-init pattern from rust-atomics-and-locks.
     let mut p = TTY_PROTOCOLS.load(Acquire);
     if p.is_null() {
         // Try to swap in a new TTY protocols set.
-        p = Box::into_raw(Box::new(TtyMetadata::detect().get_protocols()));
-        if let Err(e) = TTY_PROTOCOLS.compare_exchange(std::ptr::null_mut(), p, Release, Acquire) {
+        p = Box::into_raw(Box::new(TtyMetadata::detect(xtversion).get_protocols()));
+        if let Err(_e) = TTY_PROTOCOLS.compare_exchange(std::ptr::null_mut(), p, Release, Acquire) {
             // Safety: p comes from Box::into_raw right above,
             // and wasn't shared with any other thread.
             drop(unsafe { Box::from_raw(p) });
-            p = e;
         }
     }
-    // Safety: p is not null and points to a properly initialized value.
-    unsafe { &*p }
-}
-
-// Initialize TTY metadata.
-pub fn initialize_tty_metadata() {
-    get_or_init_tty_protocols();
 }
 
 // A marker of the current state of the tty protocols.
@@ -563,17 +559,18 @@ fn drop(&mut self) {
 }
 
 // If we are running under iTerm2, get the version as a tuple of (major, minor, patch).
-fn get_iterm2_version() -> Option<(u32, u32, u32)> {
-    use std::env::var;
-    let term = var("LC_TERMINAL").ok()?;
-    if term != "iTerm2" {
+fn get_iterm2_version(xtversion: &wstr) -> Option<(u32, u32, u32)> {
+    // TODO split_once
+    let mut xtversion = xtversion.split(' ');
+    let name = xtversion.next().unwrap();
+    let version = xtversion.next()?;
+    if name != "iTerm2" {
         return None;
     }
-    let version = var("LC_TERMINAL_VERSION").ok()?;
     let mut parts = version.split('.');
     Some((
-        parts.next()?.parse().ok()?,
-        parts.next()?.parse().ok()?,
-        parts.next()?.parse().ok()?,
+        wcstoi(parts.next()?).ok()?,
+        wcstoi(parts.next()?).ok()?,
+        wcstoi(parts.next()?).ok()?,
     ))
 }

tests/test_driver.py

@@ -62,15 +62,12 @@ def makeenv(script_path: Path, home: Path) -> Dict[str, str]:
         "LANGUAGE",
         "MC_SID",
         "MC_TMPDIR",
-        "LC_TERMINAL",
-        "LC_TERMINAL_VERSION",
         "COLORTERM",
         "KONSOLE_VERSION",
         "STY",
         "TERM",  # Erase this since we still respect TERM=dumb etc.
         "TERM_PROGRAM",
         "TERM_PROGRAM_VERSION",
-        "VTE_VERSION",
     ]:
         if var in env:
             del env[var]