From ca4f0ab04863ff009f8cceb575caaac3a80cfb45 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Thu, 27 Apr 2023 11:56:50 +0200 Subject: [PATCH 1/5] Adds telemetry to understand how long the diff editor was visible to the user. --- .../browser/parts/editor/textDiffEditor.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 5583dfd890f40..4b93d0183898f 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -94,7 +94,10 @@ export class TextDiffEditor extends AbstractTextEditor imp return this.diffEditorControl?.getModifiedEditor(); } + private inputShownTimestampMs: number | undefined; + override async setInput(input: DiffEditorInput, options: ITextEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken): Promise { + this.inputShownTimestampMs = undefined; // Dispose previous diff navigator this.diffNavigatorDisposables.clear(); @@ -149,6 +152,8 @@ export class TextDiffEditor extends AbstractTextEditor imp readOnly: resolvedDiffEditorModel.modifiedModel?.isReadonly(), originalEditable: !resolvedDiffEditorModel.originalModel?.isReadonly() }); + + this.inputShownTimestampMs = Date.now(); } catch (error) { await this.handleSetInputError(error, input, options); } @@ -297,6 +302,10 @@ export class TextDiffEditor extends AbstractTextEditor imp } override clearInput(): void { + const editorVisibleTimeMs = this.inputShownTimestampMs !== undefined ? Date.now() - this.inputShownTimestampMs : undefined; + this.inputShownTimestampMs = undefined; + const languageId = this.diffEditorControl?.getModel().modified.getLanguageId(); + super.clearInput(); // Dispose previous diff navigator @@ -304,6 +313,21 @@ export class TextDiffEditor extends AbstractTextEditor imp // Clear Model this.diffEditorControl?.setModel(null); + + if (editorVisibleTimeMs !== undefined) { + this.telemetryService.publicLog2<{ + editorVisibleTimeMs: number; + languageId: string; + }, { + owner: 'hediet'; + editorVisibleTimeMs: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Indicates the time the diff editor was visible to the user' }; + languageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Indicates for which language the diff editor was shown' }; + comment: 'This event gives insight about how long the diff editor was visible to the user.'; + }>('diffEditor.editorVisibleTime', { + editorVisibleTimeMs, + languageId: languageId ?? '', + }); + } } getDiffNavigator(): DiffNavigator | undefined { From 266f1b23ef43b70bad2e830b2f4893bdaba2c535 Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Thu, 27 Apr 2023 12:09:20 +0200 Subject: [PATCH 2/5] Uses StopWatch utility. --- .../workbench/browser/parts/editor/textDiffEditor.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 4b93d0183898f..179b53d720a2f 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -36,6 +36,7 @@ import { Dimension, multibyteAwareBtoa } from 'vs/base/browser/dom'; import { ByteSize, FileOperationError, FileOperationResult, IFileService, TooLargeFileOperationError } from 'vs/platform/files/common/files'; import { IBoundarySashes } from 'vs/base/browser/ui/sash/sash'; import { IPreferencesService } from 'vs/workbench/services/preferences/common/preferences'; +import { StopWatch } from 'vs/base/common/stopwatch'; /** * The text editor that leverages the diff text editor for the editing experience. @@ -94,10 +95,10 @@ export class TextDiffEditor extends AbstractTextEditor imp return this.diffEditorControl?.getModifiedEditor(); } - private inputShownTimestampMs: number | undefined; + private inputShownStopWatch: StopWatch | undefined; override async setInput(input: DiffEditorInput, options: ITextEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken): Promise { - this.inputShownTimestampMs = undefined; + this.inputShownStopWatch = undefined; // Dispose previous diff navigator this.diffNavigatorDisposables.clear(); @@ -153,7 +154,7 @@ export class TextDiffEditor extends AbstractTextEditor imp originalEditable: !resolvedDiffEditorModel.originalModel?.isReadonly() }); - this.inputShownTimestampMs = Date.now(); + this.inputShownStopWatch = new StopWatch(false); } catch (error) { await this.handleSetInputError(error, input, options); } @@ -302,8 +303,8 @@ export class TextDiffEditor extends AbstractTextEditor imp } override clearInput(): void { - const editorVisibleTimeMs = this.inputShownTimestampMs !== undefined ? Date.now() - this.inputShownTimestampMs : undefined; - this.inputShownTimestampMs = undefined; + const editorVisibleTimeMs = this.inputShownStopWatch !== undefined ? this.inputShownStopWatch.elapsed() : undefined; + this.inputShownStopWatch = undefined; const languageId = this.diffEditorControl?.getModel().modified.getLanguageId(); super.clearInput(); From f1790aab70b50f5d07f3b2b81732a0aeda52491b Mon Sep 17 00:00:00 2001 From: Henning Dieterichs Date: Thu, 27 Apr 2023 12:36:34 +0200 Subject: [PATCH 3/5] Fixes CI --- src/vs/workbench/browser/parts/editor/textDiffEditor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 179b53d720a2f..13e9b2ffb5767 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -305,7 +305,7 @@ export class TextDiffEditor extends AbstractTextEditor imp override clearInput(): void { const editorVisibleTimeMs = this.inputShownStopWatch !== undefined ? this.inputShownStopWatch.elapsed() : undefined; this.inputShownStopWatch = undefined; - const languageId = this.diffEditorControl?.getModel().modified.getLanguageId(); + const languageId = this.diffEditorControl?.getModel().modified?.getLanguageId(); super.clearInput(); From e90593d9e7213f6fc12a45c57e4dbcce1f69ec4a Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 27 Apr 2023 14:55:46 +0200 Subject: [PATCH 4/5] :lipstick: --- .../browser/parts/editor/textDiffEditor.ts | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 13e9b2ffb5767..09b74e68bb0d9 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -50,6 +50,8 @@ export class TextDiffEditor extends AbstractTextEditor imp private diffNavigator: DiffNavigator | undefined; private readonly diffNavigatorDisposables = this._register(new DisposableStore()); + private inputLifecycleStopWatch: StopWatch | undefined = undefined; + override get scopedContextKeyService(): IContextKeyService | undefined { if (!this.diffEditorControl) { return undefined; @@ -95,12 +97,10 @@ export class TextDiffEditor extends AbstractTextEditor imp return this.diffEditorControl?.getModifiedEditor(); } - private inputShownStopWatch: StopWatch | undefined; - override async setInput(input: DiffEditorInput, options: ITextEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken): Promise { - this.inputShownStopWatch = undefined; - // Dispose previous diff navigator + // Cleanup previous things associated with the input + this.inputLifecycleStopWatch = undefined; this.diffNavigatorDisposables.clear(); // Set input and resolve @@ -154,7 +154,8 @@ export class TextDiffEditor extends AbstractTextEditor imp originalEditable: !resolvedDiffEditorModel.originalModel?.isReadonly() }); - this.inputShownStopWatch = new StopWatch(false); + // Start to measure input lifecycle + this.inputLifecycleStopWatch = new StopWatch(false); } catch (error) { await this.handleSetInputError(error, input, options); } @@ -303,19 +304,22 @@ export class TextDiffEditor extends AbstractTextEditor imp } override clearInput(): void { - const editorVisibleTimeMs = this.inputShownStopWatch !== undefined ? this.inputShownStopWatch.elapsed() : undefined; - this.inputShownStopWatch = undefined; - const languageId = this.diffEditorControl?.getModel().modified?.getLanguageId(); - super.clearInput(); + // Log input lifecycle telemetry + const inputLifecycleElapsed = this.inputLifecycleStopWatch?.elapsed(); + this.inputLifecycleStopWatch = undefined; + this.logInputLifecycleTelemetry(inputLifecycleElapsed, this.getControl()?.getModel()?.modified?.getLanguageId()); + // Dispose previous diff navigator this.diffNavigatorDisposables.clear(); // Clear Model this.diffEditorControl?.setModel(null); + } - if (editorVisibleTimeMs !== undefined) { + private logInputLifecycleTelemetry(duration: number | undefined, languageId: string | undefined): void { + if (typeof duration === 'number') { this.telemetryService.publicLog2<{ editorVisibleTimeMs: number; languageId: string; @@ -325,7 +329,7 @@ export class TextDiffEditor extends AbstractTextEditor imp languageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Indicates for which language the diff editor was shown' }; comment: 'This event gives insight about how long the diff editor was visible to the user.'; }>('diffEditor.editorVisibleTime', { - editorVisibleTimeMs, + editorVisibleTimeMs: duration, languageId: languageId ?? '', }); } From 225cb76e05420d9ed3bd1520a8fa8f140fccb5bc Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 27 Apr 2023 15:06:37 +0200 Subject: [PATCH 5/5] :lipstick: --- .../browser/parts/editor/textDiffEditor.ts | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts index 09b74e68bb0d9..aedd503099350 100644 --- a/src/vs/workbench/browser/parts/editor/textDiffEditor.ts +++ b/src/vs/workbench/browser/parts/editor/textDiffEditor.ts @@ -309,7 +309,9 @@ export class TextDiffEditor extends AbstractTextEditor imp // Log input lifecycle telemetry const inputLifecycleElapsed = this.inputLifecycleStopWatch?.elapsed(); this.inputLifecycleStopWatch = undefined; - this.logInputLifecycleTelemetry(inputLifecycleElapsed, this.getControl()?.getModel()?.modified?.getLanguageId()); + if (typeof inputLifecycleElapsed === 'number') { + this.logInputLifecycleTelemetry(inputLifecycleElapsed, this.getControl()?.getModel()?.modified?.getLanguageId()); + } // Dispose previous diff navigator this.diffNavigatorDisposables.clear(); @@ -318,21 +320,19 @@ export class TextDiffEditor extends AbstractTextEditor imp this.diffEditorControl?.setModel(null); } - private logInputLifecycleTelemetry(duration: number | undefined, languageId: string | undefined): void { - if (typeof duration === 'number') { - this.telemetryService.publicLog2<{ - editorVisibleTimeMs: number; - languageId: string; - }, { - owner: 'hediet'; - editorVisibleTimeMs: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Indicates the time the diff editor was visible to the user' }; - languageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Indicates for which language the diff editor was shown' }; - comment: 'This event gives insight about how long the diff editor was visible to the user.'; - }>('diffEditor.editorVisibleTime', { - editorVisibleTimeMs: duration, - languageId: languageId ?? '', - }); - } + private logInputLifecycleTelemetry(duration: number, languageId: string | undefined): void { + this.telemetryService.publicLog2<{ + editorVisibleTimeMs: number; + languageId: string; + }, { + owner: 'hediet'; + editorVisibleTimeMs: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'Indicates the time the diff editor was visible to the user' }; + languageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Indicates for which language the diff editor was shown' }; + comment: 'This event gives insight about how long the diff editor was visible to the user.'; + }>('diffEditor.editorVisibleTime', { + editorVisibleTimeMs: duration, + languageId: languageId ?? '', + }); } getDiffNavigator(): DiffNavigator | undefined {