Generate snippets for community format#2744
Conversation
| actions.user.private_cursorless_command_no_wait( | ||
| { | ||
| "name": "generateSnippet", | ||
| "target": target, |
There was a problem hiding this comment.
By default don't send dirPath argument. This is for the legacy Cursorless snippet format. The vscode setting for snippet directory will be used.
| { | ||
| "name": "generateSnippet", | ||
| "target": target, | ||
| "dirPath": get_dir_path(), |
There was a problem hiding this comment.
If the community snippet tag has been set send dirPath argument. A new community snippet will be added to this folder.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const metaSnippetText = serializeSnippetFile(snippetDocuments) | ||
| // Escape dollar signs in the snippet text so that they don't get used as | ||
| // placeholders in the meta snippet | ||
| .replace(/\$/g, "\\$") |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI over 1 year ago
To fix the problem, we need to ensure that backslashes are properly escaped in the metaSnippetText string. This can be done by adding a replace operation to escape backslashes before any other replacements are made. Specifically, we should replace each backslash (\) with a double backslash (\\). This ensures that any backslashes in the input are correctly handled and do not lead to injection vulnerabilities or malformed strings.
| @@ -187,2 +187,4 @@ | ||
| const metaSnippetText = serializeSnippetFile(snippetDocuments) | ||
| // Escape backslashes in the snippet text | ||
| .replace(/\\/g, "\\\\") | ||
| // Escape dollar signs in the snippet text so that they don't get used as |
| @@ -0,0 +1,55 @@ | |||
| languageId: typescript | |||
There was a problem hiding this comment.
Double up on existing snippet make tests in the community format
| spokenForm: snippet make funk | ||
| action: | ||
| name: generateSnippet | ||
| dirPath: "" |
There was a problem hiding this comment.
Including the dirPath argument indicates that this is a community snippet.
| user_dir = Path(actions.path.talon_user()) | ||
| dir = user_dir / dir | ||
|
|
||
| return str(dir.resolve()) |
There was a problem hiding this comment.
I'd make all of these return Path and only coalesce to a string when you need to stick it into the JSON dictionary
| return get_community_snippets_dir() | ||
|
|
||
|
|
||
| def get_setting_dir() -> Path | None: |
There was a problem hiding this comment.
TIL:
- This syntax is now preferred
- Public uses 3.11 so we can use it
- Talon didn't used to support it in action definitions but does in beta: TalonScript TypeError when using
X | Y-style union types for action parameter type hints talonvoice/talon#634
We can't currently use it in action definitions until v0.5 releases but we can use it in code like this
…eSnippetCommunity.ts
| // Insert the meta-snippet | ||
| await editableEditor.insertSnippet(snippetText); |
There was a problem hiding this comment.
NB: this was technically a bug and relying on odd Visual Studio Code behavior because we aren't using the new editor that was created inside of this.snippets.openNewSnippetFile(snippetName);. It just so happens that insertSnippet always uses the current editor even if you call it on a different one. The new code fixes this by making the function return a promise with the actual new editor which we needed for appending.
There was a problem hiding this comment.
NB: Andreas fixed this because we now want to append the text to the file if it already exists, and reading the text from an editor that's not the current one does work, and that's when he found that the editor object was actually technically wrong
| run(targets: Target[], snippetName?: string): Promise<ActionReturnValue>; | ||
| run( | ||
| targets: Target[], | ||
| dirPath?: string, |
There was a problem hiding this comment.
Not thrilled about dirPath but it makes sense after discussion -- we sometimes have the directory name dirName so it's needed to disambiguate.
If we had a Path type of class it would be nicer so we didn't have to put the type information in the name itself, but we don't. We could make a wrapper ourselves or just do two type aliases. We might make a followup for this.
storageDirectory would be my choice for the name but not if it has to be storageDirectoryPath, I think that's worse than the status quo
If the user has set the `user.cursorless_use_community_snippets` tag we create a snippet in the community snippet format instead of the Cursorless one. The legacy Cursorless snippets will be removed in a final pull request. ## 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 --------- 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>
3 participants
If the user has set the
user.cursorless_use_community_snippetstag we create a snippet in the community snippet format instead of the Cursorless one.The legacy Cursorless snippets will be removed in a final pull request.
Checklist