feat(term): let the cell size be configured when the pty reports none - #376
Open
szsolt wants to merge 1 commit into
Open
feat(term): let the cell size be configured when the pty reports none#376szsolt wants to merge 1 commit into
szsolt wants to merge 1 commit into
Conversation
Some ptys report cell counts but no pixel geometry -- WSL2 always, and often
SSH. utils/term then assumed 8x16, and since cell size scales every crop
rectangle, a wrong assumption samples the wrong region of the source image.
Add a `cell_size` option, accepting "14x32" or { width = 14, height = 32 }.
It is consulted only when TIOCGWINSZ reports no pixels, so terminals that do
report geometry are unaffected.
term.lua is required (via utils) before setup() runs, so its size is computed
at load time and an option read there would be ignored. Export set_cell_size()
and call it from setup() to re-resolve once the option is known.
Also honour IMAGE_NVIM_CELL_SIZE as a fallback when no option is given: Neovim
cannot query the terminal itself -- TermResponse never fires for the CSI 16 t
reply, and a child process gets no /dev/tty -- so the value sometimes has to
come from the shell, which does own the tty:
printf '\033[16t' # reply: CSI 6 ; height ; width t
Precedence is option > IMAGE_NVIM_CELL_SIZE > 8x16. Sizes of zero are rejected
rather than passed through to divide cell_width downstream.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
utils/termgets the cell size fromTIOCGWINSZ, but some ptys report cell counts whileleaving the pixel fields zero. WSL2 does this always; SSH often does.
update_size()thenfalls back to assuming 8x16.
Cell size scales every crop rectangle in
backends/kitty/init.lua, so when the assumptionis wrong the crop samples the wrong region of the source image — images render visibly
mis-cropped or mis-scaled rather than failing outright. On this machine the real cells are
14x32, so every dimension was off by roughly a factor of two.
Neovim can't discover the value on its own.
CSI 16 tis answered by the terminal, butTermResponsenever fires for the reply, and a child process spawned to read it gets no/dev/tty. So the size has to be supplied from outside.Independently reported at wezterm#6781,
where the same WSL setup reports
{size}: 0 x 0 pixels/{cell}: 0 x 0 pixels— thezero there is severe enough to make
wezterm imgcatitself bail with "no size informationavailable", and an earlier build divide by zero.
Change
Add a
cell_sizeoption:It's consulted only when
TIOCGWINSZreports no pixel geometry, so terminals that doreport it are completely unaffected.
One wrinkle worth flagging for review:
utils/termis required (viautils) beforesetup()runs, and it computes its size at load time — so an option read there wouldsilently never apply. This exports
set_cell_size()and calls it fromsetup()tore-resolve once the option is known.
IMAGE_NVIM_CELL_SIZEis also honoured when no option is passed, which covers the casewhere the value has to come from the shell — the shell does own the tty, so it can ask
the terminal directly, and it doesn't have to be hardcoded per machine:
Precedence is
cell_size>IMAGE_NVIM_CELL_SIZE> 8x16. Zero sizes are rejected insteadof being passed through to divide
cell_widthdownstream.zsh snippet to detect and export it automatically
Sourced from
.zshrc. The point of querying rather than hardcoding is that it followsfont-size changes and works unchanged across machines.
() { [[ -n ${IMAGE_NVIM_CELL_SIZE:-} ]] && return 0 [[ -o interactive ]] || return 0 [[ -t 0 && -t 1 ]] || return 0 # Skip terminals that won't answer; they'd just eat the timeout below. case ${TERM:-}${TERM_PROGRAM:-} in *kitty*|*ghostty*|*WezTerm*|*wezterm*) ;; *) [[ -n ${KITTY_WINDOW_ID:-} || -n ${WEZTERM_PANE:-} ]] || return 0 ;; esac local saved reply saved=$(stty -g 2>/dev/null) || return 0 # min 0 time 3: give up after 0.3s rather than hanging shell startup. stty raw -echo min 0 time 3 2>/dev/null || return 0 printf '\033[16t' > /dev/tty IFS= read -r -d 't' reply stty "$saved" 2>/dev/null # Reply: CSI 6 ; height ; width t if [[ $reply =~ '\[6;([0-9]+);([0-9]+)$' ]]; then local h=$match[1] w=$match[2] (( h > 0 && w > 0 )) && export IMAGE_NVIM_CELL_SIZE="${w}x${h}" fi return 0 }Note it must run in the shell, not from inside Neovim — a process Neovim spawns has no
/dev/ttyto read the reply from, which is the whole reason this can't be autodetectedin-process. Restores tty state on every exit path, and bounded to 0.3s so a
non-responding terminal can't hang startup.
Verified
Under a pty with known dimensions, exercising each rung:
IMAGE_NVIM_CELL_SIZE=14x32cell_size = "20x40"cell_size = { width = 7, height = 15 }cell_size = "garbage"cell_size = nil, env setHappy to drop the env-var half if you'd rather keep the surface to the option alone,
though it is the only channel that works when the value must come from the shell.