Git - Unsafe repositories flow polish

extensions/git/package.json

@@ -625,13 +625,8 @@
         "enablement": "isMergeEditor"
       },
       {
-        "command": "git.addSafeDirectoryAndOpenRepository",
-        "title": "%command.addSafeDirectoryAndOpenRepository%",
-        "category": "Git"
-      },
-      {
-        "command": "git.api.getUnsafeRepositories",
-        "title": "%command.api.getUnsafeRepositories%",
+        "command": "git.manageUnsafeRepositories",
+        "title": "%command.manageUnsafeRepositories%",
         "category": "Git"
       }
     ],
@@ -1057,10 +1052,6 @@
           "command": "git.api.getRemoteSources",
           "when": "false"
         },
-        {
-          "command": "git.api.getUnsafeRepositories",
-          "when": "false"
-        },
         {
           "command": "git.openMergeEditor",
           "when": "false"

extensions/git/package.nls.json

@@ -99,11 +99,10 @@
 	"command.timelineCopyCommitMessage": "Copy Commit Message",
 	"command.timelineSelectForCompare": "Select for Compare",
 	"command.timelineCompareWithSelected": "Compare with Selected",
-	"command.addSafeDirectoryAndOpenRepository": "Mark Repository as Safe and Open",
+	"command.manageUnsafeRepositories": "Manage Unsafe Repositories",
 	"command.api.getRepositories": "Get Repositories",
 	"command.api.getRepositoryState": "Get Repository State",
 	"command.api.getRemoteSources": "Get Remote Sources",
-	"command.api.getUnsafeRepositories": "Get Unsafe Repositories",
 	"command.git.acceptMerge": "Complete Merge",
 	"command.git.openMergeEditor": "Resolve in Merge Editor",
 	"command.git.runGitMerge": "Compute Conflicts With Git",
@@ -331,19 +330,17 @@
 		"message": "Scanning workspace for git repositories..."
 	},
 	"view.workbench.scm.unsafeRepository": {
-		"message": "The git repository in the following folder is potentially unsafe as the folder is owned by someone other than the current user: ${command:git.api.getUnsafeRepositories}.\nDo you want to open the repository?\n[Open Repository](command:git.addSafeDirectoryAndOpenRepository)\n[Learn More](https://aka.ms/vscode-scm)",
+		"message": "The detected git repository is potentially unsafe as the folder is owned by someone other than the current user.\n[Manage Unsafe Repositories](command:git.manageUnsafeRepositories)\nTo learn more about unsafe repositories [read our docs](https://aka.ms/vscode-scm).",
 		"comment": [
-			"{Locked='](command:git.api.getUnsafeRepositories'}",
-			"{Locked='](command:git.addSafeDirectoryAndOpenRepository'}",
+			"{Locked='](command:git.manageUnsafeRepositories'}",
 			"Do not translate the 'command:*' part inside of the '(..)'. It is an internal command syntax for VS Code",
 			"Please make sure there is no space between the right bracket and left parenthesis:  ]( this is an internal syntax for links"
 		]
 	},
 	"view.workbench.scm.unsafeRepositories": {
-		"message": "The git repositories in the following folders are potentially unsafe as the folders are owned by someone other than the current user: ${command:git.api.getUnsafeRepositories}.\nDo you want to open the repositories?\n[Open Repositories](command:git.addSafeDirectoryAndOpenRepository)\n[Learn More](https://aka.ms/vscode-scm)",
+		"message": "The detected git repositories are potentially unsafe as the folders are owned by someone other than the current user.\n[Manage Unsafe Repositories](command:git.manageUnsafeRepositories)\nTo learn more about unsafe repositories [read our docs](https://aka.ms/vscode-scm).",
 		"comment": [
-			"{Locked='](command:git.api.getUnsafeRepositories'}",
-			"{Locked='](command:git.addSafeDirectoryAndOpenRepository'}",
+			"{Locked='](command:git.manageUnsafeRepositories'}",
 			"Do not translate the 'command:*' part inside of the '(..)'. It is an internal command syntax for VS Code",
 			"Please make sure there is no space between the right bracket and left parenthesis:  ]( this is an internal syntax for links"
 		]

extensions/git/src/commands.ts

@@ -194,6 +194,14 @@ class FetchAllRemotesItem implements QuickPickItem {
 	}
 }
 
+class UnsafeRepositoryItem implements QuickPickItem {
+	get label(): string {
+		return `$(repo) ${this.path}`;
+	}
+
+	constructor(public readonly path: string) { }
+}
+
 interface ScmCommandOptions {
 	repository?: boolean;
 	diff?: boolean;
@@ -3182,15 +3190,49 @@ export class CommandCenter {
 		repository.closeDiffEditors(undefined, undefined, true);
 	}
 
-	@command('git.api.getUnsafeRepositories')
-	getUnsafeRepositories(): string {
-		const repositories = Array.from(this.model.unsafeRepositories.values());
-		return repositories.sort().map(m => `"${m}"`).join(', ');
-	}
+	@command('git.manageUnsafeRepositories')
+	async manageUnsafeRepositories(): Promise<void> {
+		const unsafeRepositories: string[] = [];
 
-	@command('git.addSafeDirectoryAndOpenRepository')
-	async addSafeDirectoryAndOpenRepository(): Promise<void> {
-		await this.model.addSafeDirectoryAndOpenRepository();
+		const quickpick = window.createQuickPick();
+		quickpick.title = l10n.t('Manage Unsafe Repositories');
+		quickpick.placeholder = l10n.t('Pick a repository to mark as safe and open');
+
+		const allRepositoriesLabel = l10n.t('All Repositories');
+		const allRepositoriesQuickPickItem: QuickPickItem = { label: allRepositoriesLabel };
+		const repositoriesQuickPickItems: QuickPickItem[] = Array.from(this.model.unsafeRepositories.values()).sort().map(r => new UnsafeRepositoryItem(r));
+
+		quickpick.items = this.model.unsafeRepositories.size === 1 ? [...repositoriesQuickPickItems] :
+			[...repositoriesQuickPickItems, { label: '', kind: QuickPickItemKind.Separator }, allRepositoriesQuickPickItem];
+
+		quickpick.show();
+		const repositoryItem = await new Promise<UnsafeRepositoryItem | QuickPickItem | undefined>(
+			resolve => {
+				quickpick.onDidAccept(() => resolve(quickpick.activeItems[0]));
+				quickpick.onDidHide(() => resolve(undefined));
+			});
+		quickpick.hide();
+
+		if (!repositoryItem) {
+			return;
+		}
+
+		if (repositoryItem.label === allRepositoriesLabel) {
+			// All Repositories
+			unsafeRepositories.push(...this.model.unsafeRepositories.values());
+		} else {
+			// One Repository
+			unsafeRepositories.push((repositoryItem as UnsafeRepositoryItem).path);
+		}
+
+		for (const unsafeRepository of unsafeRepositories) {
+			// Mark as Safe
+			await this.git.addSafeDirectory(unsafeRepository);
+
+			// Open Repository
+			await this.model.openRepository(unsafeRepository);
+			this.model.unsafeRepositories.delete(unsafeRepository);
+		}
 	}
 
 	private createCommand(id: string, key: string, method: Function, options: ScmCommandOptions): (...args: any[]) => any {

extensions/git/src/git.ts

@@ -686,6 +686,9 @@ export class Git {
 	}
 
 	async addSafeDirectory(repositoryPath: string): Promise<void> {
+		// safe.directory only supports paths with `/` as separator
+		repositoryPath = repositoryPath.replaceAll('\\', '/');
+
 		await this.exec(repositoryPath, ['config', '--global', '--add', 'safe.directory', repositoryPath]);
 		return;
 	}

extensions/git/src/model.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { workspace, WorkspaceFoldersChangeEvent, Uri, window, Event, EventEmitter, QuickPickItem, Disposable, SourceControl, SourceControlResourceGroup, TextEditor, Memento, commands, LogOutputChannel, l10n, ProgressLocation, QuickPickItemKind } from 'vscode';
+import { workspace, WorkspaceFoldersChangeEvent, Uri, window, Event, EventEmitter, QuickPickItem, Disposable, SourceControl, SourceControlResourceGroup, TextEditor, Memento, commands, LogOutputChannel, l10n, ProgressLocation } from 'vscode';
 import TelemetryReporter from '@vscode/extension-telemetry';
 import { Operation, Repository, RepositoryState } from './repository';
 import { memoize, sequentialize, debounce } from './decorators';
@@ -175,9 +175,9 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
 			await initialScanFn();
 		}
 
-		// Show unsafe repositories notification if we cannot use a welcome view
-		if (this.repositories.length > 0 && this._unsafeRepositories.size > 0) {
-			await this.showUnsafeRepositoryNotification();
+		// Unsafe repositories notification
+		if (this._unsafeRepositories.size !== 0) {
+			this.showUnsafeRepositoryNotification();
 		}
 
 		/* __GDPR__
@@ -435,10 +435,9 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
 				const unsafeRepositoryPath = path.normalize(match[1]);
 				this.logger.trace(`Unsafe repository: ${unsafeRepositoryPath}`);
 
-				// If the unsafe repository is opened after the initial repository scan, and we cannot use the welcome view
-				// as there is already at least one opened repository, we will be showing a notification for the repository.
-				if (this._state === 'initialized' && this.openRepositories.length > 0 && !this._unsafeRepositories.has(unsafeRepositoryPath)) {
-					this.showUnsafeRepositoryNotification(unsafeRepositoryPath);
+				// Show a notification if the unsafe repository is opened after the initial repository scan
+				if (this._state === 'initialized' && !this._unsafeRepositories.has(unsafeRepositoryPath)) {
+					this.showUnsafeRepositoryNotification();
 				}
 
 				this._unsafeRepositories.add(unsafeRepositoryPath);
@@ -727,69 +726,18 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
 		return [...this.pushErrorHandlers];
 	}
 
-	async addSafeDirectoryAndOpenRepository() {
-		const unsafeRepositories: string[] = [];
+	private async showUnsafeRepositoryNotification(): Promise<void> {
+		const message = this._unsafeRepositories.size === 1 ?
+			l10n.t('The git repository in the current folder is potentially unsafe as the folder is owned by someone other than the current user.') :
+			l10n.t('The git repositories in the current folder are potentially unsafe as the folders are owned by someone other than the current user.');
 
-		if (this._unsafeRepositories.size === 1) {
-			// One unsafe repository
-			unsafeRepositories.push(this._unsafeRepositories.values().next().value);
-		} else {
-			// Multiple unsafe repositories
-			const allRepositoriesLabel = l10n.t('All Repositories');
-			const allRepositoriesQuickPickItem: QuickPickItem = { label: allRepositoriesLabel };
-			const repositoriesQuickPickItems: QuickPickItem[] = Array.from(this._unsafeRepositories.values()).sort().map(r => ({ label: `$(repo) ${r}` }));
-
-			const quickpick = window.createQuickPick();
-			quickpick.title = l10n.t('Mark Repository as Safe and Open');
-			quickpick.placeholder = l10n.t('Pick a repository to mark as safe and open');
-			quickpick.items = [...repositoriesQuickPickItems, { label: '', kind: QuickPickItemKind.Separator }, allRepositoriesQuickPickItem];
-
-			quickpick.show();
-			const repositoryItem = await new Promise<string | undefined>(
-				resolve => {
-					quickpick.onDidAccept(() => resolve(quickpick.activeItems[0].label));
-					quickpick.onDidHide(() => resolve(undefined));
-				});
-			quickpick.hide();
-
-			if (!repositoryItem) {
-				return;
-			}
-
-			if (repositoryItem === allRepositoriesLabel) {
-				// All Repositories
-				unsafeRepositories.push(...this._unsafeRepositories.values());
-			} else {
-				// One Repository
-				unsafeRepositories.push(repositoryItem);
-			}
-		}
-
-		for (const unsafeRepository of unsafeRepositories) {
-			// Mark as Safe
-			await this.git.addSafeDirectory(unsafeRepository);
-
-			// Open Repository
-			await this.openRepository(unsafeRepository);
-			this._unsafeRepositories.delete(unsafeRepository);
-		}
-	}
-
-	private async showUnsafeRepositoryNotification(path?: string): Promise<void> {
-		const unsafeRepositoryPaths: string[] = path ? [path] : Array.from(this._unsafeRepositories.values());
-		const unsafeRepositoryPathLabels = unsafeRepositoryPaths.sort().map(m => `"${m}"`).join(', ');
-
-		const message = unsafeRepositoryPaths.length === 1 ?
-			l10n.t('The git repository in the following folder is potentially unsafe as the folder is owned by someone other than the current user: {0}. Do you want to open the repository?', unsafeRepositoryPathLabels) :
-			l10n.t('The git repositories in the following folders are potentially unsafe as the folders are owned by someone other than the current user: {0}. Do you want to open the repositories?', unsafeRepositoryPathLabels);
-
-		const openRepository = unsafeRepositoryPaths.length === 1 ? l10n.t('Open Repository') : l10n.t('Open Repositories');
+		const manageUnsafeRepositories = l10n.t('Manage Unsafe Repositories');
 		const learnMore = l10n.t('Learn More');
 
-		const choice = await window.showErrorMessage(message, openRepository, learnMore);
-		if (choice === openRepository) {
-			// Open Repository
-			await this.addSafeDirectoryAndOpenRepository();
+		const choice = await window.showErrorMessage(message, manageUnsafeRepositories, learnMore);
+		if (choice === manageUnsafeRepositories) {
+			// Manage Unsafe Repositories
+			commands.executeCommand('git.manageUnsafeRepositories');
 		} else if (choice === learnMore) {
 			// Learn More
 			commands.executeCommand('vscode.open', Uri.parse('https://aka.ms/vscode-scm'));