Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Issue Reporter API #180971

Merged
merged 3 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions src/vs/code/electron-sandbox/issue/IssueReporterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import { normalizeGitHubUrl } from 'vs/platform/issue/common/issueReporterUtil';
import { INativeHostService } from 'vs/platform/native/common/native';
import { applyZoom, zoomIn, zoomOut } from 'vs/platform/window/electron-sandbox/window';
import { CancellationError } from 'vs/base/common/errors';
import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';

// GitHub has let us know that we could up our limit here to 8k. We chose 7500 to play it safe.
// ref https://github.com/microsoft/vscode/issues/159191
Expand Down Expand Up @@ -91,15 +90,15 @@ export class IssueReporter extends Disposable {
}
}

this.issueMainService.getSystemInfo().then(info => {
this.issueMainService.$getSystemInfo().then(info => {
this.issueReporterModel.update({ systemInfo: info });
this.receivedSystemInfo = true;

this.updateSystemInfo(this.issueReporterModel.getData());
this.updatePreviewButtonState();
});
if (configuration.data.issueType === IssueType.PerformanceIssue) {
this.issueMainService.getPerformanceInfo().then(info => {
this.issueMainService.$getPerformanceInfo().then(info => {
this.updatePerformanceInfo(info as Partial<IssueReporterData>);
});
}
Expand Down Expand Up @@ -225,9 +224,9 @@ export class IssueReporter extends Disposable {
this.updateExtensionSelector(installedExtensions);
}

private async updateIssueReporterUri(extension: IssueReporterExtensionData, token: CancellationToken): Promise<void> {
private async updateIssueReporterUri(extension: IssueReporterExtensionData): Promise<void> {
try {
const uri = await this.issueMainService.getIssueReporterUri(extension.id, token);
const uri = await this.issueMainService.$getIssueReporterUri(extension.id);
extension.bugsUrl = uri.toString(true);
} catch (e) {
extension.hasIssueUriRequestHandler = false;
Expand All @@ -241,7 +240,7 @@ export class IssueReporter extends Disposable {
const issueType = parseInt((<HTMLInputElement>event.target).value);
this.issueReporterModel.update({ issueType: issueType });
if (issueType === IssueType.PerformanceIssue && !this.receivedPerformanceInfo) {
this.issueMainService.getPerformanceInfo().then(info => {
this.issueMainService.$getPerformanceInfo().then(info => {
this.updatePerformanceInfo(info as Partial<IssueReporterData>);
});
}
Expand Down Expand Up @@ -340,7 +339,7 @@ export class IssueReporter extends Disposable {
});

this.addEventListener('disableExtensions', 'click', () => {
this.issueMainService.reloadWithExtensionsDisabled();
this.issueMainService.$reloadWithExtensionsDisabled();
});

this.addEventListener('extensionBugsLink', 'click', (e: Event) => {
Expand All @@ -351,7 +350,7 @@ export class IssueReporter extends Disposable {
this.addEventListener('disableExtensions', 'keydown', (e: Event) => {
e.stopPropagation();
if ((e as KeyboardEvent).keyCode === 13 || (e as KeyboardEvent).keyCode === 32) {
this.issueMainService.reloadWithExtensionsDisabled();
this.issueMainService.$reloadWithExtensionsDisabled();
}
});

Expand All @@ -375,7 +374,7 @@ export class IssueReporter extends Disposable {
const { issueDescription } = this.issueReporterModel.getData();
if (!this.hasBeenSubmitted && (issueTitle || issueDescription)) {
// fire and forget
this.issueMainService.showConfirmCloseDialog();
this.issueMainService.$showConfirmCloseDialog();
} else {
this.close();
}
Expand Down Expand Up @@ -505,7 +504,7 @@ export class IssueReporter extends Disposable {
}

private async close(): Promise<void> {
await this.issueMainService.closeReporter();
await this.issueMainService.$closeReporter();
}

private clearSearchResults(): void {
Expand Down Expand Up @@ -882,7 +881,7 @@ export class IssueReporter extends Disposable {
}

private async writeToClipboard(baseUrl: string, issueBody: string): Promise<string> {
const shouldWrite = await this.issueMainService.showClipboardDialog();
const shouldWrite = await this.issueMainService.$showClipboardDialog();
if (!shouldWrite) {
throw new CancellationError();
}
Expand Down Expand Up @@ -1058,23 +1057,19 @@ export class IssueReporter extends Disposable {
const { selectedExtension } = this.issueReporterModel.getData();
reset(extensionsSelector, $<HTMLOptionElement>('option'), ...extensionOptions.map(extension => makeOption(extension, selectedExtension)));

let tokenSource: CancellationTokenSource | undefined;
this.addEventListener('extension-selector', 'change', (e: Event) => {
tokenSource?.cancel();
const selectedExtensionId = (<HTMLInputElement>e.target).value;
const extensions = this.issueReporterModel.getData().allExtensions;
const matches = extensions.filter(extension => extension.id === selectedExtensionId);
if (matches.length) {
this.issueReporterModel.update({ selectedExtension: matches[0] });
this.validateSelectedExtension();

if (matches[0].hasIssueUriRequestHandler) {
tokenSource = new CancellationTokenSource();
this.updateIssueReporterUri(matches[0], tokenSource?.token);
this.updateIssueReporterUri(matches[0]);
} else {
this.validateSelectedExtension();
const title = (<HTMLInputElement>this.getElementById('issue-title')).value;
this.searchExtensionIssues(title);
}

const title = (<HTMLInputElement>this.getElementById('issue-title')).value;
this.searchExtensionIssues(title);
} else {
this.issueReporterModel.update({ selectedExtension: undefined });
this.clearSearchResults();
Expand All @@ -1096,13 +1091,14 @@ export class IssueReporter extends Disposable {
hide(extensionValidationMessage);
hide(extensionValidationNoUrlsMessage);

if (!this.issueReporterModel.getData().selectedExtension) {
const extension = this.issueReporterModel.getData().selectedExtension;
if (!extension) {
this.previewButton.enabled = true;
return;
}

const hasValidGitHubUrl = this.getExtensionGitHubUrl();
if (hasValidGitHubUrl) {
if (hasValidGitHubUrl || extension.hasIssueUriRequestHandler) {
this.previewButton.enabled = true;
} else {
this.setExtensionValidationMessage();
Expand Down
15 changes: 7 additions & 8 deletions src/vs/platform/issue/common/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { CancellationToken } from 'vs/base/common/cancellation';
import { URI } from 'vs/base/common/uri';
import { ISandboxConfiguration } from 'vs/base/parts/sandbox/common/sandboxTypes';
import { PerformanceInfo, SystemInfo } from 'vs/platform/diagnostics/common/diagnostics';
Expand Down Expand Up @@ -124,11 +123,11 @@ export interface IIssueMainService {

// Used by the issue reporter

getSystemInfo(): Promise<SystemInfo>;
getPerformanceInfo(): Promise<PerformanceInfo>;
reloadWithExtensionsDisabled(): Promise<void>;
showConfirmCloseDialog(): Promise<void>;
showClipboardDialog(): Promise<boolean>;
getIssueReporterUri(extensionId: string, token: CancellationToken): Promise<URI>;
closeReporter(): Promise<void>;
$getSystemInfo(): Promise<SystemInfo>;
$getPerformanceInfo(): Promise<PerformanceInfo>;
$reloadWithExtensionsDisabled(): Promise<void>;
$showConfirmCloseDialog(): Promise<void>;
$showClipboardDialog(): Promise<boolean>;
$getIssueReporterUri(extensionId: string): Promise<URI>;
$closeReporter(): Promise<void>;
}
23 changes: 13 additions & 10 deletions src/vs/platform/issue/electron-main/issueMainService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { randomPath } from 'vs/base/common/extpath';
import { withNullAsUndefined } from 'vs/base/common/types';
import { IStateService } from 'vs/platform/state/node/state';
import { UtilityProcess } from 'vs/platform/utilityProcess/electron-main/utilityProcess';
import { CancellationToken } from 'vs/base/common/cancellation';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { URI } from 'vs/base/common/uri';
import { IWindowsMainService } from 'vs/platform/windows/electron-main/windows';
import { Promises, timeout } from 'vs/base/common/async';
Expand Down Expand Up @@ -302,13 +302,13 @@ export class IssueMainService implements IIssueMainService {

//#region used by issue reporter window

async getSystemInfo(): Promise<SystemInfo> {
async $getSystemInfo(): Promise<SystemInfo> {
const [info, remoteData] = await Promise.all([this.diagnosticsMainService.getMainDiagnostics(), this.diagnosticsMainService.getRemoteDiagnostics({ includeProcesses: false, includeWorkspaceMetadata: false })]);
const msg = await this.diagnosticsService.getSystemInfo(info, remoteData);
return msg;
}

async getPerformanceInfo(): Promise<PerformanceInfo> {
async $getPerformanceInfo(): Promise<PerformanceInfo> {
try {
const [info, remoteData] = await Promise.all([this.diagnosticsMainService.getMainDiagnostics(), this.diagnosticsMainService.getRemoteDiagnostics({ includeProcesses: true, includeWorkspaceMetadata: true })]);
return await this.diagnosticsService.getPerformanceInfo(info, remoteData);
Expand All @@ -319,7 +319,7 @@ export class IssueMainService implements IIssueMainService {
}
}

async reloadWithExtensionsDisabled(): Promise<void> {
async $reloadWithExtensionsDisabled(): Promise<void> {
if (this.issueReporterParentWindow) {
try {
await this.nativeHostMainService.reload(this.issueReporterParentWindow.id, { disableExtensions: true });
Expand All @@ -329,7 +329,7 @@ export class IssueMainService implements IIssueMainService {
}
}

async showConfirmCloseDialog(): Promise<void> {
async $showConfirmCloseDialog(): Promise<void> {
if (this.issueReporterWindow) {
const { response } = await this.dialogMainService.showMessageBox({
type: 'warning',
Expand All @@ -349,7 +349,7 @@ export class IssueMainService implements IIssueMainService {
}
}

async showClipboardDialog(): Promise<boolean> {
async $showClipboardDialog(): Promise<boolean> {
if (this.issueReporterWindow) {
const { response } = await this.dialogMainService.showMessageBox({
type: 'warning',
Expand All @@ -366,7 +366,7 @@ export class IssueMainService implements IIssueMainService {
return false;
}

async getIssueReporterUri(extensionId: string, token: CancellationToken): Promise<URI> {
async $getIssueReporterUri(extensionId: string): Promise<URI> {
if (!this.issueReporterParentWindow) {
throw new Error('Issue reporter window not available');
}
Expand All @@ -376,22 +376,25 @@ export class IssueMainService implements IIssueMainService {
}
const replyChannel = `vscode:triggerIssueUriRequestHandlerResponse${window.id}`;
return Promises.withAsyncBody<URI>(async (resolve, reject) => {
window.sendWhenReady('vscode:triggerIssueUriRequestHandler', token, { replyChannel, extensionId });

const cts = new CancellationTokenSource();
window.sendWhenReady('vscode:triggerIssueUriRequestHandler', cts.token, { replyChannel, extensionId });

validatedIpcMain.once(replyChannel, (_: unknown, data: string) => {
resolve(URI.parse(data));
});

try {
await timeout(5000, token);
await timeout(5000);
cts.cancel();
reject(new Error('Timed out waiting for issue reporter URI'));
} finally {
validatedIpcMain.removeHandler(replyChannel);
}
});
}

async closeReporter(): Promise<void> {
async $closeReporter(): Promise<void> {
this.issueReporterWindow?.close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class NativeIssueService implements IWorkbenchIssueService {
version: manifest.version,
repositoryUrl: manifest.repository && manifest.repository.url,
bugsUrl: manifest.bugs && manifest.bugs.url,
hasIssueUriRequestHandler: this._handlers.has(extension.identifier.id),
hasIssueUriRequestHandler: this._handlers.has(extension.identifier.id.toLowerCase()),
displayName: manifest.displayName,
id: extension.identifier.id,
isTheme,
Expand Down Expand Up @@ -145,7 +145,7 @@ export class NativeIssueService implements IWorkbenchIssueService {
}

registerIssueUriRequestHandler(extensionId: string, handler: IIssueUriRequestHandler): IDisposable {
this._handlers.set(extensionId, handler);
this._handlers.set(extensionId.toLowerCase(), handler);
return {
dispose: () => this._handlers.delete(extensionId)
};
Expand Down
14 changes: 14 additions & 0 deletions src/vscode-dts/vscode.proposed.handleIssueUri.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,24 @@ declare module 'vscode' {
// https://github.com/microsoft/vscode/issues/46726

export interface IssueUriRequestHandler {
/**
*Handle the request by the issue reporter for the Uri you want to direct the user to.
*/
handleIssueUrlRequest(): ProviderResult<Uri>;
}

export namespace env {
/**
* Register an {@link IssueUriRequestHandler}. By registering an issue uri request handler,
* you can direct the built-in issue reporter to your issue reporting web experience of choice.
* The Uri that the handler returns will be opened in the user's browser.
*
* Examples of this include:
* - Using GitHub Issue Forms or GitHub Discussions you can pre-fill the issue creation with relevant information from the current workspace using query parameters
* - Directing to a different web form that isn't on GitHub for reporting issues
*
* @param handler the issue uri request handler to register for this extension.
*/
export function registerIssueUriRequestHandler(handler: IssueUriRequestHandler): Disposable;
}
}