Implement Air: Initialize Workspace Folder
#390
Conversation
Also make some helpers usable elsewhere
| export function workspaceFolderFormatting(ctx: Ctx): Cmd { | ||
| return async () => { |
There was a problem hiding this comment.
There are no actual changes to this function, I've just de-dented it one level since it no longer returns a callback
| * This seems to be the only way to update language specific settings for a | ||
| * particular workspace folder in a way that: | ||
| * | ||
| * - Doesn't destroy existing `[r]` or `[quarto]` settings that we aren't | ||
| * updating (like `config.update("[r]", value)` naively does) | ||
| * - Doesn't pull extra inherited global settings (like `config.get()` would do) | ||
| * | ||
| * The trick is to `inspect()` all of the `[r]` configuration specific to just | ||
| * the `workspaceFolder` and bulk update it, retaining all old settings but | ||
| * overriding the ones we care about updating. | ||
| * | ||
| * This does unfortunately drop comments in the `[r]` and `[quarto]` sections, | ||
| * but we can't do better. | ||
| * | ||
| * It would be great if you could do `update("[r].editor.formatOnSave")` to | ||
| * precisely target a single value of a single language, but you cannot. | ||
| */ | ||
| async function updateSettingsJson( | ||
| workspaceFolder: vscode.WorkspaceFolder, | ||
| ): Promise<void> { | ||
| const config = vscode.workspace.getConfiguration( | ||
| undefined, | ||
| workspaceFolder, | ||
| ); | ||
|
|
||
| const configR = config.inspect("[r]")?.workspaceFolderValue || {}; | ||
| await config.update( | ||
| "[r]", | ||
| { | ||
| ...configR, | ||
| "editor.formatOnSave": true, | ||
| "editor.defaultFormatter": "Posit.air-vscode", | ||
| }, | ||
| vscode.ConfigurationTarget.WorkspaceFolder, | ||
| ); |
There was a problem hiding this comment.
Wonky config updating scheme, I did check and other people also do it like this
For example, the eslint extension does it this way, and that's maintained by the main LSP guy
https://github.com/microsoft/vscode-eslint/blob/001edda5bd168a9c6a79cba767111ba21eadd052/client/src/settings.ts#L330
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| const message = `Successfully initialized the ${workspaceFolder.name} workspace folder.`; | ||
| const shouldFormatItem = `Click to format the ${workspaceFolder.name} workspace folder`; | ||
|
|
||
| const item = await vscode.window.showInformationMessage( |
There was a problem hiding this comment.
So this resolves only when the toast notif is discarded one way or another?
There was a problem hiding this comment.
It actually goes away on its own after about 20 seconds of waiting around
| * This does unfortunately drop comments in the `[r]` and `[quarto]` sections, | ||
| * but we can't do better. |
There was a problem hiding this comment.
I'm assuming that modifying the json via the workspace settings UI would also drop the comments? So it's more like settings comments are not supported in vscode/positron at all.
There was a problem hiding this comment.
oh hmm, just tried it and they are preserved.
There was a problem hiding this comment.
Looks like the editing UI uses jsonc-parser (https://github.com/microsoft/node-jsonc-parser) whereas the extension-facing API is plain json 🤔
Probably because plain json is easier to use.
There was a problem hiding this comment.
The fact that they use different parsers is so bonkers!
There was a problem hiding this comment.
Core loop looks like
private async doWriteConfiguration(path: JSONPath, value: any): Promise<void> {
let content: string;
try {
const fileContent = await this.fileService.readFile(this.settingsResource);
content = fileContent.value.toString();
} catch (error) {
if ((<FileOperationError>error).fileOperationResult === FileOperationResult.FILE_NOT_FOUND) {
content = '{}';
} else {
throw error;
}
}
const parseErrors: ParseError[] = [];
parse(content, parseErrors, { allowTrailingComma: true, allowEmptyContent: true });
if (parseErrors.length > 0) {
throw new Error('Unable to write into the settings file. Please open the file to correct errors/warnings in the file and try again.');
}
const edits = this.getEdits(content, path, value);
content = applyEdits(content, edits);
await this.fileService.writeFile(this.settingsResource, VSBuffer.fromString(content));
}
Closes #323
This is a Positron and VS Code specific complement to
usethis::use_air(), this way our setup recommendation for Positron doesn't need to involve R at all, it's just:Air: Initialize Workspace FolderAir: Format Workspace Folder(or click the button during initialization)A few quirks:
settings.json, but no API to updateextensions.json. Theextensions.jsonfile is less critical, so I've taken an all or nothing approach where we just don't even try to do anything if the user already has one. I think it would even be more complex than I'd want to try and parse the existing one and see if we are recommended or not, and if not, prompt the user with what they could add. That all just doesn't seem worth it (and toast notifications aren't a great UI for that kind of prompt IMO).settings.jsonupdate UI is a bit wonky for this. There isn't a great API combination for "I know the workspace folder, and now I'm trying to tweak just one language specific setting". Instead you have to replace all of[r]in bulk. You'll see some comments in there about how we have to do this.furrr-slider-setup-video.mov