Bulk scope test recorder #2383
Bulk scope test recorder #2383
Conversation
|
@pokey Scope facet description is now added |
|
Any idea how we would go about recording tests for eg |
That was more or less what I was thinking |
|
yeah probably fine for now. I don't see any easy way to do something smarter, and I believe for everything other than ts/js/tsx/jsx you could get away with just recording the test for the base language. Eg for css/scss you'd just add the support info to css and record test for css and it would just work |
|
Pretty much. |
There was a problem hiding this comment.
Ok I made some tweaks and left a couple comments. My changes were as follows:
- Add spoken forms to our dev talon file
- Use a snippet so that you can just say "next" to go from one facet to the next
- Set language id to markdown, as the highlighting is not bad and it prevents VSCode from trying to guess a language
- Fix the regex for faced ids that have a
.in them, egbranch.if
| @@ -29,7 +29,7 @@ import { yamlScopeSupport } from "./yaml"; | |||
|
|
|||
| export const languageScopeSupport: Record< | |||
| string, | |||
| LanguageScopeSupportFacetMap | |||
| LanguageScopeSupportFacetMap | undefined | |||
There was a problem hiding this comment.
The key is a string so I think this is much clearer that you can actually get back undefined. I actually think the implementation of record in typescript is a bit problematic where it does not have the key value map expectation where the key can be missing.
There was a problem hiding this comment.
If the key is a literal union then TypeScript will enforce exhaustiveness of the record.
There was a problem hiding this comment.
In this case the key type is string. If I do languageScopeSupport["hello"] I will get back undefined and I think that the type system should reflect this.
There was a problem hiding this comment.
Yeah this one has bitten me before. I'd be tempted to have a utility type
type StringRecord<T> = Partial<Record<string, T>>
There was a problem hiding this comment.
Should I implement this here?
There was a problem hiding this comment.
Yeah I think so. I don't think it's significant enough to warrant its own PR
|
I'm happy with those changes |
Figured while I was testing #2383 I might as well do something useful at the same time 😊 ## Checklist - [x] 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
scopeSupportFacets\typescript.tssupportedfacets for the language that are missing.scopefilesThoughts:
Checklist