Conversation
pokey
left a comment
There was a problem hiding this comment.
we should prob have api test for this, no?
Extension side we actually have multiple tests for this already |
|
Yes but I mean the talon-side api introduced by this PR. We have tests for our other talon-side APIs |
|
True. Test added. |
Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
pokey
left a comment
There was a problem hiding this comment.
made some changes:
- Stopped using the public api for our internal impl. I'm not in love with coupling those
- Switched
no_decorationstosilent. Double-negatives confuse me, so I prefer to avoid them if possible - I made all args to our function explicit. If we have the decorations flag reversed between talon api and cursorless api, the only way to make heads or tails I think is to just be super explicit and pass them every time
Feel free to merge if you're happy
|
I'm not in love with the argument |
I agree. But I also really don't like double negatives (ie
cc/ @josharian |
|
|
Ok made some final tweaks and confirmed talon tests work locally. I'll give you a chance for a quick look in case I did anything dumb; merge if happy |
packages/cursorless-engine/src/test/fixtures/talonApi.fixture.ts
Outdated
Show resolved
Hide resolved
Fixes #452 ## Checklist - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] 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) - [x] I have not broken the cheatsheet --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
Fixes cursorless-dev#452 ## Checklist - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] 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) - [x] I have not broken the cheatsheet --------- Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
Fixes #452
Checklist