fix(command): support Shift/Alt + arrow and navigation keys - #779
Open
nityanand123gupta wants to merge 1 commit into
Open
fix(command): support Shift/Alt + arrow and navigation keys#779nityanand123gupta wants to merge 1 commit into
nityanand123gupta wants to merge 1 commit into
Conversation
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
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.
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 betweenExecuteShift/ExecuteAltrather 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:parseShift/parseAltonly accept a bareSTRING, or theENTER/TABkeyword tokens, as the argument afterShift+/Alt+. SinceDown,Up,PageUp,Home, etc. are all registered keywords (not plain strings),Shift+Downfails to parse at all — this is exactly the "compile error" the reporter hit.ExecuteShift/ExecuteAlt's keywordswitchonly has cases fortoken.ENTERandtoken.TAB— every other keyword would silently fall through and do nothing.Fix
modifierKeywordArgsset inparser/parser.gocovering every keywordvhsalready knows how to type standalone (arrows, PageUp/PageDown, Home/End, Backspace/Delete/Insert, Space, Escape, ScrollUp/ScrollDown), and use it in bothparseShiftandparseAlt.modifierKeywordKeysmap incommand.goand a sharedtypeModifiedKeyhelper, replacing the near-duplicate bodies ofExecuteShift/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
TestParseShiftandTestParseAltinparser/parser_test.go, covering every newly-supported keyword plus the existing Enter/Tab/literal-character cases and the no-argument error case.git stashonparser/parser.goalone that these new subtests fail without the fix (Shift+Up/Down/Left/Right/PageUp/PageDown/End/Backspaceetc. all failed to parse) and pass with it restored.go build ./...andgo test ./...pass;gofmt -landgo vet ./...are clean on the changed files.