complete support of variable substitution

[akosyakov]

What it does

• fix support vscode variable substitution #3434 - complete support of vscode variable substitution
  • only input variables for tasks are left out, since it depends on tasks configs as preferences #5013 and requires substantial refactoring of the task extension first
• fixed resolution of env variables, see fa3016e
• a CQ for copied input schema and types: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20504

How to test

• look at https://code.visualstudio.com/docs/editor/variables-reference and test different variables
• you can also follow commit history to see support for which variables were added
• for most variables, you can use Variables: List All command
  • then select a variable to test
  • then you will be prompted to provide an argument for a variable, an argument is a part of ':' in variable substitution expression
  • if a variable does not accept argument just press enter, if it accepts then provide a proper argument
  • you should see a variable value if it can be computed in the given context, or raw expression if it cannot
• alternatively, you can define variables in tasks.json and launch.json and then try to run a task or launch configuration
  • like defined here for example, there is a bug though that an output for short-lived task is lost, you may want to add watch echo in order to keep a task running
• for command variables you may want to install vscode node js extensions instead of native and test with extension.pickNodeProcess, don't forget that you need both vscode nodejs extension to use them
• for input variables you can follow: https://code.visualstudio.com/docs/editor/variables-reference#_input-variables
  • tasks.json is not supported, since the task extension is not ready for it, see support input variable substitution for tasks #5836

Review checklist

• as an author, I have thoroughly tested my changes and carefully reviewed following the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[kittaakos]

I am verifying it.

[kittaakos]

What should happen when a variable is not available from the shell?
For instance, when I am usign "${env:PATH}" then I can see value of the PATH variable is printed. If I use "${env:USERNAME}" it is printed as ${env:USERNAME} instead of an empty string.

Akoss-MacBook-Pro:theia akos.kitta$ echo $USERNAME

Akoss-MacBook-Pro:theia akos.kitta$ 

[kittaakos]

[bug]: When defining a variable, select Variable: List All > ${env} > Esc:

[akosyakov]

If I use "${env:USERNAME}" it is printed as ${env:USERNAME} instead of an empty string

That's a great point! It should be an empty string for sure.

[kittaakos]

I have verified the followinngs I could understand from the description; they work.

Verified variables:

"${workspaceFolder}",
"${workspaceFolderBasename}",
"${file}",
"${relativeFile}",
"${fileBasename}",
"${fileBasenameNoExtension}",
"${fileDirname}",
"${fileExtname}",
"${cwd}",
"${lineNumber}",
"${selectedText}",
"${execPath}",

Executed module:

console.log('ARGS:');
for (const e of process.argv.splice(2)) {
    console.log(e);
}

Running ./xxx/index.js:

ARGS:
/Users/akos.kitta/git/theia
theia
/Users/akos.kitta/git/theia/xxx/index.js
xxx/index.js
index.js
index
/Users/akos.kitta/git/theia/xxx
.js
/Users/akos.kitta/git/theia
2
process.argv.splice(2)
/Users/akos.kitta/.nvm/versions/node/v10.15.3/bin/node

Running ./index.js:

ARGS:
/Users/akos.kitta/git/theia
theia
/Users/akos.kitta/git/theia/index.js
index.js
index.js
index
/Users/akos.kitta/git/theia
.js
/Users/akos.kitta/git/theia
2
process.argv.splice(2)
/Users/akos.kitta/.nvm/versions/node/v10.15.3/bin/node

I did no verify the case sensitivity/insensitivity:

Note: Be sure to match the environment variable name's casing, for example ${env:Path} on Windows.

[akosyakov]

I did no verify the case sensitivity/insensitivity:

I've noticed that VS Code always convert env var name to lower case on windows: https://github.com/microsoft/vscode/blob/0a34756cae4fc67739e60c708b04637089f8bb0d/src/vs/workbench/services/configurationResolver/common/variableResolver.ts#L172-L175 Should we do the same?

[kittaakos]

What I have tested, worked as expected. Please note the following items:

• Cannot cancel the quick pick when setting variables: complete support of variable substitution #5835 (comment)
• Unavailable variables should be empty strings: complete support of variable substitution #5835 (comment)

[kittaakos]

Should go to the breaking changes section.

[akosyakov]

ok

[akosyakov]

Unavailable variables should be empty strings

I don't think it is a case for all variables, just for env because they are evaluated to empty string when missing. In other cases it is not true. Maybe we should throw in other cases to prevent running tasks and launch configuration with bogus arguments.

[akosyakov]

I will wait till CQ is resolved before continuing working on it. It is pending already for 5 days and they don't seem to work on it: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20504#c3 cc @marcdumais-work

[marcdumais-work]

It is pending already for 5 days and they don't seem to work on it

Yes, I noticed things seem slow at the Foundation recently; I suspect late summer vacations.

[vince-fugnitto]

I will wait till CQ is resolved before continuing working on it. It is pending already for 5 days and they don't seem to work on it: https://dev.eclipse.org/ipzilla/show_bug.cgi?id=20504#c3 cc @marcdumais-work

I believe the CQ is waiting for some form of feedback from you @akosyakov :( I see the message "IPTeam awaiting response from Committer."

[marcdumais-work]

I believe the CQ is waiting for some form of feedback from you @akosyakov :( I see the message "IPTeam awaiting response from Committer."

Thanks for pointing this out, Vince. I answered the question and sent the CQ back to the IP team.

[azatsarynnyy]

tested several random variables through the Variables: List All command
works as expected

[akosyakov]

@kittaakos I've pushed a commit to fix resolving env variables values on windows and evaluating them to an empty string if they are missing to align with the shell behaviour. I'm going to merge it tomorrow if no one objects and CI is green.