-
-
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 support for user home variable in settings #1916
Conversation
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.
Mostly looks good, with minor comment; see also my commit for a couple changes I made
@@ -42,3 +46,20 @@ export default class VscodeConfiguration implements Configuration { | |||
|
|||
onDidChangeConfiguration = this.notifier.registerListener; | |||
} | |||
|
|||
export function vscodeGetConfigurationString(path: string): string | undefined { | |||
const value = vscode.workspace.getConfiguration().get<string>(path); |
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.
This loads the whole configuration, right? Before we just loaded the section we cared about. Probably prematurely optimising by caring about this stuff, but I'm assuming there's a reason VSCode supports retrieving just one section?
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.
I'm not really sure why vscode has this section separation. If you like I can split on the last dot and just fetch the section first?
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.
Seems reasonable for now
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.
done
@pokey poke! |
eg `"cursorless.private.hatShapesDir": "C:\\Users\\Andreas\\repositories\\pokey-dotfiles\\cursorless-hat-shapes",` ## Checklist - [/] 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: Pokey Rule <[email protected]> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
eg
"cursorless.private.hatShapesDir": "C:\\Users\\Andreas\\repositories\\pokey-dotfiles\\cursorless-hat-shapes",
Checklist