From a9fcbdb6cf0f660ae46cf81aaa848c2f940796cb Mon Sep 17 00:00:00 2001 From: greazer Date: Mon, 16 Mar 2020 19:53:45 -0700 Subject: [PATCH 1/8] * Update gathered notebook description * Send telemetry when gather isn't available --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- src/client/datascience/gather/gather.ts | 8 ++++++-- src/client/telemetry/index.ts | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/package.nls.json b/package.nls.json index 8a0db0d40533..4af96f84d0d6 100644 --- a/package.nls.json +++ b/package.nls.json @@ -418,7 +418,7 @@ "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", "DataScience.gatheredScriptDescription": "# This file contains only the code required to produce the results of the gathered cell.\n", - "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGenerated from ```{0}```\n\nThis notebook contains only the code and cells required to produce the same results as the gathered cell.\n\nPlease note that the python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.\n\nAs this is an experimental feature, please let us know how well Gather works for you at [https://aka.ms/gathersurvey](https://aka.ms/gathersurvey)", + "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey) [No](https://aka.ms/gathersurvey)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index a6c017df4d00..3b712ea96a6d 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -726,7 +726,7 @@ export namespace DataScience { ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', - '## Gathered Notebook\nGenerated from ```{0}```\n\nThis notebook contains only the code and cells required to produce the same results as the gathered cell.\n\nPlease note that the python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.\n\nAs this is an experimental feature, please let us know how well Gather works for you at [https://aka.ms/gathersurvey](https://aka.ms/gathersurvey)' + '## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey) [No](https://aka.ms/gathersurvey)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( diff --git a/src/client/datascience/gather/gather.ts b/src/client/datascience/gather/gather.ts index 97d74d54a977..27a76e0960fe 100644 --- a/src/client/datascience/gather/gather.ts +++ b/src/client/datascience/gather/gather.ts @@ -7,7 +7,8 @@ import { IConfigurationService, IDisposableRegistry } from '../../common/types'; import * as localize from '../../common/utils/localize'; // tslint:disable-next-line: no-duplicate-imports import { Common } from '../../common/utils/localize'; -import { Identifiers } from '../constants'; +import { sendTelemetryEvent } from '../../telemetry'; +import { Identifiers, Telemetry } from '../constants'; import { CellState, ICell as IVscCell, IGatherProvider } from '../types'; /** @@ -46,7 +47,9 @@ export class GatherProvider implements IGatherProvider { ); } } catch (ex) { - traceInfo('Gathering tools could not be activated. Indicates build of VSIX was not'); + traceInfo( + 'Gathering tools could not be activated. Indicates build of VSIX could not find @msrvida/python-program-analysis' + ); } } } @@ -72,6 +75,7 @@ export class GatherProvider implements IGatherProvider { */ public gatherCode(vscCell: IVscCell): string { if (!this._executionSlicer) { + sendTelemetryEvent(Telemetry.GatherCompleted, undefined, { result: 'unavailable' }); return '# %% [markdown]\n## Gather not available'; } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index aba19912ec31..b702fab2020f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1894,7 +1894,7 @@ export interface IEventNamePropertyMapping { /** * result indicates whether the gather was completed to a script, notebook or suffered an internal error. */ - result: 'err' | 'script' | 'notebook'; + result: 'err' | 'script' | 'notebook' | 'unavailable'; }; /** * Telemetry event sent when the ZMQ native binaries do not work. From 4fc1f86ab2dfa44db6f4fed405eb20cdc10ae9f7 Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 01:08:13 -0700 Subject: [PATCH 2/8] Add succeed_value to links --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.json b/package.nls.json index 4af96f84d0d6..d0dba7307ab4 100644 --- a/package.nls.json +++ b/package.nls.json @@ -418,7 +418,7 @@ "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", "DataScience.gatheredScriptDescription": "# This file contains only the code required to produce the results of the gathered cell.\n", - "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey) [No](https://aka.ms/gathersurvey)", + "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 3b712ea96a6d..f5b29df4aac8 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -726,7 +726,7 @@ export namespace DataScience { ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', - '## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey) [No](https://aka.ms/gathersurvey)' + '# Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( From ff451087485a197d69bb5124d955871c0d39c455 Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 01:29:37 -0700 Subject: [PATCH 3/8] Update gathered script comment. --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.json b/package.nls.json index d0dba7307ab4..1c2a978a8d77 100644 --- a/package.nls.json +++ b/package.nls.json @@ -417,7 +417,7 @@ "DataScience.findJupyterCommandProgress": "Active interpreter does not support {0}. Searching for the best available interpreter.", "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", - "DataScience.gatheredScriptDescription": "# This file contains only the code required to produce the results of the gathered cell.\n", + "DataScience.gatheredScriptDescription":"# This file was generated by an experimental feature called 'Gather'.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n", "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index f5b29df4aac8..ab6dc6c6ee8f 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -722,7 +722,7 @@ export namespace DataScience { ); export const gatheredScriptDescription = localize( 'DataScience.gatheredScriptDescription', - '# This file contains only the code required to produce the results of the gathered cell.\n' + '# This file was generated by an experimental feature called "Gather".\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n' ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', From e5567b553e47bda579303e95b3d1fc547a67400a Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 01:31:11 -0700 Subject: [PATCH 4/8] Restore filename param in gather msg. --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.json b/package.nls.json index 1c2a978a8d77..2bdead0147f8 100644 --- a/package.nls.json +++ b/package.nls.json @@ -418,7 +418,7 @@ "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", "DataScience.gatheredScriptDescription":"# This file was generated by an experimental feature called 'Gather'.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n", - "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)", + "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index ab6dc6c6ee8f..dc13e6a662d3 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -726,7 +726,7 @@ export namespace DataScience { ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', - '# Gathered Notebook\nGathered from ```Untitled-1.ipynb```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)' + '# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( From 6af4600c95446402e84d9cca2aca31eddc021280 Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 11:05:40 -0700 Subject: [PATCH 5/8] Change succeed value to numeric. --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.nls.json b/package.nls.json index 2bdead0147f8..36a153817396 100644 --- a/package.nls.json +++ b/package.nls.json @@ -418,7 +418,7 @@ "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.", "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.", "DataScience.gatheredScriptDescription":"# This file was generated by an experimental feature called 'Gather'.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n", - "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)", + "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=1) [No](https://aka.ms/gathersurvey?succeed_value=0)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index dc13e6a662d3..b7e322b0029f 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -726,7 +726,7 @@ export namespace DataScience { ); export const gatheredNotebookDescriptionInMarkdown = localize( 'DataScience.gatheredNotebookDescriptionInMarkdown', - '# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=yes) [No](https://aka.ms/gathersurvey?succeed_value=no)' + '# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|   |This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=1) [No](https://aka.ms/gathersurvey?succeed_value=0)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( From b20e7baf5e4cd11c42e02d47d5655bb9e1d9787f Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 16:31:31 -0700 Subject: [PATCH 6/8] Send telemetry on saved nb Remove unneeded metadata --- src/client/datascience/constants.ts | 1 + .../datascience/gather/gatherListener.ts | 21 ++++++++++++++++--- src/client/telemetry/index.ts | 4 ++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 02cb95a1c6a9..b87e48b63f0b 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -284,6 +284,7 @@ export enum Telemetry { NewFileForInteractiveWindow = 'DS_INTERNAL.NEW_FILE_USED_IN_INTERACTIVE', KernelInvalid = 'DS_INTERNAL.INVALID_KERNEL_USED', GatherCompleted = 'DATASCIENCE.GATHER_COMPLETED', + GatheredNotebookSaved = 'DATASCIENCE.GATHERED_NOTEBOOK_SAVED', ZMQNotSupported = 'DATASCIENCE.ZMQ_NATIVE_BINARIES_NOT_LOADING' } diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts index 08221ba14207..8b8aea24ce5f 100644 --- a/src/client/datascience/gather/gatherListener.ts +++ b/src/client/datascience/gather/gatherListener.ts @@ -1,6 +1,6 @@ import { inject, injectable } from 'inversify'; import * as uuid from 'uuid/v4'; -import { Event, EventEmitter, Position, Uri, ViewColumn } from 'vscode'; +import { Event, EventEmitter, Position, TextDocument, TextEditor, Uri, ViewColumn } from 'vscode'; import { createMarkdownCell } from '../../../datascience-ui/common/cellFactory'; import { IApplicationShell, IDocumentManager } from '../../common/application/types'; import { PYTHON_LANGUAGE } from '../../common/constants'; @@ -21,6 +21,7 @@ import { IInteractiveWindowProvider, IJupyterExecution, INotebook, + INotebookEditor, INotebookEditorProvider, INotebookExecutionLogger, INotebookExporter @@ -37,6 +38,7 @@ export class GatherListener implements IInteractiveWindowListener { private notebookUri: Uri | undefined; private gatherProvider: IGatherProvider | undefined; private gatherTimer: StopWatch | undefined; + private gatheredNotebookList: INotebookEditor[] = []; constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, @@ -173,9 +175,10 @@ export class GatherListener implements IInteractiveWindowListener { const notebook = await this.jupyterExporter.translateToNotebook(cells); if (notebook) { - notebook.metadata.gatheredNotebook = true; const contents = JSON.stringify(notebook); - await this.ipynbProvider.createNew(contents); + const editor = await this.ipynbProvider.createNew(contents); + this.gatheredNotebookList.push(editor); + editor.saved(this.onSavedGatheredNotebook.bind(this)); } } } @@ -212,4 +215,16 @@ export class GatherListener implements IInteractiveWindowListener { editBuilder.insert(new Position(editor.document.lineCount, 0), '\n'); }); } + + private onSavedGatheredNotebook(editor: INotebookEditor) { + const n = this.gatheredNotebookList.findIndex(e => e === editor); + + // If the editor is in our list of editors representing gathered + // notebooks, send telemetry that it's being saved, then remove it + // from the list so we don't send another event for the same one. + if (n !== -1) { + sendTelemetryEvent(Telemetry.GatheredNotebookSaved); + this.gatheredNotebookList.splice(n, 1); + } + } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index b702fab2020f..3a279a109a79 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1896,6 +1896,10 @@ export interface IEventNamePropertyMapping { */ result: 'err' | 'script' | 'notebook' | 'unavailable'; }; + /** + * Telemetry event sent when a gathered notebook has been saved by the user. + */ + [Telemetry.GatheredNotebookSaved]: undefined | never; /** * Telemetry event sent when the ZMQ native binaries do not work. */ From a6dcd48297fab83e3ddab065f3bb14f91182dc32 Mon Sep 17 00:00:00 2001 From: greazer Date: Tue, 17 Mar 2020 16:56:51 -0700 Subject: [PATCH 7/8] Cleanup tslint. --- src/client/datascience/gather/gatherListener.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts index 8b8aea24ce5f..280752260691 100644 --- a/src/client/datascience/gather/gatherListener.ts +++ b/src/client/datascience/gather/gatherListener.ts @@ -1,6 +1,6 @@ import { inject, injectable } from 'inversify'; import * as uuid from 'uuid/v4'; -import { Event, EventEmitter, Position, TextDocument, TextEditor, Uri, ViewColumn } from 'vscode'; +import { Event, EventEmitter, Position, Uri, ViewColumn } from 'vscode'; import { createMarkdownCell } from '../../../datascience-ui/common/cellFactory'; import { IApplicationShell, IDocumentManager } from '../../common/application/types'; import { PYTHON_LANGUAGE } from '../../common/constants'; From cf80cdc4acdce3c5e95dfcf368c8063c3a7d4e95 Mon Sep 17 00:00:00 2001 From: greazer Date: Wed, 18 Mar 2020 11:32:36 -0700 Subject: [PATCH 8/8] More graceful onSaved handling --- .../datascience/gather/gatherListener.ts | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts index 280752260691..2824da8e0d59 100644 --- a/src/client/datascience/gather/gatherListener.ts +++ b/src/client/datascience/gather/gatherListener.ts @@ -1,4 +1,5 @@ import { inject, injectable } from 'inversify'; +import { IDisposable } from 'monaco-editor'; import * as uuid from 'uuid/v4'; import { Event, EventEmitter, Position, Uri, ViewColumn } from 'vscode'; import { createMarkdownCell } from '../../../datascience-ui/common/cellFactory'; @@ -21,7 +22,6 @@ import { IInteractiveWindowProvider, IJupyterExecution, INotebook, - INotebookEditor, INotebookEditorProvider, INotebookExecutionLogger, INotebookExporter @@ -38,7 +38,6 @@ export class GatherListener implements IInteractiveWindowListener { private notebookUri: Uri | undefined; private gatherProvider: IGatherProvider | undefined; private gatherTimer: StopWatch | undefined; - private gatheredNotebookList: INotebookEditor[] = []; constructor( @inject(IApplicationShell) private applicationShell: IApplicationShell, @@ -177,8 +176,15 @@ export class GatherListener implements IInteractiveWindowListener { if (notebook) { const contents = JSON.stringify(notebook); const editor = await this.ipynbProvider.createNew(contents); - this.gatheredNotebookList.push(editor); - editor.saved(this.onSavedGatheredNotebook.bind(this)); + + let disposable: IDisposable; + const handler = () => { + sendTelemetryEvent(Telemetry.GatheredNotebookSaved); + if (disposable) { + disposable.dispose(); + } + }; + disposable = editor.saved(handler); } } } @@ -215,16 +221,4 @@ export class GatherListener implements IInteractiveWindowListener { editBuilder.insert(new Position(editor.document.lineCount, 0), '\n'); }); } - - private onSavedGatheredNotebook(editor: INotebookEditor) { - const n = this.gatheredNotebookList.findIndex(e => e === editor); - - // If the editor is in our list of editors representing gathered - // notebooks, send telemetry that it's being saved, then remove it - // from the list so we don't send another event for the same one. - if (n !== -1) { - sendTelemetryEvent(Telemetry.GatheredNotebookSaved); - this.gatheredNotebookList.splice(n, 1); - } - } }