Skip to content

Conversation

@browntarik
Copy link
Contributor

@browntarik browntarik commented Mar 6, 2024

Addresses: #11756

@Colengms
Copy link
Contributor

Colengms commented Mar 6, 2024

Are there other variables we should add support for while we're in there? Of the ones on https://code.visualstudio.com/docs/editor/variables-reference, the following also seem supportable in this context:

${workspaceFolderBasename} - the name of the folder opened in VS Code without any slashes (/)
${execPath} - the path to the running VS Code executable
${pathSeparator} - the character used by the operating system to separate components in file paths
${/} - shorthand for ${pathSeparator}

@bobbrow
Copy link
Member

bobbrow commented Mar 6, 2024

Are there other variables we should add support for while we're in there? Of the ones on https://code.visualstudio.com/docs/editor/variables-reference, the following also seem supportable in this context:

${workspaceFolderBasename} - the name of the folder opened in VS Code without any slashes (/) ${execPath} - the path to the running VS Code executable ${pathSeparator} - the character used by the operating system to separate components in file paths ${/} - shorthand for ${pathSeparator}

We actually have support for the few of these already. I don't think the proposed change is the "best" approach. Please see:

private get ExtendedEnvironment()

in configurations.ts (around line 421). This is where we dealt with vs code's variables (except for workspaceFolder which is special).

@sean-mcmanus
Copy link
Contributor

Are there other variables we should add support for while we're in there? Of the ones on https://code.visualstudio.com/docs/editor/variables-reference, the following also seem supportable in this context:

${workspaceFolderBasename} - the name of the folder opened in VS Code without any slashes (/) ${execPath} - the path to the running VS Code executable ${pathSeparator} - the character used by the operating system to separate components in file paths ${/} - shorthand for ${pathSeparator}

I think that's beyond the scope of this particular change? i.e. I would think those could/should be fixed separately unless it's trivial?

@bobbrow
Copy link
Member

bobbrow commented Mar 6, 2024

Are there other variables we should add support for while we're in there? Of the ones on https://code.visualstudio.com/docs/editor/variables-reference, the following also seem supportable in this context:
${workspaceFolderBasename} - the name of the folder opened in VS Code without any slashes (/) ${execPath} - the path to the running VS Code executable ${pathSeparator} - the character used by the operating system to separate components in file paths ${/} - shorthand for ${pathSeparator}

I think that's beyond the scope of this particular change? i.e. I would think those could/should be fixed separately unless it's trivial?

They are all implemented already with the exception of ${/}. It's trivial to add support for variables, so it can be done now if we want.

@browntarik browntarik merged commit 699b3d5 into main Mar 7, 2024
@browntarik browntarik deleted the browntarik/addUserHome branch March 7, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants