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 <noreply@anthropic.com>
This commit is contained in:
Michael Grant
2026-09-23 09:02:13 +01:00
parent 20f76fb069
commit 6933bc417c
4 changed files with 117 additions and 149 deletions

117
regress/tty-margins-scrollbar.sh Executable file
View File

@@ -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

View File

@@ -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

View File

@@ -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;

View File

@@ -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);