Skip to content

fix(command): support Shift/Alt + arrow and navigation keys - #779

Open
nityanand123gupta wants to merge 1 commit into
charmbracelet:mainfrom
nityanand123gupta:fix/shift-alt-arrow-nav-keys
Open

fix(command): support Shift/Alt + arrow and navigation keys#779
nityanand123gupta wants to merge 1 commit into
charmbracelet:mainfrom
nityanand123gupta:fix/shift-alt-arrow-nav-keys

Conversation

@nityanand123gupta

Copy link
Copy Markdown

Fixes #641.

Note: there was a previous attempt at this in #672, which went unreviewed and was auto-closed for staleness back in March. I read that PR for context before writing this one, but the fix here is my own independent implementation against current main, restructured to share logic between ExecuteShift/ExecuteAlt rather than duplicating the keyword switch, and it includes parser-level regression tests that #672 didn't have.

Root cause

Two separate gaps combine to make Shift+Down (and friends) a no-op:

  1. Parser: parseShift/parseAlt only accept a bare STRING, or the ENTER/TAB keyword tokens, as the argument after Shift+/Alt+. Since Down, Up, PageUp, Home, etc. are all registered keywords (not plain strings), Shift+Down fails to parse at all — this is exactly the "compile error" the reporter hit.
  2. Executor: even if parsing were fixed, ExecuteShift/ExecuteAlt's keyword switch only has cases for token.ENTER and token.TAB — every other keyword would silently fall through and do nothing.

Fix

  • Added a modifierKeywordArgs set in parser/parser.go covering every keyword vhs already knows how to type standalone (arrows, PageUp/PageDown, Home/End, Backspace/Delete/Insert, Space, Escape, ScrollUp/ScrollDown), and use it in both parseShift and parseAlt.
  • Added the matching modifierKeywordKeys map in command.go and a shared typeModifiedKey helper, replacing the near-duplicate bodies of ExecuteShift/ExecuteAlt. This also makes an unrecognized keyword an explicit error instead of a silent no-op, since the parser and executor keyword sets are now the same source of truth in spirit.

Testing

  • Added TestParseShift and TestParseAlt in parser/parser_test.go, covering every newly-supported keyword plus the existing Enter/Tab/literal-character cases and the no-argument error case.
  • Verified via git stash on parser/parser.go alone that these new subtests fail without the fix (Shift+Up/Down/Left/Right/PageUp/PageDown/End/Backspace etc. all failed to parse) and pass with it restored.
  • go build ./... and go test ./... pass; gofmt -l and go vet ./... are clean on the changed files.

Shift+Down (and Up/Left/Right/PageUp/PageDown/Home/End/Backspace/
Delete/Insert/Space/Escape) failed to parse at all: parseShift and
parseAlt only accepted a bare STRING or the ENTER/TAB keywords as the
combo's argument, rejecting every other keyword token with a parse
error. Even had parsing succeeded, ExecuteShift/ExecuteAlt's keyword
switch only handled ENTER and TAB, silently doing nothing for
anything else.

Add every keyword that vhs already knows how to type standalone (see
ExecuteKey's registrations) to both the parser's accepted-argument set
and a shared modifierKeywordKeys map used by both ExecuteShift and
ExecuteAlt, deduplicating the two near-identical functions into a
typeModifiedKey helper.

Fixes charmbracelet#641
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.

Can't combine Shift and Down

1 participant