Add bootcommand mappings for left/right command and option keys#293
Add bootcommand mappings for left/right command and option keys#293torarnv wants to merge 2 commits intohashicorp:mainfrom
Conversation
|
I see there's a copy of the docs in |
f896caa to
c4c5259
Compare
|
@anshulsharma-hashicorp, @anurag5sh, @JenGoldstrich, would one of you have time to review this PR, or suggest someone else who could? I'd love to get this one merged in. It would make working with macOS VMs nicer. |
|
@anshulsharma-hashicorp, @anurag5sh, @JenGoldstrich is there any feedback on this PR? How do we proceed? Thanks 😊 |
|
@anshulsharma-hashicorp, @anurag5sh, @JenGoldstrich Can you please comment on this PR? Thanks |
The super key maps to ⌘ for the USB driver, but for the VNC driver ⌘ is produced by left/right Alt, so we can't promise that leftSuper/rightSuper produces ⌘.
When automating macOS systems it's convenient to refer to the keys by their natural names, and for the VNC driver none of the existing special keys mapped to option, so this enables boot commands using the option key.
c4c5259 to
81531cc
Compare
Awaiting the merge of the feature in the upstream Packer SDK repo, let's override the plugin SDK with a branch that has the feature. See hashicorp/packer-plugin-sdk#293
Awaiting the merge of the feature in the upstream Packer SDK repo. See hashicorp/packer-plugin-sdk#293
Awaiting the merge of the feature in the upstream Packer SDK repo. See hashicorp/packer-plugin-sdk#293
Awaiting the merge of the feature in the upstream Packer SDK repo. See hashicorp/packer-plugin-sdk#293
|
@tanmay-hc Is Packer maintained still? |
Awaiting the merge of the feature in the upstream Packer SDK repo. See hashicorp/packer-plugin-sdk#293
|
@devashish-patel @taru-garg-hashicorp @kp2099 Would appreciate a review of this PR, thanks! |
There was a problem hiding this comment.
Pull request overview
This PR expands the boot command special-key vocabulary so macOS automation can refer to Command/Option by their natural names, including left/right variants, across the boot command parser and relevant drivers.
Changes:
- Added
<leftCommand>/<rightCommand>and<leftOption>/<rightOption>to the boot command grammar and regenerated the parser. - Added corresponding key mappings for the VNC and USB bootcommand drivers.
- Updated boot command documentation to mention the new special keys.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
bootcommand/vnc_driver.go |
Adds VNC keysym mappings for left/right Command and Option. |
bootcommand/usb_driver.go |
Adds USB scancode mappings for left/right Command and Option. |
bootcommand/config.go |
Documents the new special keys. |
bootcommand/boot_command.pigeon |
Extends the grammar to recognize the new special keys. |
bootcommand/boot_command.go |
Regenerated parser output reflecting the grammar changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Verified against the built-in VNC server on macOS | ||
| sMap["leftcommand"] = 0xFFE9 | ||
| sMap["rightcommand"] = 0xFFEA | ||
| sMap["leftoption"] = 0xFFE7 | ||
| sMap["rightoption"] = 0xFFE8 |
There was a problem hiding this comment.
New special-key mappings were added here, but there’s no corresponding test coverage to ensure <leftCommand>, <rightCommand>, <leftOption>, and <rightOption> generate the expected keysyms (and remain case-insensitive). Consider extending vnc_driver_test.go to assert the emitted key events for these new tokens.
| // https://developer.apple.com/accessories/Accessory-Design-Guidelines.pdf | ||
| "leftcommand": key.CodeLeftGUI, | ||
| "rightcommand": key.CodeRightGUI, | ||
| "leftoption": key.CodeLeftAlt, | ||
| "rightoption": key.CodeRightAlt, | ||
| } |
There was a problem hiding this comment.
New special-key mappings were added here, but the USB driver tests currently don’t cover <leftCommand>, <rightCommand>, <leftOption>, or <rightOption>. Consider adding cases to usb_driver_test.go to verify these tokens map to the intended key.Code values (and that mixed-case input still works).
| / "leftCommand"i / "rightCommand"i / "leftOption"i / "rightOption"i | ||
| // left/right must go last, to not take priority over {left/right}Foo | ||
| / "left"i / "right"i / "menu"i |
There was a problem hiding this comment.
The grammar now accepts <leftCommand>, <rightCommand>, <leftOption>, and <rightOption>, but the PC-XT driver (pc_xt_driver.go) does not map these specials. This means templates using these keys will parse successfully but fail at runtime with “special … not found” when the PC-XT driver is used. Consider adding aliases in the PC-XT driver (e.g., command -> left/right super (GUI), option -> left/right alt) or restrict/qualify these tokens to drivers that support them.
| // - `<leftCommand> <rightCommand>` - Simulates pressing the ⌘ key. | ||
| // | ||
| // - `<leftOption> <rightOption>` - Simulates pressing the ⌥ key. |
There was a problem hiding this comment.
This doc block now lists <leftCommand>/<rightCommand> and <leftOption>/<rightOption> as generally available special keys, but the PC-XT driver does not currently implement these mappings. Either add the aliases to the PC-XT driver or clarify here that these keys are only supported by specific drivers (e.g., VNC/USB) to avoid runtime surprises.
| // - `<leftCommand> <rightCommand>` - Simulates pressing the ⌘ key. | |
| // | |
| // - `<leftOption> <rightOption>` - Simulates pressing the ⌥ key. | |
| // - `<leftCommand> <rightCommand>` - Simulates pressing the ⌘ key. Supported | |
| // only by certain keyboard drivers (such as VNC/USB), and not by all | |
| // builders (for example, the legacy PC-XT driver). | |
| // | |
| // - `<leftOption> <rightOption>` - Simulates pressing the ⌥ key. Supported | |
| // only by certain keyboard drivers (such as VNC/USB), and not by all | |
| // builders (for example, the legacy PC-XT driver). |
When automating macOS systems it's convenient to refer to the keys by their natural names, and for the VNC driver none of the existing special keys mapped to option, so this enables boot commands using the option key.