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

command.go

@@ -263,39 +263,66 @@ func ExecuteCtrl(c parser.Command, v *VHS) error {
 	return nil
 }
 
+// modifierKeywordKeys maps the keyword tokens that Shift+<key> and Alt+<key>
+// accept (per parseShift/parseAlt) to the input.Key they should type.
+var modifierKeywordKeys = map[token.Type]input.Key{
+	token.ENTER:       input.Enter,
+	token.TAB:         input.Tab,
+	token.UP:          input.ArrowUp,
+	token.DOWN:        input.ArrowDown,
+	token.LEFT:        input.ArrowLeft,
+	token.RIGHT:       input.ArrowRight,
+	token.PAGE_UP:     input.PageUp,
+	token.PAGE_DOWN:   input.PageDown,
+	token.HOME:        input.Home,
+	token.END:         input.End,
+	token.BACKSPACE:   input.Backspace,
+	token.DELETE:      input.Delete,
+	token.INSERT:      input.Insert,
+	token.SPACE:       input.Space,
+	token.ESCAPE:      input.Escape,
+	token.SCROLL_UP:   input.PageUp,
+	token.SCROLL_DOWN: input.PageDown,
+}
+
+// typeModifiedKey types c.Args's key, held with the given modifier already
+// pressed. c.Args is either a keyword recognized by modifierKeywordKeys (as
+// accepted by parseShift/parseAlt) or a literal string typed character by
+// character.
+func typeModifiedKey(c parser.Command, v *VHS, modifierName string) error {
+	if k, ok := token.Keywords[c.Args]; ok {
+		key, ok := modifierKeywordKeys[k]
+		if !ok {
+			return fmt.Errorf("%s+%s is not supported", modifierName, c.Args)
+		}
+		if err := v.Page.Keyboard.Type(key); err != nil {
+			return fmt.Errorf("failed to type %s key: %w", c.Args, err)
+		}
+		return nil
+	}
+
+	for _, r := range c.Args {
+		if k, ok := keymap[r]; ok {
+			if err := v.Page.Keyboard.Type(k); err != nil {
+				return fmt.Errorf("failed to type key %c: %w", r, err)
+			}
+		}
+	}
+	return nil
+}
+
 // ExecuteAlt is a CommandFunc that presses the argument key with the alt key
 // held down on the running instance of vhs.
 func ExecuteAlt(c parser.Command, v *VHS) error {
-	err := v.Page.Keyboard.Press(input.AltLeft)
-	if err != nil {
+	if err := v.Page.Keyboard.Press(input.AltLeft); err != nil {
 		return fmt.Errorf("failed to press Alt key: %w", err)
 	}
-	if k, ok := token.Keywords[c.Args]; ok { //nolint:nestif
-		switch k {
-		case token.ENTER:
-			err = v.Page.Keyboard.Type(input.Enter)
-			if err != nil {
-				return fmt.Errorf("failed to type Enter key: %w", err)
-			}
-		case token.TAB:
-			err := v.Page.Keyboard.Type(input.Tab)
-			if err != nil {
-				return fmt.Errorf("failed to type Tab key: %w", err)
-			}
-		}
-	} else {
-		for _, r := range c.Args {
-			if k, ok := keymap[r]; ok {
-				err = v.Page.Keyboard.Type(k)
-				if err != nil {
-					return fmt.Errorf("failed to type key %c: %w", r, err)
-				}
-			}
-		}
+
+	if err := typeModifiedKey(c, v, "Alt"); err != nil {
+		return err
 	}
 
-	err = v.Page.Keyboard.Release(input.AltLeft)
-	if err != nil {
+	if err := v.Page.Keyboard.Release(input.AltLeft); err != nil {
 		return fmt.Errorf("failed to release Alt key: %w", err)
 	}
 
@@ -305,37 +332,15 @@ func ExecuteAlt(c parser.Command, v *VHS) error {
 // ExecuteShift is a CommandFunc that presses the argument key with the shift
 // key held down on the running instance of vhs.
 func ExecuteShift(c parser.Command, v *VHS) error {
-	err := v.Page.Keyboard.Press(input.ShiftLeft)
-	if err != nil {
+	if err := v.Page.Keyboard.Press(input.ShiftLeft); err != nil {
 		return fmt.Errorf("failed to press Shift key: %w", err)
 	}
 
-	if k, ok := token.Keywords[c.Args]; ok { //nolint:nestif
-		switch k {
-		case token.ENTER:
-			err = v.Page.Keyboard.Type(input.Enter)
-			if err != nil {
-				return fmt.Errorf("failed to type Enter key: %w", err)
-			}
-		case token.TAB:
-			err = v.Page.Keyboard.Type(input.Tab)
-			if err != nil {
-				return fmt.Errorf("failed to type Tab key: %w", err)
-			}
-		}
-	} else {
-		for _, r := range c.Args {
-			if k, ok := keymap[r]; ok {
-				err = v.Page.Keyboard.Type(k)
-				if err != nil {
-					return fmt.Errorf("failed to type key %c: %w", r, err)
-				}
-			}
-		}
+	if err := typeModifiedKey(c, v, "Shift"); err != nil {
+		return err
 	}
 
-	err = v.Page.Keyboard.Release(input.ShiftLeft)
-	if err != nil {
+	if err := v.Page.Keyboard.Release(input.ShiftLeft); err != nil {
 		return fmt.Errorf("failed to release Shift key: %w", err)
 	}
 

parser/parser.go

@@ -351,18 +351,41 @@ func (p *Parser) parseCtrl() Command {
 	return Command{Type: token.CTRL, Args: ctrlArgs}
 }
 
+// modifierKeywordArgs are the keyword tokens accepted as the argument to a
+// Shift+ or Alt+ combo, in addition to a literal STRING. Kept in sync with
+// the keys ExecuteShift/ExecuteAlt know how to type (modifierKeywordKeys in
+// command.go).
+var modifierKeywordArgs = map[token.Type]bool{
+	token.STRING:        true,
+	token.LEFT_BRACKET:  true,
+	token.RIGHT_BRACKET: true,
+	token.ENTER:         true,
+	token.TAB:           true,
+	token.UP:            true,
+	token.DOWN:          true,
+	token.LEFT:          true,
+	token.RIGHT:         true,
+	token.PAGE_UP:       true,
+	token.PAGE_DOWN:     true,
+	token.HOME:          true,
+	token.END:           true,
+	token.BACKSPACE:     true,
+	token.DELETE:        true,
+	token.INSERT:        true,
+	token.SPACE:         true,
+	token.ESCAPE:        true,
+	token.SCROLL_UP:     true,
+	token.SCROLL_DOWN:   true,
+}
+
 // parseAlt parses an alt command.
 // An alt command takes a character to type while the modifier is held down.
 //
 //	Alt+<character>
 func (p *Parser) parseAlt() Command {
 	if p.peek.Type == token.PLUS {
 		p.nextToken()
-		if p.peek.Type == token.STRING ||
-			p.peek.Type == token.ENTER ||
-			p.peek.Type == token.LEFT_BRACKET ||
-			p.peek.Type == token.RIGHT_BRACKET ||
-			p.peek.Type == token.TAB {
+		if modifierKeywordArgs[p.peek.Type] {
 			c := p.peek.Literal
 			p.nextToken()
 			return Command{Type: token.ALT, Args: c}
@@ -384,11 +407,7 @@ func (p *Parser) parseAlt() Command {
 func (p *Parser) parseShift() Command {
 	if p.peek.Type == token.PLUS {
 		p.nextToken()
-		if p.peek.Type == token.STRING ||
-			p.peek.Type == token.ENTER ||
-			p.peek.Type == token.LEFT_BRACKET ||
-			p.peek.Type == token.RIGHT_BRACKET ||
-			p.peek.Type == token.TAB {
+		if modifierKeywordArgs[p.peek.Type] {
 			c := p.peek.Literal
 			p.nextToken()
 			return Command{Type: token.SHIFT, Args: c}

parser/parser_test.go

@@ -345,6 +345,98 @@ func TestParseCtrl(t *testing.T) {
 	}
 }
 
+// TestParseShift is a regression test for
+// https://github.com/charmbracelet/vhs/issues/641: Shift+<arrow/nav key>
+// used to fail to parse at all, so ExecuteShift's keyword switch never had a
+// chance to run for these keys.
+func TestParseShift(t *testing.T) {
+	tests := []struct {
+		name    string
+		tape    string
+		wantArg string
+		wantErr bool
+	}{
+		{name: "Shift+Enter", tape: "Shift+Enter", wantArg: "Enter"},
+		{name: "Shift+Tab", tape: "Shift+Tab", wantArg: "Tab"},
+		{name: "Shift+Up", tape: "Shift+Up", wantArg: "Up"},
+		{name: "Shift+Down", tape: "Shift+Down", wantArg: "Down"},
+		{name: "Shift+Left", tape: "Shift+Left", wantArg: "Left"},
+		{name: "Shift+Right", tape: "Shift+Right", wantArg: "Right"},
+		{name: "Shift+PageUp", tape: "Shift+PageUp", wantArg: "PageUp"},
+		{name: "Shift+PageDown", tape: "Shift+PageDown", wantArg: "PageDown"},
+		{name: "Shift+Home", tape: "Shift+Home", wantArg: "Home"},
+		{name: "Shift+End", tape: "Shift+End", wantArg: "End"},
+		{name: "Shift+Backspace", tape: "Shift+Backspace", wantArg: "Backspace"},
+		{name: "Shift+A", tape: "Shift+A", wantArg: "A"},
+		{name: "should not parse Shift with no argument", tape: "Shift+", wantErr: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			l := lexer.New(tc.tape)
+			p := New(l)
+
+			cmd := p.parseShift()
+			if tc.wantErr {
+				if len(p.errors) == 0 {
+					t.Errorf("Expected to parse with errors but was success")
+				}
+				return
+			}
+
+			if len(p.errors) > 0 {
+				t.Fatalf("unexpected parse errors: %v", p.errors)
+			}
+
+			if cmd.Args != tc.wantArg {
+				t.Errorf("want arg %q, got %q", tc.wantArg, cmd.Args)
+			}
+		})
+	}
+}
+
+// TestParseAlt mirrors TestParseShift for Alt+<key>.
+func TestParseAlt(t *testing.T) {
+	tests := []struct {
+		name    string
+		tape    string
+		wantArg string
+		wantErr bool
+	}{
+		{name: "Alt+Enter", tape: "Alt+Enter", wantArg: "Enter"},
+		{name: "Alt+Tab", tape: "Alt+Tab", wantArg: "Tab"},
+		{name: "Alt+Up", tape: "Alt+Up", wantArg: "Up"},
+		{name: "Alt+Down", tape: "Alt+Down", wantArg: "Down"},
+		{name: "Alt+Left", tape: "Alt+Left", wantArg: "Left"},
+		{name: "Alt+Right", tape: "Alt+Right", wantArg: "Right"},
+		{name: "Alt+A", tape: "Alt+A", wantArg: "A"},
+		{name: "should not parse Alt with no argument", tape: "Alt+", wantErr: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			l := lexer.New(tc.tape)
+			p := New(l)
+
+			cmd := p.parseAlt()
+			if tc.wantErr {
+				if len(p.errors) == 0 {
+					t.Errorf("Expected to parse with errors but was success")
+				}
+				return
+			}
+
+			if len(p.errors) > 0 {
+				t.Fatalf("unexpected parse errors: %v", p.errors)
+			}
+
+			if cmd.Args != tc.wantArg {
+				t.Errorf("want arg %q, got %q", tc.wantArg, cmd.Args)
+			}
+		})
+	}
+}
+
 type parseSourceTest struct {
 	tape      string
 	srcTape   string