-
-
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
Added voice command to migrate Cursorless snippet to community format #2747
Conversation
…eSnippetCommunity.ts
packages/cursorless-org-docs/src/docs/user/experimental/snippets.md
Outdated
Show resolved
Hide resolved
} | ||
|
||
async function migrateFile(targetDirectory: string, filePath: string) { | ||
const fileName = path.basename(filePath, ".cursorless-snippets"); |
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.
CURSORLESS_SNIPPETS_SUFFIX?
|
||
async function writeCommunityFile(snippetFile: SnippetFile, filePath: string) { | ||
const snippetText = serializeSnippetFile(snippetFile); | ||
const file = await fs.open(filePath, "w"); |
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.
Checking for file existence separately from opening the file is generally racy, best to just do this:
const file = await fs.open(filePath, "w"); | |
const file = await fs.open(filePath, "wx"); |
'wx': Like 'w' but fails if the path exists.
Then catch and put your conflict filename behavior in the catch block.
…s into migrateSnippets
…#2747) 2 fields in the Cursorless snippets are not available in the community format 1. scopeTypes. To my knowledge the only use case is omitting the `function` keyword when in a javascript class 2. excludeDescendantScopeTypes: Used in conjunction with `scopeTypes` to include the function keyword when in a function inside a class. scopeTypes and excludeDescendantScopeTypes I'm not to sure we want to add to community. First of all they have a single use case and needing two separate fields just to describe this behavior fields a bit verbose. As well community doesn't have support for sending multiple snippets. I'm planning this as a follow up. Reference https://github.com/cursorless-dev/cursorless/blob/57acd6c0fcc32e5df122ce09bb26bde5f09495ca/packages/common/src/types/snippet.types.ts#L4-L24 Fixes #2355 ## Checklist - [ ] 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) - [/] 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: Phil Cohen <phillip@phillip.io>
What does this tool do with the |
Discards them. These fields are not really used that much (There is only one use case as far as I know). And since we haven't decided on the format we are going to use in the future I want to avoid a community to community migration script if we decide to use something else. |
I would bail if you encounter them. Silently dropping data is not good ux |
You mean bail on the entire migration or just on that snippet? |
Maybe just that snippet? I don't feel strongly, as long as we raise a flag rather than silently losing data |
Totally agree. Here is the follow up. |
2 fields in the Cursorless snippets are not available in the community format
function
keyword when in a javascript classscopeTypes
to include the function keyword when in a function inside a class.scopeTypes and excludeDescendantScopeTypes I'm not to sure we want to add to community. First of all they have a single use case and needing two separate fields just to describe this behavior fields a bit verbose.
As well community doesn't have support for sending multiple snippets.
I'm planning this as a follow up.
Reference
cursorless/packages/common/src/types/snippet.types.ts
Lines 4 to 24 in 57acd6c
Fixes #2355
Checklist