-
-
Notifications
You must be signed in to change notification settings - Fork 84
Added generation of spoken form #1671
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
Conversation
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.
partial review
packages/common/src/types/command/PartialTargetDescriptor.types.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/upgradeV1ToV2/upgradeV1ToV2.ts
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/upgradeV2ToV3/upgradeV2ToV3.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/upgradeV4ToV5/upgradeV4ToV5.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/canonicalizeSpokenFormTestCommand.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/canonicalizeSpokenFormTestCommand.ts
Outdated
Show resolved
Hide resolved
@pokey Updated wit requested changes from our meetup |
@AndreasArvidsson did we want to use the spoken form generator for recording test cases in this PR? Tbh, I think I'd almost prefer if update fixtures launch config updated spoken forms. Makes me a bit less nervous than just automatically generating new test cases for the spoken form generator that are based on its behaviour at that moment and requiring us to carefully review them. But I'd lean towards having one or the other in this PR, as otherwise it's going to be pretty inconvenient the next time someone records a test |
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/core/commandVersionUpgrades/generateSpokenForm.ts
Outdated
Show resolved
Hide resolved
3d29789
to
cd498dc
Compare
packages/cursorless-engine/src/generateSpokenForm/defaultSpokenForms/characters.ts
Show resolved
Hide resolved
packages/cursorless-engine/src/generateSpokenForm/defaultSpokenForms/actions.ts
Show resolved
Hide resolved
cd2749c
to
f0092fa
Compare
if (destination.destinations.length < 2) { | ||
throw new NoSpokenFormError("List destination with < 2 elements"); | ||
} |
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.
No way to speak this one
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.
Same here. I would argue that this should be a normal error.
f0092fa
to
ef56dae
Compare
77d2682
to
c287af6
Compare
packages/cursorless-engine/src/generateSpokenForm/generateSpokenForm.test.ts
Show resolved
Hide resolved
packages/cursorless-engine/src/generateSpokenForm/NoSpokenFormError.ts
Outdated
Show resolved
Hide resolved
9af3d4f
to
5321898
Compare
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.
I'm happy, but @AndreasArvidsson have a look through my comments to make sure you're happy with my changes
aggh looks like it's time to fix that E2BIG thing. wrote up a fix; going to test that it works here then cherry pick it to new PR |
b5c3837
to
1e97077
Compare
2eb2572
to
ee8cbbd
Compare
Checklist