From 14b2bc0d88838be860e6a06946551a01c288413c Mon Sep 17 00:00:00 2001 From: greazer Date: Thu, 19 Mar 2020 16:59:43 -0700 Subject: [PATCH 1/4] Sending gather quality report (works - not pretty) --- package.json | 6 ++++++ package.nls.json | 2 +- src/client/common/application/commands.ts | 1 + src/client/common/application/webPanels/webPanel.ts | 3 ++- src/client/common/utils/localize.ts | 2 +- src/client/datascience/commands/commandRegistry.ts | 8 +++++++- src/client/datascience/constants.ts | 2 ++ src/client/datascience/interactive-common/linkProvider.ts | 8 +++++++- src/client/telemetry/index.ts | 4 ++++ 9 files changed, 31 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 67e539502a11..e959dba0cf4c 100644 --- a/package.json +++ b/package.json @@ -596,6 +596,12 @@ "title": "%DataScience.selectKernel%", "category": "Python", "enablement": "python.datascience.isnativeactive" + }, + { + "command": "python.datascience.gatherquality", + "title": "%DataScience.gatherGood%", + "category": "Python", + "enablement": "python.datascience.isnativeactive" } ], "menus": { diff --git a/package.nls.json b/package.nls.json index 36a153817396..02fdeeb7a92a 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=1) [No](https://aka.ms/gathersurvey?succeed_value=0)", + "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://command:python.datascience.gatherquality?1) [No](https://command:python.datascience.gatherquality?0)", "DataScience.savePngTitle": "Save Image", "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter", "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index f75a1760ffd3..8da4a3d858b4 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -161,4 +161,5 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.SaveNotebookNonCustomEditor]: [Uri]; [DSCommands.SaveAsNotebookNonCustomEditor]: [Uri, Uri]; [DSCommands.OpenNotebookNonCustomEditor]: [Uri]; + [DSCommands.GatherQuality]: [string]; } diff --git a/src/client/common/application/webPanels/webPanel.ts b/src/client/common/application/webPanels/webPanel.ts index f64494d71140..942109cfd010 100644 --- a/src/client/common/application/webPanels/webPanel.ts +++ b/src/client/common/application/webPanels/webPanel.ts @@ -34,7 +34,8 @@ export class WebPanel implements IWebPanel { const webViewOptions: WebviewOptions = { enableScripts: true, localResourceRoots: [Uri.file(this.options.rootPath), Uri.file(this.options.cwd)], - portMapping: port ? [{ webviewPort: RemappedPort, extensionHostPort: port }] : undefined + portMapping: port ? [{ webviewPort: RemappedPort, extensionHostPort: port }] : undefined, + enableCommandUris: true }; if (options.webViewPanel) { this.panel = options.webViewPanel; diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index b7e322b0029f..6848e9742763 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=1) [No](https://aka.ms/gathersurvey?succeed_value=0)' + '# 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://command:python.datascience.gatherquality?1) [No](https://command:python.datascience.gatherquality?0)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( diff --git a/src/client/datascience/commands/commandRegistry.ts b/src/client/datascience/commands/commandRegistry.ts index a3e3ab123348..112acd145b85 100644 --- a/src/client/datascience/commands/commandRegistry.ts +++ b/src/client/datascience/commands/commandRegistry.ts @@ -9,7 +9,8 @@ import { ICommandNameArgumentTypeMapping } from '../../common/application/comman import { ICommandManager, IDebugService, IDocumentManager } from '../../common/application/types'; import { IDisposable, IOutputChannel } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; -import { captureTelemetry } from '../../telemetry'; +import { noop } from '../../common/utils/misc'; +import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { Commands, JUPYTER_OUTPUT_CHANNEL, Telemetry } from '../constants'; import { ICodeWatcher, @@ -68,6 +69,7 @@ export class CommandRegistry implements IDisposable { this.registerCommand(Commands.DebugCurrentCellPalette, this.debugCurrentCellFromCursor); this.registerCommand(Commands.CreateNewNotebook, this.createNewNotebook); this.registerCommand(Commands.ViewJupyterOutput, this.viewJupyterOutput); + this.registerCommand(Commands.GatherQuality, this.reportGatherQuality); if (this.commandListeners) { this.commandListeners.forEach((listener: IDataScienceCommandListener) => { listener.register(this.commandManager); @@ -344,4 +346,8 @@ export class CommandRegistry implements IDisposable { // Ask our code lens provider to find the matching code watcher for the current document return this.dataScienceCodeLensProvider.getCodeWatcher(activeEditor.document); } + + private reportGatherQuality(val: string) { + sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val === '0' ? 'bad' : 'good' }); + } } diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index b87e48b63f0b..f5db0f19d32a 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -81,6 +81,7 @@ export namespace Commands { export const SaveNotebookNonCustomEditor = 'python.datascience.notebookeditor.save'; export const SaveAsNotebookNonCustomEditor = 'python.datascience.notebookeditor.saveAs'; export const OpenNotebookNonCustomEditor = 'python.datascience.notebookeditor.open'; + export const GatherQuality = 'python.datascience.gatherquality'; } export namespace CodeLensCommands { @@ -285,6 +286,7 @@ export enum Telemetry { KernelInvalid = 'DS_INTERNAL.INVALID_KERNEL_USED', GatherCompleted = 'DATASCIENCE.GATHER_COMPLETED', GatheredNotebookSaved = 'DATASCIENCE.GATHERED_NOTEBOOK_SAVED', + GatherQualityReport = 'DS_INTERNAL.GATHER_QUALITY_REPORT', ZMQNotSupported = 'DATASCIENCE.ZMQ_NATIVE_BINARIES_NOT_LOADING' } diff --git a/src/client/datascience/interactive-common/linkProvider.ts b/src/client/datascience/interactive-common/linkProvider.ts index 636f940ea02e..f59295b56bec 100644 --- a/src/client/datascience/interactive-common/linkProvider.ts +++ b/src/client/datascience/interactive-common/linkProvider.ts @@ -6,6 +6,7 @@ import '../../common/extensions'; import { inject, injectable } from 'inversify'; import { Event, EventEmitter, Position, Range, Selection, TextEditorRevealType, Uri } from 'vscode'; +import * as vscode from 'vscode'; import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types'; import { IFileSystem } from '../../common/platform/types'; import * as localize from '../../common/utils/localize'; @@ -40,9 +41,14 @@ export class LinkProvider implements IInteractiveWindowListener { case InteractiveWindowMessages.OpenLink: if (payload) { // Special case file URIs - const href = payload.toString(); + const href: string = payload.toString(); if (href.startsWith('file')) { this.openFile(href); + } else if (href.startsWith('https://command:')) { + const temp: string = href.split(':')[2]; + const command = temp.split('/?')[0]; + const param: string = temp.split('/?')[1]; + vscode.commands.executeCommand(command, [param]); } else { this.applicationShell.openUrl(href); } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 3a279a109a79..c7ab1d24e240 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1900,6 +1900,10 @@ export interface IEventNamePropertyMapping { * Telemetry event sent when a gathered notebook has been saved by the user. */ [Telemetry.GatheredNotebookSaved]: undefined | never; + /** + * Telemetry event sent when the user reports whether Gathered notebook was good or not + */ + [Telemetry.GatherQualityReport]: { result: 'good' | 'bad' }; /** * Telemetry event sent when the ZMQ native binaries do not work. */ From 661bbd0af87879a0ae1d4a517fa420ffbe82c6b5 Mon Sep 17 00:00:00 2001 From: greazer Date: Fri, 20 Mar 2020 14:51:21 -0700 Subject: [PATCH 2/4] Actually open the gather survey And whitelist the command that does so. We don't necessarily want to open the door of being able to execute arbitrary commands to notebooks. Yet. --- src/client/datascience/commands/commandRegistry.ts | 4 ++-- .../datascience/interactive-common/linkProvider.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/client/datascience/commands/commandRegistry.ts b/src/client/datascience/commands/commandRegistry.ts index 112acd145b85..42f13484e4d4 100644 --- a/src/client/datascience/commands/commandRegistry.ts +++ b/src/client/datascience/commands/commandRegistry.ts @@ -4,12 +4,11 @@ 'use strict'; import { inject, injectable, multiInject, named, optional } from 'inversify'; -import { CodeLens, Range } from 'vscode'; +import { CodeLens, env, Range, Uri } from 'vscode'; import { ICommandNameArgumentTypeMapping } from '../../common/application/commands'; import { ICommandManager, IDebugService, IDocumentManager } from '../../common/application/types'; import { IDisposable, IOutputChannel } from '../../common/types'; import { DataScience } from '../../common/utils/localize'; -import { noop } from '../../common/utils/misc'; import { captureTelemetry, sendTelemetryEvent } from '../../telemetry'; import { Commands, JUPYTER_OUTPUT_CHANNEL, Telemetry } from '../constants'; import { @@ -349,5 +348,6 @@ export class CommandRegistry implements IDisposable { private reportGatherQuality(val: string) { sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val === '0' ? 'bad' : 'good' }); + env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed_value=${val}`)); } } diff --git a/src/client/datascience/interactive-common/linkProvider.ts b/src/client/datascience/interactive-common/linkProvider.ts index f59295b56bec..a6ca7818b869 100644 --- a/src/client/datascience/interactive-common/linkProvider.ts +++ b/src/client/datascience/interactive-common/linkProvider.ts @@ -4,9 +4,8 @@ import '../../common/extensions'; import { inject, injectable } from 'inversify'; -import { Event, EventEmitter, Position, Range, Selection, TextEditorRevealType, Uri } from 'vscode'; +import { commands, Event, EventEmitter, Position, Range, Selection, TextEditorRevealType, Uri } from 'vscode'; -import * as vscode from 'vscode'; import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types'; import { IFileSystem } from '../../common/platform/types'; import * as localize from '../../common/utils/localize'; @@ -16,6 +15,10 @@ import { InteractiveWindowMessages } from './interactiveWindowTypes'; const LineQueryRegex = /line=(\d+)/; +// The following list of commands represent those that can be executed +// in a markdown cell using the syntax: https://command:[my.vscode.command]. +const linkCommandWhitelist = ['python.datascience.gatherquality']; + // tslint:disable: no-any @injectable() export class LinkProvider implements IInteractiveWindowListener { @@ -47,8 +50,10 @@ export class LinkProvider implements IInteractiveWindowListener { } else if (href.startsWith('https://command:')) { const temp: string = href.split(':')[2]; const command = temp.split('/?')[0]; - const param: string = temp.split('/?')[1]; - vscode.commands.executeCommand(command, [param]); + const params: string[] = temp.split('/?')[1].split(','); + if (linkCommandWhitelist.includes(command)) { + commands.executeCommand(command, params); + } } else { this.applicationShell.openUrl(href); } From d799d12fa1620321a404014549b3e893c3fa0c63 Mon Sep 17 00:00:00 2001 From: greazer Date: Fri, 20 Mar 2020 15:31:56 -0700 Subject: [PATCH 3/4] Fixup for PR submit --- package.json | 3 +-- src/client/common/application/webPanels/webPanel.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 440e94584414..ca605ff667b9 100644 --- a/package.json +++ b/package.json @@ -600,8 +600,7 @@ { "command": "python.datascience.gatherquality", "title": "%DataScience.gatherGood%", - "category": "Python", - "enablement": "python.datascience.isnativeactive" + "category": "Python" } ], "menus": { diff --git a/src/client/common/application/webPanels/webPanel.ts b/src/client/common/application/webPanels/webPanel.ts index 942109cfd010..f64494d71140 100644 --- a/src/client/common/application/webPanels/webPanel.ts +++ b/src/client/common/application/webPanels/webPanel.ts @@ -34,8 +34,7 @@ export class WebPanel implements IWebPanel { const webViewOptions: WebviewOptions = { enableScripts: true, localResourceRoots: [Uri.file(this.options.rootPath), Uri.file(this.options.cwd)], - portMapping: port ? [{ webviewPort: RemappedPort, extensionHostPort: port }] : undefined, - enableCommandUris: true + portMapping: port ? [{ webviewPort: RemappedPort, extensionHostPort: port }] : undefined }; if (options.webViewPanel) { this.panel = options.webViewPanel; From 09ff866f805750f666676fc5220e7855cc85c115 Mon Sep 17 00:00:00 2001 From: greazer Date: Fri, 20 Mar 2020 16:36:39 -0700 Subject: [PATCH 4/4] Tweak to work correctly with surveymonkey --- package.nls.json | 2 +- src/client/common/utils/localize.ts | 2 +- src/client/datascience/commands/commandRegistry.ts | 4 ++-- src/client/telemetry/index.ts | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.nls.json b/package.nls.json index 02fdeeb7a92a..e7529070ee4d 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://command:python.datascience.gatherquality?1) [No](https://command:python.datascience.gatherquality?0)", + "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://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?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 6848e9742763..c17fbfc7672c 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://command:python.datascience.gatherquality?1) [No](https://command:python.datascience.gatherquality?0)' + '# 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://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)' ); export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image'); export const fallbackToUseActiveInterpeterAsKernel = localize( diff --git a/src/client/datascience/commands/commandRegistry.ts b/src/client/datascience/commands/commandRegistry.ts index 42f13484e4d4..280a8d731d1c 100644 --- a/src/client/datascience/commands/commandRegistry.ts +++ b/src/client/datascience/commands/commandRegistry.ts @@ -347,7 +347,7 @@ export class CommandRegistry implements IDisposable { } private reportGatherQuality(val: string) { - sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val === '0' ? 'bad' : 'good' }); - env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed_value=${val}`)); + sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val === 'no' ? 'no' : 'yes' }); + env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed=${val}`)); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index c7ab1d24e240..2077192e2aa4 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1903,7 +1903,7 @@ export interface IEventNamePropertyMapping { /** * Telemetry event sent when the user reports whether Gathered notebook was good or not */ - [Telemetry.GatherQualityReport]: { result: 'good' | 'bad' }; + [Telemetry.GatherQualityReport]: { result: 'yes' | 'no' }; /** * Telemetry event sent when the ZMQ native binaries do not work. */