From 6933bc417ccad3d9934b9decdd21e6eb755c8b63 Mon Sep 17 00:00:00 2001 From: Michael Grant Date: Wed, 23 Sep 2026 09:02:13 +0100 Subject: [PATCH] tty: drop WT_SESSION-based Windows Terminal detection nicm is opposed to identifying a terminal via an environment variable, so remove the WindowsTerminal tty_default_features() entry and the WT_SESSION check in tty_term_create() entirely, pending some other accepted identification mechanism for Windows Terminal (it cannot be identified via the existing XTVERSION path at all - see the previous commit's now-removed comment for why). This leaves WezTerm and ghostty's margins grants in place - both are identified via the existing, already-accepted XTVERSION mechanism, with DECSLRM support confirmed directly in their own source, independent of this change. regress/tty-margins-wt-session.sh tested only the removed mechanism and is replaced by regress/tty-margins-scrollbar.sh, which checks the same underlying scrollbar/margins scroll-decision mechanism (still real, and still what WezTerm's and ghostty's table entries rely on) via the generic terminal-features option instead of simulating any one terminal's identification handshake. Co-Authored-By: Claude Sonnet 5 --- regress/tty-margins-scrollbar.sh | 117 ++++++++++++++++++++++++++++ regress/tty-margins-wt-session.sh | 124 ------------------------------ tty-features.c | 16 ---- tty-term.c | 9 --- 4 files changed, 117 insertions(+), 149 deletions(-) create mode 100755 regress/tty-margins-scrollbar.sh delete mode 100755 regress/tty-margins-wt-session.sh diff --git a/regress/tty-margins-scrollbar.sh b/regress/tty-margins-scrollbar.sh new file mode 100755 index 000000000..70c924ced --- /dev/null +++ b/regress/tty-margins-scrollbar.sh @@ -0,0 +1,117 @@ +#!/bin/sh + +# A pane that doesn't span the terminal's full width - because +# pane-scrollbars is on (the scrollbar occupies a column) or the pane is +# one of a side-by-side split - needs DECSLRM (left/right margin) support +# to use the fast native-scroll path (tty_cmd_linefeed()/scrollup()/ +# scrolldown()/reverseindex(), tty.c: +# "(!tty_full_width(tty, ctx) && !tty_use_margin(tty))"). Without it, every +# single scroll falls back to tty_redraw_region()'s full manual repaint of +# the whole region - a real, confirmed source of flicker (and, separately, +# of image content not surviving a scroll in branches with image support). +# +# tty_default_features() (tty-features.c) grants the "margins" feature to +# several terminals it can positively identify via XTVERSION/DA2 (mintty, +# iTerm2, WezTerm, ghostty, XTerm-as-VT420) - this checks the actual +# server-side scroll decision via the -vv log for the underlying mechanism +# those table entries all rely on, using the terminal-features option +# directly (which any of them - or a user's own terminal-overrides - +# ultimately feed into) rather than simulating any one terminal's +# identification handshake. + +PATH=/bin:/usr/bin +TERM=screen +LC_ALL=C.UTF-8 +export PATH TERM LC_ALL + +[ -z "$TEST_TMUX" ] && TEST_TMUX=$(readlink -f ../tmux) + +DIR=$(mktemp -d) || exit 1 +cd "$DIR" || exit 1 +INNER="$TEST_TMUX -vv -Lmarginsscrollbar-inner-$$ -f/dev/null" +OUTER="$TEST_TMUX -Lmarginsscrollbar-outer-$$ -f/dev/null" + +fail() +{ + echo "$*" >&2 + exit 1 +} + +cleanup() +{ + $OUTER kill-server 2>/dev/null + $INNER kill-server 2>/dev/null + cd / + rm -rf "$DIR" +} +trap cleanup 0 1 15 + +wait_for_client() +{ + i=0 + while [ "$i" -lt 50 ]; do + CLIENT=$($INNER list-clients -F '#{client_name}' 2>/dev/null) + [ -n "$CLIENT" ] && return 0 + sleep 0.1 + i=$((i + 1)) + done + fail "inner client did not attach" +} + +run_scroll_phase() +{ + label=$1 + margins=$2 + + rm -f tmux-server*.log + + $INNER new-session -d -s inner -x 40 -y 6 'exec sh' || exit 1 + $INNER set -g status off || exit 1 + $INNER set -g window-size manual || exit 1 + $INNER set -g pane-scrollbars on || exit 1 + if [ "$margins" = "on" ]; then + $INNER set -as terminal-features ',*:margins' || exit 1 + fi + + $OUTER new-session -d -x 40 -y 6 || exit 1 + OUTERPANE=$($OUTER list-panes -F '#{pane_id}') || exit 1 + $OUTER set -g status off || exit 1 + $OUTER set -g window-size manual || exit 1 + $OUTER set -g default-terminal screen-256color || exit 1 + $OUTER send-keys -t "$OUTERPANE" -l "$INNER attach -t inner" || exit 1 + $OUTER send-keys -t "$OUTERPANE" Enter || exit 1 + sleep 1 + + wait_for_client + + i=0 + while [ "$i" -lt 8 ]; do + $INNER send-keys -t inner Enter || exit 1 + sleep 0.2 + i=$((i + 1)) + done + sleep 0.3 + + LOG=$(ls tmux-server*.log 2>/dev/null | head -1) + [ -n "$LOG" ] || fail "$label: sanity: no server -vv log was produced" + + n=$(grep -c "tty_redraw_region.*large region redraw" "$LOG") + + $OUTER kill-server 2>/dev/null + $INNER kill-server 2>/dev/null + + echo "$n" +} + +# Phase 1: margins granted - must never fall back to a full region redraw. +n_with=$(run_scroll_phase "with margins" "on") +[ "$n_with" -eq 0 ] || + fail "with margins granted, scrolling a scrollbar-enabled pane still fell back to a full region redraw ($n_with times)" + +# Phase 2: sanity check - without margins, the same scenario must actually +# hit the fallback, proving phase 1 wasn't accidentally trivial. +n_without=$(run_scroll_phase "without margins" "off") +[ "$n_without" -gt 0 ] || + fail "sanity: without margins, scrolling a scrollbar-enabled pane never fell back to a full region redraw - this scenario no longer exercises the bug this test checks for" + +exit 0 diff --git a/regress/tty-margins-wt-session.sh b/regress/tty-margins-wt-session.sh deleted file mode 100755 index 858bcc3d5..000000000 --- a/regress/tty-margins-wt-session.sh +++ /dev/null @@ -1,124 +0,0 @@ -#!/bin/sh - -# Windows Terminal cannot be identified by the XTVERSION mechanism -# tty_default_features() (tty-features.c) otherwise uses for every other -# terminal in its table - its maintainers have explicitly declined to -# implement it (github.com/microsoft/terminal issue 18382, closed -# not_planned). tty_term_create() (tty-term.c) instead detects it via the -# WT_SESSION environment variable Windows Terminal sets for every child -# process, granting it the "margins" (DECSLRM) terminal feature. -# -# Without DECSLRM, a pane that doesn't span the terminal's full width - -# because pane-scrollbars is on (the scrollbar occupies a column) or the -# pane is one of a side-by-side split - can't use the fast native-scroll -# path (tty_cmd_linefeed()/scrollup()/scrolldown()/reverseindex(), tty.c: -# "(!tty_full_width(tty, ctx) && !tty_use_margin(tty))") and falls back to -# tty_redraw_region()'s full manual repaint on every single scroll - a -# real, confirmed source of flicker (and, separately, of image content not -# surviving a scroll). -# -# This checks the actual server-side decision via the -vv log: a pane with -# pane-scrollbars on, scrolled several times, must never fall back to -# tty_redraw_region() when the attaching client's environment carries -# WT_SESSION - and must (as a sanity check that this scenario would -# otherwise hit the fallback at all) when it does not. - -PATH=/bin:/usr/bin -TERM=screen -LC_ALL=C.UTF-8 -export PATH TERM LC_ALL - -[ -z "$TEST_TMUX" ] && TEST_TMUX=$(readlink -f ../tmux) - -DIR=$(mktemp -d) || exit 1 -cd "$DIR" || exit 1 -INNER="$TEST_TMUX -vv -Lwtsession-inner-$$ -f/dev/null" -OUTER="$TEST_TMUX -Lwtsession-outer-$$ -f/dev/null" - -fail() -{ - echo "$*" >&2 - exit 1 -} - -cleanup() -{ - $OUTER kill-server 2>/dev/null - $INNER kill-server 2>/dev/null - cd / - rm -rf "$DIR" -} -trap cleanup 0 1 15 - -wait_for_client() -{ - i=0 - while [ "$i" -lt 50 ]; do - CLIENT=$($INNER list-clients -F '#{client_name}' 2>/dev/null) - [ -n "$CLIENT" ] && return 0 - sleep 0.1 - i=$((i + 1)) - done - fail "inner client did not attach" -} - -run_scroll_phase() -{ - label=$1 - attachcmd=$2 - - rm -f tmux-server*.log - - $INNER new-session -d -s inner -x 40 -y 6 'exec sh' || exit 1 - $INNER set -g status off || exit 1 - $INNER set -g window-size manual || exit 1 - $INNER set -g pane-scrollbars on || exit 1 - - $OUTER new-session -d -x 40 -y 6 || exit 1 - OUTERPANE=$($OUTER list-panes -F '#{pane_id}') || exit 1 - $OUTER set -g status off || exit 1 - $OUTER set -g window-size manual || exit 1 - $OUTER set -g default-terminal screen-256color || exit 1 - $OUTER send-keys -t "$OUTERPANE" -l "$attachcmd" || exit 1 - $OUTER send-keys -t "$OUTERPANE" Enter || exit 1 - sleep 1 - - wait_for_client - - i=0 - while [ "$i" -lt 8 ]; do - $INNER send-keys -t inner Enter || exit 1 - sleep 0.2 - i=$((i + 1)) - done - sleep 0.3 - - LOG=$(ls tmux-server*.log 2>/dev/null | head -1) - [ -n "$LOG" ] || fail "$label: sanity: no server -vv log was produced" - - n=$(grep -c "tty_redraw_region.*large region redraw" "$LOG") - - $OUTER kill-server 2>/dev/null - $INNER kill-server 2>/dev/null - - echo "$n" -} - -# Phase 1: WT_SESSION present in the attaching client's environment - must -# never fall back to a full region redraw. -n_with=$(run_scroll_phase "with WT_SESSION" \ - "WT_SESSION=deadbeef-0000-0000-0000-000000000000 $INNER attach -t inner") -[ "$n_with" -eq 0 ] || - fail "with WT_SESSION set, scrolling a scrollbar-enabled pane still fell back to a full region redraw ($n_with times) - margins was not granted" - -# Phase 2: sanity check - without WT_SESSION, the same scenario must -# actually hit the fallback, proving phase 1 wasn't accidentally trivial. -# Explicitly strip WT_SESSION/WSLENV rather than relying on them being -# unset in the ambient environment - this test may itself be run from -# inside a real Windows Terminal/WSL session. -n_without=$(run_scroll_phase "without WT_SESSION" \ - "env -u WT_SESSION -u WSLENV $INNER attach -t inner") -[ "$n_without" -gt 0 ] || - fail "sanity: without WT_SESSION, scrolling a scrollbar-enabled pane never fell back to a full region redraw - this scenario no longer exercises the bug this test checks for" - -exit 0 diff --git a/tty-features.c b/tty-features.c index 14f1b83b9..782b8aab6 100644 --- a/tty-features.c +++ b/tty-features.c @@ -678,22 +678,6 @@ tty_default_features(struct client *c, const char *name, u_int version) "extkeys," "focus" }, - /* - * Windows Terminal cannot be identified by the XTVERSION - * mechanism used for the other entries above - its - * maintainers have declined to implement it (see - * github.com/microsoft/terminal issue 18382). It is instead - * detected via the WT_SESSION environment variable it sets - * for every child process (tty_term_create(), tty-term.c). - * DECSLRM support was independently confirmed by direct - * (non-tmux) testing; other capabilities have not been - * verified, so only margins is granted here - deliberately - * not the full modern-xterm feature bundle other entries - * get. - */ - { .name = "WindowsTerminal", - .features = "margins" - } }; u_int i; diff --git a/tty-term.c b/tty-term.c index 944ce628e..1ac078cd4 100644 --- a/tty-term.c +++ b/tty-term.c @@ -645,15 +645,6 @@ tty_term_create(struct tty *tty, char *name, char **caps, u_int ncaps, tty_parse_client_features(c, "256", ","); } - /* - * Windows Terminal cannot be identified by XTVERSION (its - * maintainers have declined to implement it), but it sets - * WT_SESSION for every child process - see the WindowsTerminal - * entry in tty_default_features()'s table (tty-features.c). - */ - if (environ_find(c->environ, "WT_SESSION") != NULL) - tty_default_features(c, "WindowsTerminal", 0); - /* Apply overrides so any capabilities used for features are changed. */ tty_term_apply_overrides(term);