Add capability to support current file path for notebook root #7724
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7724 +/- ##
==========================================
+ Coverage 58.72% 58.81% +0.08%
==========================================
Files 494 496 +2
Lines 22027 22115 +88
Branches 3541 3558 +17
==========================================
+ Hits 12935 13006 +71
- Misses 8281 8297 +16
- Partials 811 812 +1
Continue to review full report at Codecov.
|
| break; | ||
|
|
||
| case '${cwd}': | ||
| return process.cwd(); |
There was a problem hiding this comment.
This might not be correct.
The execution context in vscode is vscode extension or vscode.
However here the execution context is jupyter or python and cwd is something completely different.
There was a problem hiding this comment.
Today we do something very similar in the src/client/common/variables/systemVariables.ts file.
Can I suggest we modify SystemVariables class to optionally accept a new argument file, this way we have everything we need in there. We can then create properties that match the new variables and it will work with other variables as well.
E.g. there are some variables that are missing, such as ${workspaceFolderBasename} and a few others (see the class).
There was a problem hiding this comment.
This way the extension also gets this new functionality.
FYI -- this has been requested in extension issues.
See here #2341 (I believe we have a few others as well, some old ones).
There was a problem hiding this comment.
There was a problem hiding this comment.
According to this, cwd is kind of ambiguous. It says this: ${cwd} - the task runner's current working directory on startup. I would think that would be the directory you started VS code from. Not the workspace folder.
In reply to: 330342914 [](ancestors = 330342914)
There was a problem hiding this comment.
Possibly something that could then be controlled via a ctor argument.
I guess that would also solve https://github.com/microsoft/vscode-python/issues/3612
| return this.changeDirectoryIfPossible(this._workingDirectory); | ||
| } else if (launchingFile && await fs.pathExists(launchingFile)) { | ||
| // Combine the working directory with this file if possible. | ||
| this._workingDirectory = expandFileVariable(this.launchInfo.workingDir, launchingFile, this.workspace); |
There was a problem hiding this comment.
What if workingDir = ${cwd}/temp.
Users enter such paths in settings.json and launch.json when using python extension.
And also in tasks.json in VS Code.
Using SystemVariables will solve this (it handles multiple variables).
| private _execPath: string; | ||
|
|
||
| constructor(workspaceFolder?: string) { | ||
| constructor(fileOrFolder: Uri | string | undefined, workspace?: IWorkspaceService, documentManager?: IDocumentManager) { |
There was a problem hiding this comment.
fileOrFolder [](start = 16, length = 12)
Sorry hold on a bit. The config settings are now trumping the values. I have to skip resolving when the result is undefined.
There was a problem hiding this comment.
Okay should be fixed now. I sent in a file for the config settings. I should have passed in a folder.
In reply to: 330657034 [](ancestors = 330657034)
There was a problem hiding this comment.
Maybe I should make these separate. It would be less easy to mess up.
In reply to: 330679865 [](ancestors = 330679865,330657034)
4 participants
For #4441, #7688
Support different variables for the root working directory. If not already set, use the first file that runs to determine the folder for an interactive session. For notebooks use the file that the notebook is in.