Add ability to ignore recommendations both globally and at a per-workspace level #51941

JacksonKearl · 2018-06-14T23:17:30Z

Resolves #48743.

…ense

ramya-rao-a

Review Phase 1 is done. I'll get back to this in an hr.

src/vs/workbench/parts/extensions/common/extensions.ts

@@ -74,6 +74,7 @@ export const IExtensionsWorkbenchService = createDecorator<IExtensionsWorkbenchS
 export interface IExtensionsWorkbenchService {
 	_serviceBrand: any;
 	onChange: Event<void>;
+	onRecommendationChange: Event<void>;


src/vs/workbench/parts/extensions/browser/extensionsList.ts

+				data.root.setAttribute('aria-label', extension.displayName + '. ' + extRecommendations[extension.id]);
+				data.root.title = extRecommendations[extension.id.toLowerCase()].reasonText;
+				addClass(data.root, 'recommended');
+			}


src/vs/workbench/parts/extensions/common/extensionsFileTemplate.ts

@@ -16,7 +16,16 @@ export const ExtensionsConfigurationSchema: IJSONSchema = {
 	properties: {
 		recommendations: {
 			type: 'array',
-			description: localize('app.extensions.json.recommendations', "List of extensions recommendations. The identifier of an extension is always '${publisher}.${name}'. For example: 'vscode.csharp'."),
+			description: localize('app.extensions.json.recommendations', "List of extensions which should be recommended for users of this repository. The identifier of an extension is always '${publisher}.${name}'. For example: 'vscode.csharp'."),


src/vs/workbench/parts/extensions/common/extensionsFileTemplate.ts

+		},
+		unwantedRecommendations: {
+			type: 'array',
+			description: localize('app.extensions.json.unwantedRecommendations', "List of extensions which are irrelevant, redundant, or otherwise unwanted, and should not be recommended for users of this repository. The identifier of an extension is always '${publisher}.${name}'. For example: 'vscode.csharp'."),


src/vs/workbench/parts/extensions/browser/extensionsActions.ts

@@ -544,6 +544,37 @@ export class EnableGloballyAction extends Action implements IExtensionAction {
 	}
 }

+export class IgnoreAction extends Action {


src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

@@ -369,6 +392,22 @@ export class ExtensionEditor extends BaseEditor {
 		this.extensionActionBar.push([disabledStatusAction, reloadAction, updateAction, enableAction, disableAction, installAction, maliciousStatusAction], { icon: true, label: true });
 		this.transientDisposables.push(enableAction, updateAction, reloadAction, disableAction, installAction, maliciousStatusAction, disabledStatusAction);

+		const ignoreAction = this.instantiationService.createInstance(IgnoreAction);
+		ignoreAction.extension = extension;
+		ignoreAction.onIgnored = () => {


src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

+			this.recommendationText.textContent = localize('recommendationHasBeenIgnoredGlobal', "You have chosen not to receive recommendations for this extension.");
+		}
+		if (ignoredRecommendations.workspace.indexOf(extension.id.toLowerCase()) !== -1) {
+			this.recommendationText.textContent = localize('recommendationHasBeenIgnoredWorkspace', "This extension has been marked as redundant or irrelevant by users of the current workspace.");


src/vs/platform/extensionManagement/common/extensionManagement.ts

+	recommendations: string[];
+	unwantedRecommendations: string[];
+}
+


src/vs/platform/extensionManagement/common/extensionManagement.ts

@@ -374,6 +384,8 @@ export interface IExtensionTipsService {
 	getKeymapRecommendations(): string[];
 	getKeywordsForExtension(extension: string): string[];
 	getRecommendationsForExtension(extension: string): string[];
+	refreshAllIgnoredRecommendations(): TPromise<void>;
+	getAllIgnoredRecommendations(): IIgnoredRecommendations;


src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

+		const ignoredRecommendations = this.extensionTipsService.getAllIgnoredRecommendations();
+
+		toggleClass(this.header, 'ignored', this.recentlyIgnored.indexOf(extension.id.toLowerCase()) !== -1 || ignoredRecommendations.workspace.indexOf(extension.id.toLowerCase()) !== -1);
+		if (this.recentlyIgnored.indexOf(extension.id.toLowerCase()) !== -1) {


ramya-rao-a · 2018-06-15T18:10:17Z

src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

 		*/
 		this.telemetryService.publicLog('extensionGallery:openExtension', assign(extension.telemetryData, recommendationsData));

+		const ignoredRecommendations = this.extensionTipsService.getAllIgnoredRecommendations();
+
+		toggleClass(this.header, 'ignored', this.recentlyIgnored.indexOf(extension.id.toLowerCase()) !== -1 || ignoredRecommendations.workspace.indexOf(extension.id.toLowerCase()) !== -1);


Do we really need to add the ignored class?
From what I can see in the css file, its used in the same situation where the recommended label is used.

It stylizes the text differently and removes the ignore button

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

+		return cleansedIDs;
+	}
+
+	refreshAllIgnoredRecommendations(): TPromise<void> {


src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

+			});
+
+			this._exeBasedRecommendations = filteredExeBased;
+			this._fileBasedRecommendations = filteredFileBased;


…arl/vscode into feature/un-recomend-extensions

ramya-rao-a · 2018-06-19T05:41:06Z

src/vs/workbench/parts/extensions/common/extensionsFileTemplate.ts

+	'\t\t',
+	'\t],',
+	'\t"unwantedRecommendations": [',
+	'\t\t// List of extensions which should not be recommended for users of this workspace. These extensions may be irrelevant, redundant, or otherwise unwanted.',


A literal translation of this would mean that we are asking users to list ALL the extensions that they would consider irrelevant/redundant etc.

How about..

List of extensions that will be skipped from the recommendations that VS Code makes for the users of this workspace... ?

ramya-rao-a · 2018-06-19T05:41:57Z

src/vs/workbench/parts/extensions/electron-browser/extensionEditor.ts

@@ -295,24 +306,25 @@ export class ExtensionEditor extends BaseEditor {
 		this.publisher.textContent = extension.publisherDisplayName;
 		this.description.textContent = extension.description;

+		removeClass(this.header, 'ignored');


ignored can be recommendationIgnored?

ramya-rao-a · 2018-06-19T05:47:00Z

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

+			.then(contents => this.processConfigContent(this.mergeExtensionRecommendationConfigs(flatten(contents))));
+	}
+
+	private resolveWorkspaceExtensionConfig(workspace: IWorkspace): TPromise<IExtensionsConfigContent[]> {


Why are both resolveWorkspaceExtensionConfig and resolveWorkspaceFolderExtensionConfig returning array of IExtensionsConfigContent? They both work on individual files... and so wouldn't the output be just `TPromise< IExtensionsConfigContent>?

I moved it back to maybe being represented by empty array or array of one rather than merging all over the place. what do you think?

ramya-rao-a · 2018-06-19T05:51:01Z

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

+	private fetchCombinedExtensionRecommendationConfig(): TPromise<IExtensionsConfigContent> {
+		const workspace = this.contextService.getWorkspace();
+		return TPromise.join([this.resolveWorkspaceExtensionConfig(workspace), ...workspace.folders.map(workspaceFolder => this.resolveWorkspaceFolderExtensionConfig(workspaceFolder))])
+			.then(contents => this.processConfigContent(this.mergeExtensionRecommendationConfigs(flatten(contents))));


Looks like now, this is the only place from where we call mergeExtensionRecommendationConfigs, in which case, how about merging its code into processConfigContent?

ramya-rao-a · 2018-06-19T05:53:19Z

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts

@@ -555,86 +636,84 @@ export class ExtensionTipsService extends Disposable implements IExtensionTipsSe
 		});
 	}

-	private _suggestWorkspaceRecommendations(): TPromise<any> {
+	private _suggestWorkspaceRecommendations(allRecommendations: string[]): void {


Does this parameter need to be passed in? Can we not use this._allWorkspaceRecommendedExtensions directly?

…arl/vscode into feature/un-recomend-extensions

Jackson Kearl added 19 commits June 11, 2018 17:23

WIP

d30415f

Initail filtering and pulling filter list logic.

76472b7

Imporive typing and naming

58e1eed

Remove the significant duplication

6803fc1

Bug fixes.

e9669f4

Fix bug where arrow functions dont maintain this in point free style

42e9a32

WIP on extension ignoring UI.

828fe0a

UI for global ignores.

7c6c8fa

Add "unwantedRecommendations" to extensions.json template and intelis…

0202014

…ense

Notify of workspace non-recommended extensions

138f45a

Wording of extensions.json template

eb876ac

More UI for ignore button.

a711169

Use seprate notification channel of recommendation changes.

f287d7f

Reload search when recommended extensions changes

08df8e0

Tests for ExtensionTipsService

d9748db

Test extensions workbench service

019f2ac

Merge branch 'master' into feature/un-recomend-extensions

75fddb9

Naming and add default vaule to workspace settings

7033d13

Merge branch 'master' into feature/un-recomend-extensions

a9863f1

JacksonKearl assigned JacksonKearl and ramya-rao-a Jun 14, 2018

JacksonKearl requested a review from ramya-rao-a June 14, 2018 23:17

nzec approved these changes Jun 15, 2018

View reviewed changes

ramya-rao-a suggested changes Jun 15, 2018

View reviewed changes

ramya-rao-a reviewed Jun 15, 2018

View reviewed changes

src/vs/platform/extensionManagement/common/extensionManagement.ts

recommendations: string[];

unwantedRecommendations: string[];

}

This comment was marked as resolved.

Sign in to view

ramya-rao-a reviewed Jun 15, 2018

View reviewed changes

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts Outdated

return cleansedIDs;

}

refreshAllIgnoredRecommendations(): TPromise<void> {

This comment was marked as resolved.

Sign in to view

ramya-rao-a reviewed Jun 15, 2018

View reviewed changes

src/vs/workbench/parts/extensions/electron-browser/extensionTipsService.ts Outdated

});

this._exeBasedRecommendations = filteredExeBased;

this._fileBasedRecommendations = filteredFileBased;

This comment was marked as resolved.

Sign in to view

ramya-rao-a and others added 12 commits June 18, 2018 11:49

Rename id to extensionId in RecommendationChangeNotification

cc2fe62

updateRecommendedness ??

bf1eea8

Not needed

0ddd992

Remove point free style thing

1421677

Simplify

f520130

Merge branch 'feature/un-recomend-extensions' of github.com:JacksonKe…

3e6310f

…arl/vscode into feature/un-recomend-extensions

naming

956c8ab

remove extrenous getWrokspaceRecommendations call

68ee832

Casing

3e8a0e1

Gracefull handle missing 'extensions' field in multiroot project config

374e49d

Refresh recommendation views on recommendation change

01a4913

more case sensitivity stuff

312cf84

ramya-rao-a reviewed Jun 19, 2018

View reviewed changes

Jackson Kearl and others added 7 commits June 19, 2018 09:30

naming/refactroing

c217a29

Merge branch 'master' into feature/un-recomend-extensions

2fc3625

Simplify return types

ba26860

Merge branch 'feature/un-recomend-extensions' of github.com:JacksonKe…

b8438bf

…arl/vscode into feature/un-recomend-extensions

Wording

ab52be6

Add telemetry

0a60392

guard telemetry service

b77ba77

ramya-rao-a approved these changes Jun 19, 2018

View reviewed changes

ramya-rao-a merged commit 442e5e2 into microsoft:master Jun 19, 2018

JacksonKearl mentioned this pull request Jun 19, 2018

Allow hiding some recommended extensions #48743

Closed

ramya-rao-a mentioned this pull request Jun 22, 2018

Error when running from sources #52440

Closed

github-actions bot locked and limited conversation to collaborators Mar 27, 2020

github-actions bot unassigned ramya-rao-a Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to ignore recommendations both globally and at a per-workspace level #51941

Add ability to ignore recommendations both globally and at a per-workspace level #51941

JacksonKearl commented Jun 14, 2018 •

edited

Loading

ramya-rao-a left a comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ramya-rao-a Jun 15, 2018

JacksonKearl Jun 15, 2018

This comment was marked as resolved.

This comment was marked as resolved.

ramya-rao-a Jun 19, 2018

ramya-rao-a Jun 19, 2018

ramya-rao-a Jun 19, 2018

JacksonKearl Jun 19, 2018

ramya-rao-a Jun 19, 2018

ramya-rao-a Jun 19, 2018

Add ability to ignore recommendations both globally and at a per-workspace level #51941

Add ability to ignore recommendations both globally and at a per-workspace level #51941

Conversation

JacksonKearl commented Jun 14, 2018 • edited Loading

ramya-rao-a left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacksonKearl commented Jun 14, 2018 •

edited

Loading