feat: added io keys command for pressing keys with modifiers#264
Conversation
WalkthroughThis PR adds a complete key-combo pressing feature. Users can now press key combinations with modifiers (like 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
commands/keys.go (1)
52-58: ⚡ Quick winTrim whitespace from modifier parts for better UX.
Users might naturally type combos with spaces (e.g.,
"cmd + a"), but the current implementation would fail with"unsupported modifier 'cmd '"because spaces are not trimmed before the alias lookup.♻️ Proposed fix to trim spaces
modifiers := make([]string, 0, len(modifierParts)) for _, part := range modifierParts { - modifier, ok := modifierAliases[strings.ToLower(part)] + modifier, ok := modifierAliases[strings.ToLower(strings.TrimSpace(part))] if !ok { return wda.KeyCombo{}, fmt.Errorf("unsupported modifier '%s' in key combo '%s'", part, combo) } modifiers = append(modifiers, modifier)Also trim the key itself:
return wda.KeyCombo{ - Key: strings.ToLower(key), + Key: strings.ToLower(strings.TrimSpace(key)), Modifiers: modifiers, }, nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@commands/keys.go` around lines 52 - 58, The parsing of modifierParts fails when users include spaces (e.g., "cmd + a"); update the loop that iterates modifierParts to trim whitespace (use strings.TrimSpace) on each part before doing the alias lookup against modifierAliases and before appending to modifiers, and also trim the key token when building the resulting wda.KeyCombo (the variables to change are modifierParts, modifier, modifierAliases, modifiers, combo and the final key parsing that produces the key field of the wda.KeyCombo).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/keys.go`:
- Around line 52-58: The parser loop over modifierParts (using modifierAliases
and appending into modifiers to build a wda.KeyCombo) does not detect duplicate
modifiers; update the loop to track seen modifiers (e.g., a local
map[string]bool keyed by the resolved modifier value) and if a modifier is
encountered twice return an error (e.g., "duplicate modifier '%s' in key combo
'%s'") instead of appending a duplicate; ensure you reference the same resolved
modifier value (not the raw part) when checking/marking seen to prevent
duplicates like "cmd+cmd+a".
In `@devices/ios.go`:
- Around line 841-843: The RPC handler sends keys as []string but
IOSDevice.PressKeys currently forwards []wda.KeyCombo to d.wdaClient.PressKeys,
causing a JSON mismatch; change the contract by updating IOSDevice.PressKeys to
accept []string (or add a new overload that does) and convert each string into a
wda.KeyCombo before calling d.wdaClient.PressKeys, so the RPC payload (keys
[]string) is translated into the wda.KeyCombo slice expected by the WDA client
(referencing IOSDevice.PressKeys and d.wdaClient.PressKeys and the RPC handler
that expects keys []string).
In `@devices/wda/press-keys.go`:
- Around line 9-16: WdaClient.PressKeys is sending the wrong RPC payload shape
("keys": []KeyCombo objects) but device.io.keys expects "keys": []string; update
PressKeys to map the incoming []KeyCombo (type KeyCombo) to the string form
(e.g., modifiers + "+" + key or the same conversion used in devices/remote.go)
and pass that []string to CallRPC("device.io.keys", params) so the params become
{"keys": []string}; change only the conversion inside PressKeys (referencing
WdaClient.PressKeys, KeyCombo, and CallRPC) before calling the RPC.
---
Nitpick comments:
In `@commands/keys.go`:
- Around line 52-58: The parsing of modifierParts fails when users include
spaces (e.g., "cmd + a"); update the loop that iterates modifierParts to trim
whitespace (use strings.TrimSpace) on each part before doing the alias lookup
against modifierAliases and before appending to modifiers, and also trim the key
token when building the resulting wda.KeyCombo (the variables to change are
modifierParts, modifier, modifierAliases, modifiers, combo and the final key
parsing that produces the key field of the wda.KeyCombo).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efde09d2-0dea-4a54-86d7-280cec077f0b
📒 Files selected for processing (11)
cli/io.gocommands/keys.gocommands/keys_test.godevices/android.godevices/common.godevices/ios.godevices/remote.godevices/simulator.godevices/wda/press-keys.goserver/dispatch.goserver/server.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/openrpc.json`:
- Around line 287-297: The "keys" parameter schema currently allows an empty
array even though the description requires "one or more key combinations";
update the "keys" property's JSON schema (the object with "name": "keys") to
include "minItems": 1 under its "schema" object so the OpenRPC contract enforces
at least one item is provided.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| { | ||
| "name": "keys", | ||
| "description": "Key combinations to press, in order (e.g. [\"cmd+a\", \"backspace\"])", | ||
| "required": true, | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add minItems: 1 constraint to match the description.
The description at line 277 states "Presses one or more key combinations", but the schema allows an empty keys array. Add a minItems constraint to enforce the documented contract and prevent no-op calls.
📋 Proposed fix
{
"name": "keys",
"description": "Key combinations to press, in order (e.g. [\"cmd+a\", \"backspace\"])",
"required": true,
"schema": {
"type": "array",
+ "minItems": 1,
"items": {
"type": "string"
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "name": "keys", | |
| "description": "Key combinations to press, in order (e.g. [\"cmd+a\", \"backspace\"])", | |
| "required": true, | |
| "schema": { | |
| "type": "array", | |
| "items": { | |
| "type": "string" | |
| } | |
| } | |
| } | |
| { | |
| "name": "keys", | |
| "description": "Key combinations to press, in order (e.g. [\"cmd+a\", \"backspace\"])", | |
| "required": true, | |
| "schema": { | |
| "type": "array", | |
| "minItems": 1, | |
| "items": { | |
| "type": "string" | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/openrpc.json` around lines 287 - 297, The "keys" parameter schema
currently allows an empty array even though the description requires "one or
more key combinations"; update the "keys" property's JSON schema (the object
with "name": "keys") to include "minItems": 1 under its "schema" object so the
OpenRPC contract enforces at least one item is provided.
No description provided.