diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts index be79548638..a132b28663 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts @@ -29,6 +29,7 @@ test("opening historical source requests its version and opens a separate native {} as never, {} as never, {} as never, + { capture() {} } as never, ); t.after(() => tabs.dispose()); await tabs.openApiSource({ reviewId: "review-a", kind: "version", version: 7 }, "Historical Review"); @@ -64,7 +65,7 @@ test("native group restoration preserves both reviews, order and pinned source v }; const groupService = { groups, mainPart: { activeGroup: undefined as EditorGroupModel | undefined } }; const createTabs = () => - new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never); + new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never, { capture() {} } as never); tabs = createTabs(); const registry = Registry.as(EditorExtensions.EditorFactory); registry.start({ get: () => instantiation } as never); @@ -131,7 +132,7 @@ test("current Source tabs retain identity and main version tabs still restore", }, invokeFunction(fn: (accessor: { get(): ReviewCanvasEditorTabsService }) => unknown) { return fn({ get: () => tabs }); }, }; - tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never); + tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, {} as never, { capture() {} } as never); try { const serializer = new ReviewApiEditorSerializer(); const restored = serializer.deserialize(instantiation as never, JSON.stringify({ kind: "api-source", reviewId: "a", title: "A", selection: { reviewId: "a", kind: "current" } })); diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts index d68d6b7c8a..a71b9d85e0 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts @@ -1060,6 +1060,7 @@ export class ReviewCanvasEditorPane extends EditorPane { this.reviewTelemetryService.capture("setting_changed", { setting: "open_files_in", enabled: true, + choice, }); await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_SETTING, choice, ConfigurationTarget.USER); return this.currentOpenFilesIn(); @@ -1170,6 +1171,7 @@ export class ReviewCanvasEditorPane extends EditorPane { this.reviewTelemetryService.capture("setting_changed", { setting: "open_files_in", enabled: true, + choice: "application", }); await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_APPLICATION_SETTING, path, ConfigurationTarget.USER); await this.configurationService.updateValue(REVIEW_OPEN_FILES_IN_SETTING, "application", ConfigurationTarget.USER); diff --git a/apps/review-desktop/code-oss/src/vs/review/contrib/telemetry/reviewLspTelemetry.contribution.ts b/apps/review-desktop/code-oss/src/vs/review/contrib/telemetry/reviewLspTelemetry.contribution.ts index 1845d70a25..48a83d2fd2 100644 --- a/apps/review-desktop/code-oss/src/vs/review/contrib/telemetry/reviewLspTelemetry.contribution.ts +++ b/apps/review-desktop/code-oss/src/vs/review/contrib/telemetry/reviewLspTelemetry.contribution.ts @@ -24,6 +24,7 @@ import { import { IExtensionService } from '../../../workbench/services/extensions/common/extensions.js'; import { LifecyclePhase } from '../../../workbench/services/lifecycle/common/lifecycle.js'; import { ReviewMonacoPeeks } from '../../services/reviewMonacoPeeks.js'; +import { isReviewDefinitionPeekEditor } from '../../services/reviewDefinitionPeek.js'; import { IReviewTelemetryService } from '../../services/reviewTelemetryService.js'; const HOVER_DWELL_MS = 1_000; @@ -74,6 +75,7 @@ const LSP_COMMANDS: Record = { 'editor.action.goToReferences': 'references', 'editor.action.referenceSearch.trigger': 'references', 'editor.action.findReferences': 'references', + 'review.action.embeddedShowReferences': 'references', 'editor.action.rename': 'rename', 'editor.action.formatDocument': 'format', 'editor.action.formatSelection': 'format', @@ -184,7 +186,7 @@ class ReviewLspTelemetryContribution extends Disposable implements IWorkbenchCon captured = true; if (Date.now() - (this.lastCommandHover.get(editor) ?? 0) >= HOVER_DWELL_MS) { const model = editor.getModel(); - if (model) { + if (model && this.languageFeaturesService.hoverProvider.has(model)) { this.captureLspUsed(editor, model, 'hover', 'mouse'); } } @@ -240,6 +242,9 @@ class ReviewLspTelemetryContribution extends Disposable implements IWorkbenchCon if (ReviewMonacoPeeks.owns(editor)) { return 'inline_peek'; } + if (isReviewDefinitionPeekEditor(editor)) { + return 'definition_peek'; + } for (const diffEditor of this.codeEditorService.listDiffEditors()) { if (diffEditor.getOriginalEditor() === editor || diffEditor.getModifiedEditor() === editor) { return 'diff'; diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts index 5e8603fd1d..903c322e33 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts @@ -41,6 +41,7 @@ async function closeWelcome(updateNeeded: boolean): Promise { {} as never, {} as never, {} as never, + { capture() {} } as never, ); try { const welcome = await tabs.openWelcome(true); @@ -75,6 +76,7 @@ function sourceTabs(t: TestContext, answer: (url: URL) => object, openFilesIn = { async writeFile(resource: URI, content: VSBuffer) { written.push(`${resource.toString()}=${content.toString()}`); } } as never, { cacheHome: URI.file("/laptop/cache") } as never, { getValue: () => openFilesIn } as never, + { capture() {} } as never, ); t.after(() => tabs.dispose()); return { tabs, opened, written }; diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts index 70c1d2dae5..c87ebb6192 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts @@ -33,6 +33,7 @@ import { REVIEW_LANGUAGE_SOURCE_SCHEME } from "../common/reviewReadonlySource.js import { sourceLocation, sourceSelectionIdentity, REVIEW_API_SOURCE_SCHEME } from "../common/reviewSourceView.js"; import { reviewRemoteAuthority } from "./remote/reviewRemoteAuthority.js"; import { IReviewDesktopConnectionService, reviewResponseError } from "./reviewDesktopConnectionService.js"; +import { IReviewTelemetryService } from "./reviewTelemetryService.js"; type NavigatorAnswer = { workspacePath: string; filePath?: string; rootPath?: string } | ReviewRemoteNavigatorAnswer; @@ -85,6 +86,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview @IFileService private readonly files: IFileService, @IEnvironmentService private readonly environment: IEnvironmentService, @IConfigurationService private readonly configurationService: IConfigurationService, + @IReviewTelemetryService private readonly telemetry: IReviewTelemetryService, ) { super(); this._register( @@ -137,6 +139,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview const result = await this.navigatorWorkspace(selection.reviewId, selection.kind === "version" ? { version: selection.version } : {}); const { uri, remote } = navigatorPaths(result); await this.host.openWindow([{ workspaceUri: uri(result.workspacePath), label: title }], { forceNewWindow: true, ...remote }); + this.telemetry.capture("source_opened", { destination: "source_window", kind: "tree" }); } /** @@ -151,26 +154,31 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview const destinations = await Promise.all(resources.map(resource => this.sourceDestination(resource))); const { workspaceUri, remote } = destinations[destinations.length - 1]; const selection = !diff ? (editor.options as ITextEditorOptions | undefined)?.selection : undefined; - if (!diff && destinations[0].external && await this.openExternalEditor(destinations[0], selection)) return true; + const external = !diff && destinations[0].external ? await this.openExternalEditor(destinations[0], selection) : undefined; + if (external) { + this.telemetry.capture("source_opened", { destination: external, kind: "file" }); + return true; + } await this.host.openWindow([ { workspaceUri }, ...destinations.map(({ fileUri }) => ({ fileUri: selection ? fileUri.with({ path: `${fileUri.path}:${selection.startLineNumber}:${selection.startColumn ?? 1}` }) : fileUri })), ], { forceNewWindow: true, gotoLineMode: true, diffMode: diff, ...remote }); + this.telemetry.capture("source_opened", { destination: "source_window", kind: diff ? "diff" : "file" }); return true; } - private async openExternalEditor({ fileUri, rootPath }: { fileUri: URI; rootPath?: string }, selection: ITextEditorOptions["selection"]): Promise { + private async openExternalEditor({ fileUri, rootPath }: { fileUri: URI; rootPath?: string }, selection: ITextEditorOptions["selection"]): Promise { const editor = this.configurationService.getValue(REVIEW_OPEN_FILES_IN_SETTING); if (editor === "application") { // An application takes the file alone: only the editors' URLs carry a line. const application = this.configurationService.getValue(REVIEW_OPEN_FILES_IN_APPLICATION_SETTING); - if (!application) return false; + if (!application) return undefined; await this.desktopConnection.openInApplication(application, fileUri.fsPath); - return true; + return editor; } - if (!isExternalEditor(editor)) return false; + if (!isExternalEditor(editor)) return undefined; await this.desktopConnection.openInExternalEditor({ editor, filePath: fileUri.fsPath, line: selection?.startLineNumber, column: selection?.startColumn, folder: rootPath }); - return true; + return editor; } async openSourceReferences(resource: URI, position: { readonly lineNumber: number; readonly column: number }): Promise { @@ -181,6 +189,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview ...remote, reviewReferencesToShow: { resource: fileUri, lineNumber: position.lineNumber, column: position.column }, }); + this.telemetry.capture("source_opened", { destination: "source_window", kind: "references" }); return true; } diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewDefinitionPeek.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewDefinitionPeek.ts index 930613b6f6..d4fd76798a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewDefinitionPeek.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewDefinitionPeek.ts @@ -40,6 +40,10 @@ export function markReviewDefinitionPeekHost(container: HTMLElement, diffs: Revi return toDisposable(() => hosts.delete(container)); } +export function isReviewDefinitionPeekEditor(editor: ICodeEditor): boolean { + return !!peeks.get(editor)?.editors.includes(editor); +} + function hostDiffs(editor: ICodeEditor): ReviewDefinitionPeekDiffs | undefined { // A native peek's preview keeps its own navigation. if (editor instanceof EmbeddedCodeEditorWidget) return undefined; @@ -169,7 +173,10 @@ export class ReviewDefinitionPeek extends Disposable { const range = (input.options as { selection?: IRange } | undefined)?.selection; if (!source || sideBySide || !range || input.resource.scheme !== REVIEW_API_SOURCE_SCHEME) return null; const peek = peeks.get(source); - if (peek?.editors.includes(source)) return peek.setInput(input.resource, range); + if (peek?.editors.includes(source)) { + telemetry.capture("peek_opened", { via: "diff_definition" }); + return peek.setInput(input.resource, range); + } const diffs = hostDiffs(source); if (!diffs) return null; telemetry.capture("peek_opened", { via: "diff_definition" }); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts index 2a26ac0cf9..db84ae3c1a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts @@ -35,6 +35,7 @@ function sourceResolver(t: TestContext, options: { openFilesIn?: string; applica {} as never, {} as never, { getValue: (key: string) => ({ "review.openFilesIn": options.openFilesIn, "review.openFilesInApplication": options.application })[key] } as never, + { capture() {} } as never, ); const resolver = new ReviewEditorResolverService( { get activeGroup() { throw new Error("Review must not create an editor group"); } } as never, diff --git a/docs/telemetry.md b/docs/telemetry.md index b55fc0e86a..49bb7d067f 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -250,6 +250,7 @@ text, and only as described in "Error reports". | `review_map_expanded` | `level` in system, container, component, code | A user expands a map element | | `review_commit_diff_opened` | `via` in row, file, footer | A user opens a commit diff | | `review_source_tree_opened` | `via` in topbar, home | A user opens the source tree | +| `review_source_opened` | `destination` in source_window, vscode, cursor, zed, application; `kind` in file, diff, references, tree | A file, diff, references list or source tree opens outside the canvas | | `review_whiteboard_file_exported` | none | A user exports a board as a .whiteboard file | | `review_client_error` | See "Error reports" | A part of Whiteboard reports an error | | `review_update_started` | Random `update_attempt_id`, `target_version` | An update is downloaded and ready to install | @@ -258,7 +259,7 @@ text, and only as described in "Error reports". | `review_bug_report_dialog_opened` | None | A user opens the bug report dialog | | `review_bug_report_cancelled` | None | A user closes the dialog without a report | | `review_bug_report_send_failed` | Short `error_name` | A bug report request fails | -| `review_setting_changed` | `setting` in telemetry_enabled, keymap, software_map_enabled, show_release_notes, update_pill, diffr_config, theme, ctrl_tab, ready_notification, document_width, code_font_size, document_font_size, reading_font, reading_font_family, code_font_family, font_ligatures, diff_font_size, diff_word_wrap, diff_ignore_whitespace; `enabled`; `value` in dark, light (theme only) | A user changes a Whiteboard setting | +| `review_setting_changed` | `setting` in telemetry_enabled, keymap, software_map_enabled, show_release_notes, update_pill, diffr_config, theme, ctrl_tab, open_files_in, ready_notification, document_width, code_font_size, document_font_size, reading_font, reading_font_family, code_font_family, font_ligatures, diff_font_size, diff_word_wrap, diff_ignore_whitespace; `enabled`; `value` in dark, light (theme only); `choice` in whiteboard, vscode, cursor, zed, application (open_files_in only) | A user changes a Whiteboard setting | | `review_review_opened` | `via` in home, other | A user opens a whiteboard | | `review_diff_layout_changed` | `layout` in split, unified; `scope` in diff, peek | A user switches the diff layout | | `review_home_empty_state_viewed` | None | The empty Home state opens | @@ -299,7 +300,7 @@ session with `outcome: "app_quit"`, the same as quitting. | Event | Additional properties | When | | --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------- | -| `review_lsp_used` | `feature` in hover, goto_definition, peek_definition, goto_type_definition, goto_implementation, references, rename, format, code_action, symbol_search; `via` in command, mouse; `language`; `editor_kind` in files_tab, inline_peek, diff | A user invokes an LSP feature | +| `review_lsp_used` | `feature` in hover, goto_definition, peek_definition, goto_type_definition, goto_implementation, references, rename, format, code_action, symbol_search; `via` in command, mouse; `language`; `editor_kind` in files_tab, inline_peek, diff, definition_peek | A user invokes an LSP feature | | `review_ls_activated` | `group` in python, go, rust, swift, csharp; `ok` | A language server activation check ends | | `review_extension_installed` | Allowlisted `extension_id`; `trigger` in user, auto_upgrade, startup_seed, keymap, rollback; `cached`; `duration_ms` | An optional extension installs | | `review_extension_install_failed` | Allowlisted `extension_id`; allowlisted `trigger`; `phase` in download, install | An optional extension install fails | diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index 1425a38a22..e512602ee5 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -724,12 +724,16 @@ export const REVIEW_KEYMAP_CHOICES = [ export type ReviewKeymapChoice = (typeof REVIEW_KEYMAP_CHOICES)[number]; +export const REVIEW_OPEN_FILES_IN_CHOICES = [ + "whiteboard", + "vscode", + "cursor", + "zed", + "application", +] as const; + export type ReviewOpenFilesInChoice = - | "whiteboard" - | "vscode" - | "cursor" - | "zed" - | "application"; + (typeof REVIEW_OPEN_FILES_IN_CHOICES)[number]; /** * Where Open file sends a file. `application` is one the reader picked with diff --git a/packages/review/src/ui-telemetry-events.ts b/packages/review/src/ui-telemetry-events.ts index d0251154b9..6b0afd8c25 100644 --- a/packages/review/src/ui-telemetry-events.ts +++ b/packages/review/src/ui-telemetry-events.ts @@ -21,6 +21,7 @@ import { REVIEW_COLOR_THEME_KINDS, REVIEW_DIFF_LAYOUTS, REVIEW_DIFF_LAYOUT_SCOPES, + REVIEW_OPEN_FILES_IN_CHOICES, isJsonObject, jsonBoolean, jsonNumber, @@ -93,7 +94,12 @@ export const LSP_FEATURE = [ export const LSP_VIA = ["command", "mouse"] as const; -export const LSP_EDITOR_KIND = ["files_tab", "inline_peek", "diff"] as const; +export const LSP_EDITOR_KIND = [ + "files_tab", + "inline_peek", + "diff", + "definition_peek", +] as const; export const LSP_LANGUAGE = [ "typescript", @@ -212,6 +218,16 @@ const MAP_LEVEL = ["system", "container", "component", "code"] as const; const SOURCE_TREE_OPENED_VIA = ["topbar", "home"] as const; +const SOURCE_OPENED_DESTINATION = [ + "source_window", + "vscode", + "cursor", + "zed", + "application", +] as const; + +const SOURCE_OPENED_KIND = ["file", "diff", "references", "tree"] as const; + /** Tab names the dwell beacon reports. Shared with its request parser. */ export const REVIEW_TELEMETRY_TABS = [ "review", @@ -431,6 +447,13 @@ export const UI_TELEMETRY_EVENTS = { event: "review_source_tree_opened", properties: { via: SOURCE_TREE_OPENED_VIA }, }, + source_opened: { + event: "review_source_opened", + properties: { + destination: SOURCE_OPENED_DESTINATION, + kind: SOURCE_OPENED_KIND, + }, + }, whiteboard_file_exported: { event: "review_whiteboard_file_exported", properties: {}, @@ -571,6 +594,7 @@ export const UI_TELEMETRY_EVENTS = { setting: SETTING_NAME, enabled: "boolean", value: REVIEW_COLOR_THEME_KINDS, + choice: REVIEW_OPEN_FILES_IN_CHOICES, }, }, review_opened: {