feat: customize paste snippet in vscode#2278
Conversation
Summary of ChangesHello @Myriad-Dreamin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the paste functionality within the VS Code extension by introducing a mechanism for users to define custom text snippets. This allows for dynamic and context-aware formatting of pasted content, such as images, based on the current editing mode. The changes involve modifying the backend's path evaluation to return a richer data structure that includes these snippet definitions, and updating the frontend to consume and apply them. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for customizing paste snippets in VSCode, which enhances user workflow. The changes are well-implemented across both the Rust backend and the TypeScript frontend.
On the backend, the introduction of PathAtOutput and the modification of eval_path_expr to handle more complex return types are solid. I've suggested a small refactoring to simplify a chain of and_then calls for better readability.
On the frontend, the logic in getDesiredNewFilePath is correctly updated to parse the new response structure. I've proposed a simplification of the validation logic to make it more concise.
I also noticed a new test case (snippet) has been added but is currently unimplemented (todo!()). It's important to either implement this test to ensure the new functionality is covered or remove the placeholder before merging.
Overall, this is a great enhancement. Addressing these points will further improve the code's quality and ensure the feature is robust.
| let res = eval_path_expr(ctx, &code, base) | ||
| .and_then(|res| { | ||
| QueryResult::success(res.map(|mut res| { | ||
| res.edits = resolve_edits(ctx, res.edits); | ||
| res | ||
| })) | ||
| }) | ||
| .and_then(|res| match serde_json::to_value(res) { | ||
| Ok(value) => QueryResult::success(value), | ||
| Err(e) => QueryResult::error(eco_format!( | ||
| "failed to serialize script result: {e}" | ||
| )), | ||
| }); |
There was a problem hiding this comment.
This chain of and_then calls can be simplified for better readability. You can combine the logic into a single and_then block, which avoids nesting and makes the data flow clearer.
let res = eval_path_expr(ctx, &code, base).and_then(|res| {
let res = res.map(|mut res| {
res.edits = resolve_edits(ctx, res.edits);
res
});
match serde_json::to_value(res) {
Ok(value) => QueryResult::success(value),
Err(e) => QueryResult::error(eco_format!(
"failed to serialize script result: {e}"
)),
}
});| #[test] | ||
| fn snippet() { | ||
| // type ModeEdit = string | { | ||
| // kind: "by-mode"; | ||
| // math?: string; | ||
| // markup?: string; | ||
| // code?: string; | ||
| // comment?: string; | ||
| // string?: string; | ||
| // raw?: string; | ||
| // rest?: string; | ||
| // }; | ||
|
|
||
| // export async function createModeEdit(edit: ModeEdit) { | ||
| // const { | ||
| // kind, | ||
| // math, | ||
| // comment, | ||
| // markup, | ||
| // code, | ||
| // string: stringContent, | ||
| // raw, | ||
| // rest, | ||
| // }: Record<string, string> = edit.newText; | ||
| // const newText = kind === "by-mode" ? rest || "" : ""; | ||
|
|
||
| // const res = await vscode.commands.executeCommand< | ||
| // [{ mode: "math" | "markup" | "code" | "comment" | "string" | | ||
| // "raw" }] >("tinymist.interactCodeContext", { | ||
| // textDocument: { | ||
| // uri: activeDocument.uri.toString(), | ||
| // }, | ||
| // query: [ | ||
| // { | ||
| // kind: "modeAt", | ||
| // position: { | ||
| // line: selectionStart.line, | ||
| // character: selectionStart.character, | ||
| // }, | ||
| // }, | ||
| // ], | ||
| // }); | ||
|
|
||
| // const mode = res[0].mode; | ||
|
|
||
| // await editor.edit((editBuilder) => { | ||
| // if (mode === "math") { | ||
| // // todo: whether to keep stupid | ||
| // // if it is before an identifier character, then add a space | ||
| // let replaceText = math || newText; | ||
| // const range = new vscode.Range( | ||
| // selectionStart.with(undefined, selectionStart.character - 1), | ||
| // selectionStart, | ||
| // ); | ||
| // const before = selectionStart.character > 0 ? | ||
| // activeDocument.getText(range) : ""; if (before.match(/ | ||
| // [\p{XID_Start}\p{XID_Continue}_]/u)) { replaceText = | ||
| // ` ${math}`; } | ||
|
|
||
| // editBuilder.replace(selection, replaceText); | ||
| // } else if (mode === "markup") { | ||
| // editBuilder.replace(selection, markup || newText); | ||
| // } else if (mode === "comment") { | ||
| // editBuilder.replace(selection, comment || markup || newText); | ||
| // } else if (mode === "string") { | ||
| // editBuilder.replace(selection, stringContent || raw || | ||
| // newText); } else if (mode === "raw") { | ||
| // editBuilder.replace(selection, raw || stringContent || | ||
| // newText); } else if (mode === "code") { | ||
| // editBuilder.replace(selection, code || newText); | ||
| // } else { | ||
| // editBuilder.replace(selection, newText); | ||
| // } | ||
| // }); | ||
| // } | ||
|
|
||
| todo!() | ||
| } |
There was a problem hiding this comment.
This test case is currently unimplemented and ends with todo!(). Please implement the test to cover the new snippet functionality or remove the placeholder before merging.
| if (typeof dir !== "string") { | ||
| if (typeof dir !== 'object') { | ||
| throw new Error( | ||
| `expect paste script to return an object { value: string | object }, got { value: ${JSON.stringify(dir)} }`, | ||
| ); | ||
| } else { | ||
| if (!("dir" in dir)) { | ||
| throw new Error( | ||
| `expect paste script to return an object { value: { dir: string } }, got { value: ${JSON.stringify(dir)} }`, | ||
| ); | ||
| } | ||
| dir = dir.dir; | ||
| } | ||
| } |
There was a problem hiding this comment.
The validation logic for the pathAt.value can be simplified to be less nested and more direct. This improves readability by reducing the indentation levels and combining the checks.
if (typeof dir !== "string") {
if (typeof dir !== "object" || dir === null || !("dir" in dir) || typeof (dir as any).dir !== "string") {
throw new Error(
`expect paste script to return a string or an object with a 'dir' string property, got { value: ${JSON.stringify(
dir,
)} }`,
);
}
dir = (dir as { dir: string }).dir;
}
|
Could it, just like with #2238 have custom scripting to create your own "resolver"? I.e. based on custom behavior, we can have it become |
2 participants
Add a field
editsto control the snippet used for generating pasted text.Example (default behavior when pasting image):
( edits: ( kind: "by-mode", math: `#image("image.png")`.text, markup: `#image("image.png")`.text, code: `image("image.png")`.text, string: "image.png", comment: "", raw: "", ) )