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

Allow additional extension data from Issue Reporter API #196103

Merged
merged 9 commits into from
Oct 23, 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
16 changes: 8 additions & 8 deletions src/vs/code/electron-sandbox/issue/issueReporterMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import 'vs/css!./media/issueReporter';
import 'vs/base/browser/ui/codicons/codiconStyles'; // make sure codicon css is loaded
import { safeInnerHtml } from 'vs/base/browser/dom';
import 'vs/base/browser/ui/codicons/codiconStyles'; // make sure codicon css is loaded
import { isLinux, isWindows } from 'vs/base/common/platform';
import BaseHtml from 'vs/code/electron-sandbox/issue/issueReporterPage';
import 'vs/css!./media/issueReporter';
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { getSingletonServiceDescriptors } from 'vs/platform/instantiation/common/extensions';
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
import { IMainProcessService } from 'vs/platform/ipc/common/mainProcessService';
import { ElectronIPCMainProcessService } from 'vs/platform/ipc/electron-sandbox/mainProcessService';
import { registerMainProcessRemoteService } from 'vs/platform/ipc/electron-sandbox/services';
import { IIssueMainService, IssueReporterWindowConfiguration } from 'vs/platform/issue/common/issue';
import { INativeHostService } from 'vs/platform/native/common/native';
import { NativeHostService } from 'vs/platform/native/electron-sandbox/nativeHostService';
import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { IMainProcessService } from 'vs/platform/ipc/common/mainProcessService';
import { IssueReporter } from './issueReporterService';
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
import { getSingletonServiceDescriptors } from 'vs/platform/instantiation/common/extensions';
import { registerMainProcessRemoteService } from 'vs/platform/ipc/electron-sandbox/services';

export function startup(configuration: IssueReporterWindowConfiguration) {
const platformClass = isWindows ? 'windows' : isLinux ? 'linux' : 'mac';
Expand Down
13 changes: 13 additions & 0 deletions src/vs/code/electron-sandbox/issue/issueReporterModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ISettingSearchResult, IssueReporterExtensionData, IssueType } from 'vs/
export interface IssueReporterData {
issueType: IssueType;
issueDescription?: string;
extensionData?: string;

versionInfo?: any;
systemInfo?: SystemInfo;
Expand All @@ -20,6 +21,7 @@ export interface IssueReporterData {
includeProcessInfo: boolean;
includeExtensions: boolean;
includeExperiments: boolean;
includeExtensionData: boolean;

numberOfThemeExtesions?: number;
allExtensions: IssueReporterExtensionData[];
Expand Down Expand Up @@ -47,6 +49,7 @@ export class IssueReporterModel {
includeProcessInfo: true,
includeExtensions: true,
includeExperiments: true,
includeExtensionData: true,
allExtensions: []
};

Expand Down Expand Up @@ -120,6 +123,12 @@ ${this.getInfos()}
private getInfos(): string {
let info = '';

if (this._data.issueType === IssueType.Bug || this._data.issueType === IssueType.PerformanceIssue) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

man this function makes my head hurt. So many ifs... we should clean this up in debt week.

if (!this._data.fileOnMarketplace && this._data.includeExtensionData && this._data.extensionData) {
info += this.getExtensionData();
}
}

if (this._data.issueType === IssueType.Bug || this._data.issueType === IssueType.PerformanceIssue) {
if (!this._data.fileOnMarketplace && this._data.includeSystemInfo && this._data.systemInfo) {
info += this.generateSystemInfoMd();
Expand Down Expand Up @@ -152,6 +161,10 @@ ${this.getInfos()}
return info;
}

private getExtensionData(): string {
return this._data.extensionData ?? '';
}

private generateSystemInfoMd(): string {
let md = `<details>
<summary>System Info</summary>
Expand Down
13 changes: 13 additions & 0 deletions src/vs/code/electron-sandbox/issue/issueReporterPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const sendProcessInfoLabel = escape(localize('sendProcessInfo', "Include my curr
const sendWorkspaceInfoLabel = escape(localize('sendWorkspaceInfo', "Include my workspace metadata"));
const sendExtensionsLabel = escape(localize('sendExtensions', "Include my enabled extensions"));
const sendExperimentsLabel = escape(localize('sendExperiments', "Include A/B experiment info"));
const sendExtensionData = escape(localize('sendExtensionData', "Include Additional Extension info"));
const reviewGuidanceLabel = localize( // intentionally not escaped because of its embedded tags
{
key: 'reviewGuidanceLabel',
Expand Down Expand Up @@ -85,6 +86,18 @@ export default (): string => `
</div>

<div class="system-info" id="block-container">
<div class="block block-extension-data">
<input class="send-extension-data" aria-label="${sendExtensionData}" type="checkbox" id="includeExtensionData" checked/>
<label class="extension-caption" id="extension-caption" for="includeExtensionData">
${sendExtensionData}
<span id="ext-loading" hidden></span>
<span class="ext-parens" hidden>(</span><a href="#" class="showInfo" id="extension-id">${escape(localize('show', "show"))}</a><span class="ext-parens" hidden>)</span>
</label>
<div class="block-info hidden">
<textarea name="extension-data" id="extension-data" placeholder="${escape(localize('extensionData', "Extension does not have additional data to include."))}"></textarea>
</div>
</div>

<div class="block block-system">
<input class="sendData" aria-label="${sendSystemInfoLabel}" type="checkbox" id="includeSystemInfo" checked/>
<label class="caption" for="includeSystemInfo">
Expand Down
124 changes: 116 additions & 8 deletions src/vs/code/electron-sandbox/issue/issueReporterService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,26 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { localize } from 'vs/nls';
import { $, reset, windowOpenNoOpener } from 'vs/base/browser/dom';
import { $, createStyleSheet, reset, windowOpenNoOpener } from 'vs/base/browser/dom';
import { Button, unthemedButtonStyles } from 'vs/base/browser/ui/button/button';
import { renderIcon } from 'vs/base/browser/ui/iconLabel/iconLabels';
import { Delayer } from 'vs/base/common/async';
import { Delayer, RunOnceScheduler } from 'vs/base/common/async';
import { Codicon } from 'vs/base/common/codicons';
import { groupBy } from 'vs/base/common/collections';
import { debounce } from 'vs/base/common/decorators';
import { CancellationError } from 'vs/base/common/errors';
import { Disposable } from 'vs/base/common/lifecycle';
import { isLinuxSnap, isMacintosh } from 'vs/base/common/platform';
import { escape } from 'vs/base/common/strings';
import { IssueReporterData as IssueReporterModelData, IssueReporterModel } from 'vs/code/electron-sandbox/issue/issueReporterModel';
import { ThemeIcon } from 'vs/base/common/themables';
import { IssueReporterModel, IssueReporterData as IssueReporterModelData } from 'vs/code/electron-sandbox/issue/issueReporterModel';
import { localize } from 'vs/nls';
import { isRemoteDiagnosticError } from 'vs/platform/diagnostics/common/diagnostics';
import { IIssueMainService, IssueReporterData, IssueReporterExtensionData, IssueReporterStyles, IssueReporterWindowConfiguration, IssueType } from 'vs/platform/issue/common/issue';
import { normalizeGitHubUrl } from 'vs/platform/issue/common/issueReporterUtil';
import { INativeHostService } from 'vs/platform/native/common/native';
import { getIconsStyleSheet } from 'vs/platform/theme/browser/iconsStyleSheet';
import { applyZoom, zoomIn, zoomOut } from 'vs/platform/window/electron-sandbox/window';
import { CancellationError } from 'vs/base/common/errors';

// 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 All @@ -41,11 +43,11 @@ export class IssueReporter extends Disposable {
private readonly issueReporterModel: IssueReporterModel;
private numberOfSearchResultsDisplayed = 0;
private receivedSystemInfo = false;
private receivedExtensionData = false;
private receivedPerformanceInfo = false;
private shouldQueueSearch = false;
private hasBeenSubmitted = false;
private delayedSubmit = new Delayer<void>(300);

private readonly previewButton!: Button;

constructor(
Expand All @@ -67,6 +69,8 @@ export class IssueReporter extends Disposable {
selectedExtension: targetExtension
});

//TODO: Handle case where extension is not activated

const issueReporterElement = this.getElementById('issue-reporter');
if (issueReporterElement) {
this.previewButton = new Button(issueReporterElement, unthemedButtonStyles);
Expand Down Expand Up @@ -107,6 +111,19 @@ export class IssueReporter extends Disposable {
show(this.getElementById('english'));
}

const codiconStyleSheet = createStyleSheet();
codiconStyleSheet.id = 'codiconStyles';

// TODO: Is there a way to use the IThemeService here instead
const iconsStyleSheet = getIconsStyleSheet(undefined);
function updateAll() {
codiconStyleSheet.textContent = iconsStyleSheet.getCSS();
}

const delayer = new RunOnceScheduler(updateAll, 0);
iconsStyleSheet.onDidChange(() => delayer.schedule());
delayer.schedule();

this.setUpTypes();
this.setEventHandlers();
applyZoom(configuration.data.zoomLevel);
Expand Down Expand Up @@ -235,6 +252,30 @@ export class IssueReporter extends Disposable {
}
}

private async getIssueDataFromExtension(extension: IssueReporterExtensionData): Promise<string> {
try {
const data = await this.issueMainService.$getIssueReporterData(extension.id);
extension.extensionData = data;
this.receivedExtensionData = true;
return data;
} catch (e) {
extension.hasIssueDataProviders = false;
// The issue handler failed so fall back to old issue reporter experience.
this.renderBlocks();
throw e;
}
}

private async getIssueTemplateFromExtension(extension: IssueReporterExtensionData): Promise<string> {
try {
const data = await this.issueMainService.$getIssueReporterTemplate(extension.id);
extension.extensionTemplate = data;
return data;
} catch (e) {
throw e;
}
}

private setEventHandlers(): void {
this.addEventListener('issue-type', 'change', (event: Event) => {
const issueType = parseInt((<HTMLInputElement>event.target).value);
Expand All @@ -249,7 +290,7 @@ export class IssueReporter extends Disposable {
this.render();
});

(['includeSystemInfo', 'includeProcessInfo', 'includeWorkspaceInfo', 'includeExtensions', 'includeExperiments'] as const).forEach(elementId => {
(['includeSystemInfo', 'includeProcessInfo', 'includeWorkspaceInfo', 'includeExtensions', 'includeExperiments', 'includeExtensionData'] as const).forEach(elementId => {
this.addEventListener(elementId, 'click', (event: Event) => {
event.stopPropagation();
this.issueReporterModel.update({ [elementId]: !this.issueReporterModel.getData()[elementId] });
Expand Down Expand Up @@ -428,6 +469,11 @@ export class IssueReporter extends Disposable {

private isPreviewEnabled() {
const issueType = this.issueReporterModel.getData().issueType;

if (this.issueReporterModel.getData().selectedExtension?.hasIssueDataProviders && !this.receivedExtensionData) {
return false;
}

if (issueType === IssueType.Bug && this.receivedSystemInfo) {
return true;
}
Expand All @@ -453,6 +499,10 @@ export class IssueReporter extends Disposable {
return selectedExtension && selectedExtension.bugsUrl;
}

private getExtensionData(): string | undefined {
return this.issueReporterModel.getData().selectedExtension?.extensionData;
}

private searchVSCodeIssues(title: string, issueDescription?: string): void {
if (title) {
this.searchDuplicates(title, issueDescription);
Expand Down Expand Up @@ -692,6 +742,7 @@ export class IssueReporter extends Disposable {
const workspaceBlock = document.querySelector('.block-workspace');
const extensionsBlock = document.querySelector('.block-extensions');
const experimentsBlock = document.querySelector('.block-experiments');
const extensionDataBlock = document.querySelector('.block-extension-data');

const problemSource = this.getElementById('problem-source')!;
const descriptionTitle = this.getElementById('issue-description-label')!;
Expand All @@ -700,6 +751,7 @@ export class IssueReporter extends Disposable {

const titleTextArea = this.getElementById('issue-title-container')!;
const descriptionTextArea = this.getElementById('description')!;
const extensionDataTextArea = this.getElementById('extension-data')!;

// Hide all by default
hide(blockContainer);
Expand All @@ -710,6 +762,8 @@ export class IssueReporter extends Disposable {
hide(experimentsBlock);
hide(problemSource);
hide(extensionSelector);
hide(extensionDataTextArea);
hide(extensionDataBlock);

show(problemSource);
show(titleTextArea);
Expand All @@ -728,6 +782,16 @@ export class IssueReporter extends Disposable {
return;
}

if (fileOnExtension && selectedExtension?.hasIssueDataProviders) {
const data = this.getExtensionData();
if (data) {
(extensionDataTextArea as HTMLTextAreaElement).value = data.toString();
}
(extensionDataTextArea as HTMLTextAreaElement).readOnly = true;
show(extensionDataBlock);
show(extensionDataTextArea);
}

if (issueType === IssueType.Bug) {
if (!fileOnMarketplace) {
show(blockContainer);
Expand Down Expand Up @@ -1061,14 +1125,27 @@ export class IssueReporter extends Disposable {
extensionsSelector.selectedIndex = 0;
}

this.addEventListener('extension-selector', 'change', (e: Event) => {
this.addEventListener('extension-selector', 'change', async (e: Event) => {
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] });
if (matches[0].hasIssueUriRequestHandler) {
this.updateIssueReporterUri(matches[0]);
} else if (matches[0].hasIssueDataProviders) {
const template = await this.getIssueTemplateFromExtension(matches[0]);
const descriptionTextArea = this.getElementById('description')!;
const fullTextArea = (descriptionTextArea as HTMLTextAreaElement).value += template;
this.issueReporterModel.update({ issueDescription: fullTextArea });

const extensionDataBlock = document.querySelector('.block-extension-data')!;
show(extensionDataBlock);

// Start loading for extension data.
this.setLoading();
await this.getIssueDataFromExtension(matches[0]);
this.removeLoading();
} else {
this.validateSelectedExtension();
const title = (<HTMLInputElement>this.getElementById('issue-title')).value;
Expand Down Expand Up @@ -1110,6 +1187,37 @@ export class IssueReporter extends Disposable {
}
}

private setLoading() {
// Show loading
this.updatePreviewButtonState();

const extensionDataCaption = this.getElementById('extension-id')!;
hide(extensionDataCaption);

const extensionDataCaption2 = Array.from(document.querySelectorAll('.ext-parens'));
extensionDataCaption2.forEach(extensionDataCaption2 => hide(extensionDataCaption2));

const showLoading = this.getElementById('ext-loading')!;
show(showLoading);

const iconElement = document.createElement('span');
iconElement.classList.add(...ThemeIcon.asClassNameArray(Codicon.loading), 'codicon-modifier-spin');
showLoading.append(iconElement);
}

private removeLoading() {
this.updatePreviewButtonState();

const extensionDataCaption = this.getElementById('extension-id')!;
show(extensionDataCaption);

const extensionDataCaption2 = Array.from(document.querySelectorAll('.ext-parens'));
extensionDataCaption2.forEach(extensionDataCaption2 => show(extensionDataCaption2));

const hideLoading = this.getElementById('ext-loading')!;
hide(hideLoading);
}

private setExtensionValidationMessage(): void {
const extensionValidationMessage = this.getElementById('extension-selection-validation-error')!;
const extensionValidationNoUrlsMessage = this.getElementById('extension-selection-validation-error-no-url')!;
Expand Down
5 changes: 5 additions & 0 deletions src/vs/code/electron-sandbox/issue/media/issueReporter.css
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ html {
height: 100%;
}

.extension-caption .codicon-modifier-spin {
padding-bottom: 3px;
margin-left: 2px;
}

/* Font Families (with CJK support) */

.mac { font-family: -apple-system, BlinkMacSystemFont, sans-serif; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ suite('IssueReporter', () => {
assert.deepStrictEqual(issueReporterModel.getData(), {
allExtensions: [],
includeSystemInfo: true,
includeExtensionData: true,
includeWorkspaceInfo: true,
includeProcessInfo: true,
includeExtensions: true,
Expand Down
5 changes: 5 additions & 0 deletions src/vs/platform/issue/common/issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ export interface IssueReporterExtensionData {
displayName: string | undefined;
repositoryUrl: string | undefined;
bugsUrl: string | undefined;
extensionData?: string;
extensionTemplate?: string;
hasIssueUriRequestHandler?: boolean;
hasIssueDataProviders?: boolean;
}

export interface IssueReporterData extends WindowData {
Expand Down Expand Up @@ -128,5 +131,7 @@ export interface IIssueMainService {
$showConfirmCloseDialog(): Promise<void>;
$showClipboardDialog(): Promise<boolean>;
$getIssueReporterUri(extensionId: string): Promise<URI>;
$getIssueReporterData(extensionId: string): Promise<string>;
$getIssueReporterTemplate(extensionId: string): Promise<string>;
$closeReporter(): Promise<void>;
}
Loading