Skip to content

Support array-based key bindings (execute multiple commands) - #235

Open
Yaruani wants to merge 2 commits into
wshanks:mainfrom
Yaruani:main
Open

Support array-based key bindings (execute multiple commands)#235
Yaruani wants to merge 2 commits into
wshanks:mainfrom
Yaruani:main

Conversation

@Yaruani

@Yaruani Yaruani commented Jan 14, 2026

Copy link
Copy Markdown

Hi, thanks for the great addon.

I found that using an array for key bindings (e.g., ["cmd:A", "cmd:B"]) was causing errors due to schema validation and implementation logic.

I have updated implementation.js and schema.json to support array-based commands properly.
This allows users to execute multiple commands with a single key press (e.g., Mark as Read -> Archive).

Please review the changes. Thanks!

Changed the "Keys" type definition from "string" to "any" to support array-based command sequences and prevent validation errors.
Modified buildKeyCommand to handle both string and array inputs. If an array is provided, it iterates through and executes each command in sequence.

@wshanks wshanks left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks nice. Can you also add something to the "Command syntax" section of the readme about using arrays of strings instead of single strings?

Comment thread addon/implementation.js
// This function is based on the default callback in Mousetrap but is extended
// to include more text input fields that are specific to Thunderbird.
// Additionally, it does not ignore text fields if the first key includes
// modifiers other than shift.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment needed to be removed?

Comment thread addon/implementation.js
Comment on lines +96 to +97
// command should be a string formatted as type:body OR an array of strings.
// Modified to support multiple commands in an array.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep the description of the string formatting (line wrapping might not be right for this suggestion):

Suggested change
// command should be a string formatted as type:body OR an array of strings.
// Modified to support multiple commands in an array.
// command should be a string or arrays of strings with
// each string formatted as type:body where type is cmd, func,
// tbkeys, unset, or eval and body is the type-specific content of the command

Comment thread addon/implementation.js
eval(command);
break;
// Internal helper to execute a single command string
const runSingleCommand = (cmdStr) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pull this definition outside of buildKeyCommand?

Comment thread addon/implementation.js
//
// keyBindings has the structure:
// {windowType: {keySequence: keyCommand}}
// {windowType: {keySequence: keyCommand}}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe undo this change unless there was a reason for it?

Comment thread addon/schema.json
"type": "object",
"description": "Mapping of key sequences to commands",
"additionalProperties": { "type": "string" }
"additionalProperties": { "type": "any" }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going from allowing strings to allow arrays of strings. Can we keep the schema more restricted to those cases instead of using any?

Suggested change
"additionalProperties": { "type": "any" }
"additionalProperties": { "type": ["array", "string"], "items": {"type": "string"} }

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.

2 participants