diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 5a371aa92ff5..03b38aaec1bf 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -368,7 +368,11 @@ export enum Telemetry { RunByLineStart = 'DATASCIENCE.RUN_BY_LINE', RunByLineStep = 'DATASCIENCE.RUN_BY_LINE_STEP', RunByLineStop = 'DATASCIENCE.RUN_BY_LINE_STOP', - RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER' + RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER', + TrustAllNotebooks = 'DATASCIENCE.TRUST_ALL_NOTEBOOKS', + TrustNotebook = 'DATASCIENCE.TRUST_NOTEBOOK', + DoNotTrustNotebook = 'DATASCIENCE.DO_NOT_TRUST_NOTEBOOK', + NotebookTrustPromptShown = 'DATASCIENCE.NOTEBOOK_TRUST_PROMPT_SHOWN' } export enum NativeKeyboardCommandTelemetry { diff --git a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts index 10f2e29eb8ed..d9600bf8f483 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditorStorage.ts @@ -282,10 +282,10 @@ export class NativeEditorStorage implements INotebookStorage { const dirtyContents = skipDirtyContents ? undefined : await this.getStoredContents(file, backupId); if (dirtyContents) { // This means we're dirty. Indicate dirty and load from this content - return this.loadContents(file, dirtyContents, true, contents, forVSCodeNotebook); + return this.loadContents(file, dirtyContents, true, forVSCodeNotebook); } else { // Load without setting dirty - return this.loadContents(file, contents, undefined, undefined, forVSCodeNotebook); + return this.loadContents(file, contents, undefined, forVSCodeNotebook); } } catch (ex) { // May not exist at this time. Should always have a single cell though @@ -308,7 +308,6 @@ export class NativeEditorStorage implements INotebookStorage { file: Uri, contents: string | undefined, isInitiallyDirty = false, - trueContents?: string, forVSCodeNotebook?: boolean ) { // tslint:disable-next-line: no-any @@ -348,18 +347,9 @@ export class NativeEditorStorage implements INotebookStorage { } const pythonNumber = json ? await this.extractPythonMainVersion(json) : 3; - /* As an optimization, we don't call trustNotebook for hot exit, since our hot exit backup code gets called by VS - Code whenever the notebook model changes. This means it's called very often, perhaps even as often as autosave. - Instead, when loading a file that is dirty, we check if the actual file contents on disk are trusted. If so, we treat - the dirty contents as trusted as well. */ - const contentsToCheck = isInitiallyDirty && trueContents !== undefined ? trueContents : contents; - const isTrusted = - contents === undefined || isUntitledFile(file) - ? true // If no contents or untitled, this is a newly created file, so it should be trusted - : await this.trustService.isNotebookTrusted(file, contentsToCheck!); - return this.factory.createModel( + const model = this.factory.createModel( { - trusted: isTrusted, + trusted: true, file, cells: remapped, notebookJson: json, @@ -369,6 +359,23 @@ export class NativeEditorStorage implements INotebookStorage { }, forVSCodeNotebook ); + + // If no contents or untitled, this is a newly created file + // If dirty, that means it's been edited before in our extension + if (contents !== undefined && !isUntitledFile(file) && !isInitiallyDirty) { + const isNotebookTrusted = await this.trustService.isNotebookTrusted(file, model.getContent()); + if (isNotebookTrusted !== model.isTrusted) { + model.update({ + source: 'user', + kind: 'updateTrust', + oldDirty: model.isDirty, + newDirty: model.isDirty, + isNotebookTrusted + }); + } + } + + return model; } private getStaticStorageKey(file: Uri): string { diff --git a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts index 6d44adda8a91..b9a002749cbb 100644 --- a/src/client/datascience/interactive-ipynb/trustCommandHandler.ts +++ b/src/client/datascience/interactive-ipynb/trustCommandHandler.ts @@ -13,7 +13,8 @@ import '../../common/extensions'; import { IDisposableRegistry, IExperimentService } from '../../common/types'; import { swallowExceptions } from '../../common/utils/decorators'; import { DataScience } from '../../common/utils/localize'; -import { Commands } from '../constants'; +import { sendTelemetryEvent } from '../../telemetry'; +import { Commands, Telemetry } from '../constants'; import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider'; import { INotebookEditorProvider, ITrustService } from '../types'; @@ -57,10 +58,12 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { DataScience.doNotTrustNotebook(), DataScience.trustAllNotebooks() ); + sendTelemetryEvent(Telemetry.NotebookTrustPromptShown); switch (selection) { case DataScience.trustAllNotebooks(): commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks'); + sendTelemetryEvent(Telemetry.TrustAllNotebooks); break; case DataScience.trustNotebook(): // Update model trust @@ -73,6 +76,10 @@ export class TrustCommandHandler implements IExtensionSingleActivationService { }); const contents = model.getContent(); await this.trustService.trustNotebook(model.file, contents); + sendTelemetryEvent(Telemetry.TrustNotebook); + break; + case DataScience.doNotTrustNotebook(): + sendTelemetryEvent(Telemetry.DoNotTrustNotebook); break; default: break; diff --git a/src/client/datascience/notebookStorage/baseModel.ts b/src/client/datascience/notebookStorage/baseModel.ts index 89fbaf7c5bad..452534c162e5 100644 --- a/src/client/datascience/notebookStorage/baseModel.ts +++ b/src/client/datascience/notebookStorage/baseModel.ts @@ -196,12 +196,8 @@ export abstract class BaseNotebookModel implements INotebookModel { this.ensureNotebookJson(); // Reuse our original json except for the cells. - const json = { - cells: this.cells.map((c) => pruneCell(c.data)), - metadata: this.notebookJson.metadata, - nbformat: this.notebookJson.nbformat, - nbformat_minor: this.notebookJson.nbformat_minor - }; + const json = { ...this.notebookJson }; + json.cells = this.cells.map((c) => pruneCell(c.data)); return JSON.stringify(json, null, this.indentAmount); } } diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 0f76ead0ec48..ff26bcf385b2 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2192,10 +2192,20 @@ export interface IEventNamePropertyMapping { [Telemetry.StartPageOpenFileBrowser]: never | undefined; [Telemetry.StartPageOpenFolder]: never | undefined; [Telemetry.StartPageOpenWorkspace]: never | undefined; + + // Run by line events [Telemetry.RunByLineStart]: never | undefined; [Telemetry.RunByLineStep]: never | undefined; [Telemetry.RunByLineStop]: never | undefined; [Telemetry.RunByLineVariableHover]: never | undefined; + + // Trusted notebooks events + [Telemetry.NotebookTrustPromptShown]: never | undefined; + [Telemetry.TrustNotebook]: never | undefined; + [Telemetry.TrustAllNotebooks]: never | undefined; + [Telemetry.DoNotTrustNotebook]: never | undefined; + + // Native notebooks events [VSCodeNativeTelemetry.AddCell]: never | undefined; [VSCodeNativeTelemetry.DeleteCell]: never | undefined; [VSCodeNativeTelemetry.MoveCell]: never | undefined; diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 77fe2ad356db..19ddede77e01 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -892,8 +892,7 @@ export function getConnectedNativeCell() { function isCellNavigationKeyboardEvent(e: IKeyboardEvent) { return ( - e.code === 'Enter' || - e.code === 'NumpadEnter' || + ((e.code === 'Enter' || e.code === 'NumpadEnter') && !e.shiftKey && !e.ctrlKey && !e.altKey) || e.code === 'ArrowUp' || e.code === 'k' || e.code === 'ArrowDown' ||