Add a "Jupyter not installed" notification helper - #16321
Conversation
| type IRowsResponse = any[]; | ||
|
|
||
| // Note: While #16102 is being worked on, this enum will be updated as we add ways to display this notification. | ||
| export enum JupyterNotInstalledOrigin { |
There was a problem hiding this comment.
While #16102 is being worked on, this enum will be updated as we add ways to display this notification.
| @inject(IJupyterExtensionDependencyManager) private depsManager: IJupyterExtensionDependencyManager, | ||
| ) {} | ||
|
|
||
| public shouldShowJupypterExtensionNotInstalledPrompt(): boolean { |
There was a problem hiding this comment.
This seems like something which can be a diagnostic, like PylanceDefaultDiagnostic you recently implemented.
There was a problem hiding this comment.
It's not something we are going to run on start, but that will be displayed in response to users taking specific actions at different points in the extension. What would be the advantage of using diagnostics in that case compared to showing a notification?
There was a problem hiding this comment.
Ah I see, sorry I missed that.
Diagnostics already have the do not show again functionality so you don't have to create a separate key, and it's consistent with a similar class we have:
which triggers in response to debugging. But eh, it's not much of an advantage, so not changing it is fine.
| @inject(IJupyterExtensionDependencyManager) private depsManager: IJupyterExtensionDependencyManager, | ||
| ) {} | ||
|
|
||
| public shouldShowJupypterExtensionNotInstalledPrompt(): boolean { |
There was a problem hiding this comment.
Ah I see, sorry I missed that.
Diagnostics already have the do not show again functionality so you don't have to create a separate key, and it's consistent with a similar class we have:
which triggers in response to debugging. But eh, it's not much of an advantage, so not changing it is fine.
d294a98
into
microsoft:16102-jupyter-dependency
* Make Jupyter an optional dependency (#16267) * News entry * Move Jupyter to the optional dependencies step * Update news/1 Enhancements/16102.md Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> * License wording update (#16278) * Wording * License wording * Add a "Jupyter not installed" notification helper (#16321) * Add telemetry info * Use enum for the telemetry * Add prompt as a standalone function * Remove "Install" from the prompt * Make it a class * Register singleton * Rename file to a long but descriptive name * Unit tests * Add to package.nls.json * Use sinon for tests * Use the same "Jupyter is not installed" message everywhere (#16372) * rename to showJupyterNotInstalledPrompt * Replace existing prompt with new prompt * Remove Jupyter check from command manager * Update the start page to use the prompt (#16417) * Update copy * Update origin key * Show prompt if jupyter not installed & should show * Add tests for this functionality only * Update news entry * Remove comments * follow-up from the merge * Add singletons for startpage functional tests * Missing one symbol * Update src/client/common/startPage/startPage.ts Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Add logging Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Kartik Raj <karraj@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>
3 participants
For #16102, will be used in later PRs wherever needed.
Tests have been updated to use
sinon.