-
Notifications
You must be signed in to change notification settings - Fork 295
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
Load widget scripts from all known Jupyter paths #10726
Conversation
Codecov Report
@@ Coverage Diff @@
## main #10726 +/- ##
======================================
Coverage 62% 62%
======================================
Files 476 476
Lines 32983 33047 +64
Branches 5356 5381 +25
======================================
+ Hits 20726 20799 +73
+ Misses 10264 10256 -8
+ Partials 1993 1992 -1
|
74ef137
to
eb5c91b
Compare
: []; | ||
|
||
if (jupyterPathVars.length > 0) { | ||
jupyterPathVars.forEach(async (jupyterPath) => { |
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 was wrong, basically we're not awaiting on this to complete, hence paths
set will always be empty.
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.
Added a test for this as well
import { parseVersion } from '../utils/version.node'; | ||
import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from './constants.node'; | ||
import { IPlatformService } from './types'; | ||
|
||
@injectable() | ||
export class PlatformService implements IPlatformService { | ||
private readonly _homeDir = Uri.file(os.homedir()); | ||
private readonly _homeDir = getUserHomeDir() || Uri.file(os.homedir()); |
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.
Modified to ensure we use the same function everywhere.
private getCodeToExecute() { | ||
if (!this.code) { | ||
this.code = this.fs.readFile( | ||
Uri.joinPath(this.context.extensionUri, 'pythonFiles', 'printJupyWidgetEntryPoints.py') |
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.
Moved code to a file, easier to manage.
import { Uri } from 'vscode'; | ||
|
||
export function fsPathToUri(path: string | undefined) { | ||
return path ? Uri.file(path) : undefined; |
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.
Removed this as this just compilcates the code unencessarily by adding nullables to the return values, even when the path is not undefined.
I.e. if we call fsPathToUri(<some string value>)
the return value is still nullable, when this is impossible.
This unnecessary nullable complicates the code, hence removing it.
Fixes #10722