-
-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make cheat sheet handle missing spoken forms #2760
Conversation
if "replaceWithTarget" in complex_actions: | ||
items.append( | ||
{ | ||
"id": "replaceWithTarget", | ||
"type": "action", | ||
"variations": [ | ||
{ | ||
"spokenForm": f"{complex_actions['replaceWithTarget']} <target> <destination>", | ||
"description": "Copy <target> to <destination>", | ||
}, | ||
{ | ||
"spokenForm": f"{complex_actions['replaceWithTarget']} <target>", | ||
"description": "Insert copy of <target> at cursor", | ||
}, | ||
], | ||
} | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bit of duplication here and this switches from defining a data structure to a fair bit of imperative code, but I think we could reduce it.
Why not something like a dictionary of tuples (one description and one lambda that generates the spoken form?):
{
"replaceWithTarget": ("Copy <target> to <destination>", lambda value: f"{value} <target> <destination>"),
}
Then we could just use a loop to generate all of these and remove some the duplication.
Not sure if you could refer to stuff outside of the scope like swap_connective
but I think it's helpful either way, if not that means the more complicated ones will stay as imperative code, which I think itself helps with readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm let's do the same magic on the others
Today the cheat sheet generation crashes if the user has disabled certain spoken forms Fixes #2647 ## Checklist - [/] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [/] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [/] I have not broken the cheatsheet
Today the cheat sheet generation crashes if the user has disabled certain spoken forms
Fixes #2647
Checklist