Skip to content

feat: surface terminal error responses via TermResponse on Neovim >= 0.12 - #56

Open
hisanari-dev wants to merge 1 commit into
mainfrom
feat/12-async-terminal-response-handling
Open

hisanari-dev wants to merge 1 commit into
mainfrom
feat/12-async-terminal-response-handling

Conversation

@hisanari-dev

Copy link
Copy Markdown
Contributor

📦 Pull Request

Description

Surfaces kitty graphics protocol error responses (e.g. a PNG the terminal rejected) instead of suppressing them unconditionally.

  • blit cannot read stdin itself (Neovim's TUI owns it), so the only safe channel is Neovim's TermResponse event, which delivers APC responses from Neovim 0.12 onward. The feature is therefore feature-detected (terminal.has_response_support()); the minimum supported version stays 0.10.
  • Neovim >= 0.12: transmits carry q=1 (suppress OK, keep errors). renderer.lua adds one TermResponse autocmd to the handle-gated blit augroup, parses each sequence with the new pure terminal.parse_response(), and records errors for ids it owns in a bounded list (most recent 20). :checkhealth blit lists them; renderer.response_errors() returns a copy.
  • Neovim 0.10 / 0.11: transmits keep q=2 and no listener is registered — behavior is unchanged.
  • Responses are diagnostic only. They arrive after show() has returned, so they cannot become a nil, err return value, and no recovery path (Ghostty retransmit, delete retries) reacts to them.
  • Memo correction: docs/spec/kitty-graphics.md said q=2 is set "always", but placement (a=p) and delete (a=d) commands have never carried a q key. The memo now says so; those bytes are unchanged in this PR. As a side effect, placement error responses are recorded on 0.12+ too.

Not verified in this PR: behavior against a real terminal. The unit tests deliver synthetic TermResponse events; the sequence shape was taken from Neovim 0.12.5's tui/input.c. Two steps were added to docs/manual-testing.md (no stray input after normal use; a corrupt PNG shows up in :checkhealth blit) and still need to be run on kitty, WezTerm, and Ghostty.

Related Issue

Closes #12

Type of Change

  • Bug fix
  • New feature
  • Refactoring
  • Documentation
  • CI / Infrastructure

Checklist

  • I have run make locally (stylua --check, selene, tests) and it passes.
  • I have added unit tests for new pure logic (escape sequences, chunking, geometry, detection).
  • I have updated the relevant docs/spec/ memo if protocol or terminal-detection behavior changed.
  • I have updated doc/blit.txt and/or README if this changes the public API.
  • I have followed Conventional Commits (feat:, fix:, etc.).

…0.12

Transmits now carry q=1 (suppress OK, keep errors) when Neovim can
deliver APC responses through TermResponse (0.12+). renderer.lua records
error responses for ids it owns in a bounded list, and :checkhealth blit
lists them. Neovim 0.10 / 0.11 keep q=2 and register no listener, so
their behavior is unchanged.

Responses are diagnostic only: they arrive after show() has returned and
no recovery path reacts to them.

Closes #12

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR successfully implements terminal error response handling for Neovim 0.12+. The implementation is well-architected with proper feature detection, bounded error storage (20 most recent errors), and comprehensive test coverage. The code correctly uses q=1 (suppress OK, keep errors) when TermResponse support is available, and falls back to q=2 (suppress all) for earlier Neovim versions. All changes are working correctly and ready to merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Async terminal response handling to surface transmission errors

1 participant