fix: address comprehensive shell integration code review issues

- Fix elvish function name mismatch and use conj for list operations
- Simplify terminfo installation command per ghostty docs (tic -x -)
- Fix conditional structure to ensure error messages always print
- Remove redundant checks and optimize array initialization
- Use consistent patterns across bash, fish, elvish, and zsh
implementations
This commit is contained in:
Jason Rayne
2025-06-17 16:37:37 -07:00
parent 995fb09813
commit b6bb9abfbc
4 changed files with 82 additions and 103 deletions

View File

@ -157,16 +157,12 @@ if [[ -n "$GHOSTTY_SSH_INTEGRATION" ]]; then
if builtin command -v infocmp >/dev/null 2>&1; then
echo "Installing Ghostty terminfo on remote host..." >&2
# Step 1: Install terminfo using the same approach that works manually
# This requires authentication but is quick and reliable
if infocmp -x xterm-ghostty 2>/dev/null | builtin command ssh "$@" 'mkdir -p ~/.terminfo/x 2>/dev/null && tic -x -o ~/.terminfo /dev/stdin 2>/dev/null'; then
# Step 1: Install terminfo
if infocmp -x xterm-ghostty 2>/dev/null | builtin command ssh "$@" 'tic -x - 2>/dev/null'; then
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
local env_vars=()
# Use xterm-ghostty since we just installed it
env_vars+=("TERM=xterm-ghostty")
local env_vars=("TERM=xterm-ghostty")
# Propagate Ghostty shell integration environment variables
[[ -n "$GHOSTTY_SHELL_FEATURES" ]] && env_vars+=("GHOSTTY_SHELL_FEATURES=$GHOSTTY_SHELL_FEATURES")
@ -174,9 +170,8 @@ if [[ -n "$GHOSTTY_SSH_INTEGRATION" ]]; then
# Normal SSH connection with Ghostty terminfo available
env "${env_vars[@]}" ssh "$@"
builtin return 0
else
echo "Terminfo installation failed. Using basic integration." >&2
fi
echo "Terminfo installation failed. Using basic integration." >&2
fi
# Fallback to basic integration

View File

@ -130,14 +130,12 @@
# Fix TERM compatibility
if (eq "xterm-ghostty" $E:TERM) {
set env-vars = [$@env-vars TERM=xterm-256color]
set env-vars = (conj $env-vars TERM=xterm-256color)
}
# Propagate Ghostty shell integration environment variables
if (has-env GHOSTTY_SHELL_FEATURES) {
if (not-eq "" $E:GHOSTTY_SHELL_FEATURES) {
set env-vars = [$@env-vars GHOSTTY_SHELL_FEATURES=$E:GHOSTTY_SHELL_FEATURES]
}
if (not-eq "" $E:GHOSTTY_SHELL_FEATURES) {
set env-vars = (conj $env-vars GHOSTTY_SHELL_FEATURES=$E:GHOSTTY_SHELL_FEATURES)
}
# Execute with environment variables if any were set
@ -148,47 +146,39 @@
}
}
fn ghostty-ssh-full {|@args|
# Full integration: Two-step terminfo installation
if (has-external infocmp) {
echo "Installing Ghostty terminfo on remote host..." >&2
fn ssh-full {|@args|
# Full integration: Two-step terminfo installation
if (has-external infocmp) {
echo "Installing Ghostty terminfo on remote host..." >&2
# Step 1: Install terminfo using the same approach that works manually
# This requires authentication but is quick and reliable
try {
infocmp -x xterm-ghostty 2>/dev/null | command ssh $@args 'mkdir -p ~/.terminfo/x 2>/dev/null && tic -x -o ~/.terminfo /dev/stdin 2>/dev/null'
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
try {
infocmp -x xterm-ghostty 2>/dev/null | (external ssh) $@args 'tic -x - 2>/dev/null'
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
var env-vars = []
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
var env-vars = [TERM=xterm-ghostty]
# Use xterm-ghostty since we just installed it
set env-vars = [$@env-vars TERM=xterm-ghostty]
# Propagate Ghostty shell integration environment variables
if (not-eq "" $E:GHOSTTY_SHELL_FEATURES) {
set env-vars = (conj $env-vars GHOSTTY_SHELL_FEATURES=$E:GHOSTTY_SHELL_FEATURES)
}
# Propagate Ghostty shell integration environment variables
if (has-env GHOSTTY_SHELL_FEATURES) {
if (not-eq "" $E:GHOSTTY_SHELL_FEATURES) {
set env-vars = [$@env-vars GHOSTTY_SHELL_FEATURES=$E:GHOSTTY_SHELL_FEATURES]
}
}
# Normal SSH connection with Ghostty terminfo available
(external env) $@env-vars ssh $@args
return
} catch e {
echo "Terminfo installation failed. Using basic integration." >&2
}
# Normal SSH connection with Ghostty terminfo available
(external env) $@env-vars ssh $@args
return
} catch e {
echo "Terminfo installation failed. Using basic integration." >&2
}
}
# Fallback to basic integration
ghostty-ssh-basic $@args
# Fallback to basic integration
ssh-basic $@args
}
# Register SSH integration if enabled
# Register SSH integration if enabled
if (and (has-env GHOSTTY_SSH_INTEGRATION) (has-external ssh)) {
edit:add-var ssh~ $ssh-with-ghostty-integration~
}
defer {
mark-prompt-start
report-pwd

View File

@ -104,10 +104,10 @@ function __ghostty_setup --on-event fish_prompt -d "Setup ghostty integration"
# Level: term-only - Just fix TERM compatibility
function _ghostty_ssh_term-only -d "SSH with TERM compatibility fix"
if test "$TERM" = xterm-ghostty
TERM=xterm-256color command ssh $argv
if test "$TERM" = "xterm-ghostty"
TERM=xterm-256color builtin command ssh $argv
else
command ssh $argv
builtin command ssh $argv
end
end
@ -117,7 +117,7 @@ function __ghostty_setup --on-event fish_prompt -d "Setup ghostty integration"
set --local env_vars
# Fix TERM compatibility
if test "$TERM" = xterm-ghostty
if test "$TERM" = "xterm-ghostty"
set --append env_vars TERM=xterm-256color
end
@ -127,10 +127,10 @@ function __ghostty_setup --on-event fish_prompt -d "Setup ghostty integration"
end
# Execute with environment variables if any were set
if test (count $env_vars) -gt 0
if test "$(count $env_vars)" -gt 0
env $env_vars ssh $argv
else
command ssh $argv
builtin command ssh $argv
end
end
@ -140,35 +140,28 @@ function __ghostty_setup --on-event fish_prompt -d "Setup ghostty integration"
if type -q infocmp
echo "Installing Ghostty terminfo on remote host..." >&2
# Step 1: Install terminfo using the same approach that works manually
# This requires authentication but is quick and reliable
if infocmp -x xterm-ghostty 2>/dev/null | command ssh $argv 'mkdir -p ~/.terminfo/x 2>/dev/null && tic -x -o ~/.terminfo /dev/stdin 2>/dev/null'
# Step 1: Install terminfo
if infocmp -x xterm-ghostty 2>/dev/null | ssh $argv 'tic -x - 2>/dev/null'
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
set -l env_vars
# Use xterm-ghostty since we just installed it
set -a env_vars TERM=xterm-ghostty
set --local env_vars TERM=xterm-ghostty
# Propagate Ghostty shell integration environment variables
if test -n "$GHOSTTY_SHELL_FEATURES"
set --append env_vars GHOSTTY_SHELL_FEATURES="$GHOSTTY_SHELL_FEATURES"
end
# Normal SSH connection with Ghostty terminfo available
env $env_vars ssh $argv
return 0
else
echo "Terminfo installation failed. Using basic integration." >&2
builtin return 0
end
echo "Terminfo installation failed. Using basic integration." >&2
end
# Fallback to basic integration
_ghostty_ssh_basic $argv
end
end
# Setup prompt marking
function __ghostty_mark_prompt_start --on-event fish_prompt --on-event fish_cancel --on-event fish_posterror
# If we never got the output end event, then we need to send it now.

View File

@ -276,49 +276,50 @@ _ghostty_deferred_init() {
# Level: basic - TERM fix + environment variable propagation
_ghostty_ssh_basic() {
# Fix TERM compatibility and propagate key environment variables
local env_vars=()
# Fix TERM compatibility
if [[ "$TERM" == "xterm-ghostty" ]]; then
TERM=xterm-256color \
GHOSTTY_SHELL_FEATURES="${GHOSTTY_SHELL_FEATURES}" \
builtin command ssh "$@"
env_vars+=("TERM=xterm-256color")
fi
# Propagate Ghostty shell integration environment variables
[[ -n "$GHOSTTY_SHELL_FEATURES" ]] && env_vars+=("GHOSTTY_SHELL_FEATURES=$GHOSTTY_SHELL_FEATURES")
# Execute with environment variables if any were set
if [[ ${#env_vars[@]} -gt 0 ]]; then
env "${env_vars[@]}" ssh "$@"
else
GHOSTTY_SHELL_FEATURES="${GHOSTTY_SHELL_FEATURES}" \
builtin command ssh "$@"
fi
}
# Level: full - All features
_ghostty_ssh_full() {
# Full integration: Two-step terminfo installation
if command -v infocmp >/dev/null 2>&1; then
echo "Installing Ghostty terminfo on remote host..." >&2
# Full integration: Two-step terminfo installation
if builtin command -v infocmp >/dev/null 2>&1; then
echo "Installing Ghostty terminfo on remote host..." >&2
# Step 1: Install terminfo using the same approach that works manually
# This requires authentication but is quick and reliable
if infocmp -x xterm-ghostty 2>/dev/null | command ssh "$@" 'mkdir -p ~/.terminfo/x 2>/dev/null && tic -x -o ~/.terminfo /dev/stdin 2>/dev/null'; then
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
# Step 1: Install terminfo
if infocmp -x xterm-ghostty 2>/dev/null | builtin command ssh "$@" 'tic -x - 2>/dev/null'; then
echo "Terminfo installed successfully. Connecting with full Ghostty support..." >&2
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
local env_vars=()
# Step 2: Connect with xterm-ghostty since we know terminfo is now available
local env_vars=("TERM=xterm-ghostty")
# Use xterm-ghostty since we just installed it
env_vars+=("TERM=xterm-ghostty")
# Propagate Ghostty shell integration environment variables
[[ -n "$GHOSTTY_SHELL_FEATURES" ]] && env_vars+=("GHOSTTY_SHELL_FEATURES=$GHOSTTY_SHELL_FEATURES")
# Propagate Ghostty shell integration environment variables
[[ -n "$GHOSTTY_SHELL_FEATURES" ]] && env_vars+=("GHOSTTY_SHELL_FEATURES=$GHOSTTY_SHELL_FEATURES")
# Normal SSH connection with Ghostty terminfo available
env "${env_vars[@]}" ssh "$@"
return 0
else
echo "Terminfo installation failed. Using basic integration." >&2
fi
# Normal SSH connection with Ghostty terminfo available
env "${env_vars[@]}" ssh "$@"
builtin return 0
fi
echo "Terminfo installation failed. Using basic integration." >&2
fi
# Fallback to basic integration
_ghostty_ssh_basic "$@"
# Fallback to basic integration
_ghostty_ssh_basic "$@"
}
fi
# Some zsh users manually run `source ~/.zshrc` in order to apply rc file