Git - Improvements to opening git repositories in parent folders (#171617)

* Initial implementation for external repositories

* Added setting

* Add basic welcome views

* Replaced "Always Open" with "Configure"

* Remove code duplication

* Polish based on feedback

* Language consistency

* Update notification severity

* Move away from the "external repository" terminology

* Refactor notification logic

* Saving my changes

* Further improvements

* Refactor parent repository notification

* Update message and fix edge case when setting is set to `never`
This commit is contained in:
Ladislau Szomoru
2023-01-18 15:27:58 +01:00
committed by GitHub
parent 6894f821dc
commit fe423bbdba
5 changed files with 230 additions and 67 deletions

View File

@@ -191,7 +191,7 @@ class FetchAllRemotesItem implements QuickPickItem {
}
}
class UnsafeRepositoryItem implements QuickPickItem {
class RepositoryItem implements QuickPickItem {
get label(): string {
return `$(repo) ${this.path}`;
}
@@ -3335,6 +3335,38 @@ export class CommandCenter {
repository.closeDiffEditors(undefined, undefined, true);
}
@command('git.openRepositoriesInParentFolders')
async openRepositoriesInParentFolders(): Promise<void> {
const parentRepositories: string[] = [];
const title = l10n.t('Open Repositories In Parent Folders');
const placeHolder = l10n.t('Pick a repository to open');
const allRepositoriesLabel = l10n.t('All Repositories');
const allRepositoriesQuickPickItem: QuickPickItem = { label: allRepositoriesLabel };
const repositoriesQuickPickItems: QuickPickItem[] = Array.from(this.model.parentRepositories.keys()).sort().map(r => new RepositoryItem(r));
const items = this.model.parentRepositories.size === 1 ? [...repositoriesQuickPickItems] :
[...repositoriesQuickPickItems, { label: '', kind: QuickPickItemKind.Separator }, allRepositoriesQuickPickItem];
const repositoryItem = await window.showQuickPick(items, { title, placeHolder });
if (!repositoryItem) {
return;
}
if (repositoryItem === allRepositoriesQuickPickItem) {
// All Repositories
parentRepositories.push(...this.model.parentRepositories.keys());
} else {
// One Repository
parentRepositories.push((repositoryItem as RepositoryItem).path);
}
for (const parentRepository of parentRepositories) {
await this.model.openParentRepository(parentRepository);
}
}
@command('git.manageUnsafeRepositories')
async manageUnsafeRepositories(): Promise<void> {
const unsafeRepositories: string[] = [];
@@ -3345,13 +3377,13 @@ export class CommandCenter {
const allRepositoriesLabel = l10n.t('All Repositories');
const allRepositoriesQuickPickItem: QuickPickItem = { label: allRepositoriesLabel };
const repositoriesQuickPickItems: QuickPickItem[] = Array.from(this.model.unsafeRepositories.keys()).sort().map(r => new UnsafeRepositoryItem(r));
const repositoriesQuickPickItems: QuickPickItem[] = Array.from(this.model.unsafeRepositories.keys()).sort().map(r => new RepositoryItem(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>(
const repositoryItem = await new Promise<RepositoryItem | QuickPickItem | undefined>(
resolve => {
quickpick.onDidAccept(() => resolve(quickpick.activeItems[0]));
quickpick.onDidHide(() => resolve(undefined));
@@ -3367,7 +3399,7 @@ export class CommandCenter {
unsafeRepositories.push(...this.model.unsafeRepositories.keys());
} else {
// One Repository
unsafeRepositories.push((repositoryItem as UnsafeRepositoryItem).path);
unsafeRepositories.push((repositoryItem as RepositoryItem).path);
}
for (const unsafeRepository of unsafeRepositories) {

View File

@@ -34,18 +34,13 @@ class RepositoryPick implements QuickPickItem {
constructor(public readonly repository: Repository, public readonly index: number) { }
}
/**
* Key - normalized path used in user interface
* Value - path extracted from the output of the `git status` command
* used when calling `git config --global --add safe.directory`
*/
class UnsafeRepositoryMap extends Map<string, string> {
abstract class RepositoryMap<T = void> extends Map<string, T> {
constructor() {
super();
this.updateContextKey();
}
override set(key: string, value: string): this {
override set(key: string, value: T): this {
const result = super.set(key, value);
this.updateContextKey();
@@ -59,11 +54,30 @@ class UnsafeRepositoryMap extends Map<string, string> {
return result;
}
private updateContextKey(): void {
abstract updateContextKey(): void;
}
/**
* Key - normalized path used in user interface
* Value - path extracted from the output of the `git status` command
* used when calling `git config --global --add safe.directory`
*/
class UnsafeRepositoryMap extends RepositoryMap<string> {
updateContextKey(): void {
commands.executeCommand('setContext', 'git.unsafeRepositoryCount', this.size);
}
}
/**
* Key - normalized path used in user interface
* Value - value indicating whether the repository should be opened
*/
class ParentRepositoryMap extends RepositoryMap {
updateContextKey(): void {
commands.executeCommand('setContext', 'git.parentRepositoryCount', this.size);
}
}
export interface ModelChangeEvent {
repository: Repository;
uri: Uri;
@@ -138,14 +152,18 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
private _onDidChangePostCommitCommandsProviders = new EventEmitter<void>();
readonly onDidChangePostCommitCommandsProviders = this._onDidChangePostCommitCommandsProviders.event;
private showRepoOnHomeDriveRootWarning = true;
private pushErrorHandlers = new Set<PushErrorHandler>();
private _unsafeRepositories = new UnsafeRepositoryMap();
get unsafeRepositories(): Map<string, string> {
get unsafeRepositories(): UnsafeRepositoryMap {
return this._unsafeRepositories;
}
private _parentRepositories = new ParentRepositoryMap();
get parentRepositories(): ParentRepositoryMap {
return this._parentRepositories;
}
private disposables: Disposable[] = [];
constructor(readonly git: Git, private readonly askpass: Askpass, private globalState: Memento, private logger: LogOutputChannel, private telemetryReporter: TelemetryReporter) {
@@ -168,6 +186,7 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
private async doInitialScan(): Promise<void> {
const config = workspace.getConfiguration('git');
const autoRepositoryDetection = config.get<boolean | 'subFolders' | 'openEditors'>('autoRepositoryDetection');
const parentRepositoryConfig = config.get<'always' | 'never' | 'prompt'>('openRepositoryInParentFolders', 'prompt');
const initialScanFn = () => Promise.all([
this.onDidChangeWorkspaceFolders({ added: workspace.workspaceFolders || [], removed: [] }),
@@ -181,8 +200,12 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
await initialScanFn();
}
// Unsafe repositories notification
if (this._unsafeRepositories.size !== 0) {
if (this._parentRepositories.size !== 0 &&
parentRepositoryConfig === 'prompt') {
// Parent repositories notification
this.showParentRepositoryNotification();
} else if (this._unsafeRepositories.size !== 0) {
// Unsafe repositories notification
this.showUnsafeRepositoryNotification();
}
@@ -394,65 +417,87 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
}
try {
const rawRoot = await this.git.getRepositoryRoot(repoPath);
// This can happen whenever `path` has the wrong case sensitivity in
// case insensitive file systems
// https://github.com/microsoft/vscode/issues/33498
const repositoryRoot = Uri.file(rawRoot).fsPath;
this.logger.trace(`Repository root: ${repositoryRoot}`);
const { repositoryRoot, unsafeRepositoryMatch } = await this.getRepositoryRoot(repoPath);
if (this.getRepositoryExact(repositoryRoot)) {
this.logger.trace(`Repository for path ${repositoryRoot} already exists`);
return;
}
if (this.shouldRepositoryBeIgnored(rawRoot)) {
if (this.shouldRepositoryBeIgnored(repositoryRoot)) {
this.logger.trace(`Repository for path ${repositoryRoot} is ignored`);
return;
}
// On Window, opening a git repository from the root of the HOMEDRIVE poses a security risk.
// We will only a open git repository from the root of the HOMEDRIVE if the user explicitly
// opens the HOMEDRIVE as a folder. Only show the warning once during repository discovery.
if (process.platform === 'win32' && process.env.HOMEDRIVE && pathEquals(`${process.env.HOMEDRIVE}\\`, repositoryRoot)) {
const isRepoInWorkspaceFolders = (workspace.workspaceFolders ?? []).find(f => pathEquals(f.uri.fsPath, repositoryRoot))!!;
// Handle git repositories that are in parent folders
const isRepositoryOutsideWorkspace = (workspace.workspaceFolders ?? [])
.find(f => pathEquals(f.uri.fsPath, repositoryRoot) || isDescendant(f.uri.fsPath, repositoryRoot)) === undefined;
const parentRepositoryConfig = config.get<'always' | 'never' | 'prompt'>('openRepositoryInParentFolders', 'prompt');
if (!isRepoInWorkspaceFolders) {
if (this.showRepoOnHomeDriveRootWarning) {
window.showWarningMessage(l10n.t('Unable to automatically open the git repository at "{0}". To open that git repository, open it directly as a folder in VS Code.', repositoryRoot));
this.showRepoOnHomeDriveRootWarning = false;
if (isRepositoryOutsideWorkspace && parentRepositoryConfig !== 'always' && this.globalState.get<boolean>(`parentRepository:${repositoryRoot}`) !== true) {
this.logger.trace(`Repository in parent folder: ${repositoryRoot}`);
if (!this._parentRepositories.has(repositoryRoot)) {
// Show a notification if the parent repository is opened after the initial scan
if (this.state === 'initialized' && parentRepositoryConfig === 'prompt') {
this.showParentRepositoryNotification();
}
this.logger.trace(`Repository for path ${repositoryRoot} is on the root of the HOMEDRIVE`);
return;
this._parentRepositories.set(repositoryRoot);
}
return;
}
// Handle unsafe repositories
if (unsafeRepositoryMatch && unsafeRepositoryMatch.length === 3) {
this.logger.trace(`Unsafe repository: ${repositoryRoot}`);
// Show a notification if the unsafe repository is opened after the initial scan
if (this._state === 'initialized' && !this._unsafeRepositories.has(repositoryRoot)) {
this.showUnsafeRepositoryNotification();
}
this._unsafeRepositories.set(repositoryRoot, unsafeRepositoryMatch[2]);
return;
}
// Open repository
const dotGit = await this.git.getRepositoryDotGit(repositoryRoot);
const repository = new Repository(this.git.open(repositoryRoot, dotGit, this.logger), this, this, this, this.globalState, this.logger, this.telemetryReporter);
this.open(repository);
repository.status(); // do not await this, we want SCM to know about the repo asap
} catch (ex) {
} catch (err) {
// noop
this.logger.trace(`Opening repository for path='${repoPath}' failed; ex=${err}`);
}
}
async openParentRepository(repoPath: string): Promise<void> {
// Mark the repository to be opened from the parent folders
this.globalState.update(`parentRepository:${repoPath}`, true);
await this.openRepository(repoPath);
this.parentRepositories.delete(repoPath);
}
private async getRepositoryRoot(repoPath: string): Promise<{ repositoryRoot: string; unsafeRepositoryMatch: RegExpMatchArray | null }> {
try {
const rawRoot = await this.git.getRepositoryRoot(repoPath);
// This can happen whenever `path` has the wrong case sensitivity in case
// insensitive file systems https://github.com/microsoft/vscode/issues/33498
return { repositoryRoot: Uri.file(rawRoot).fsPath, unsafeRepositoryMatch: null };
} catch (err) {
// Handle unsafe repository
const match = /^fatal: detected dubious ownership in repository at \'([^']+)\'[\s\S]*git config --global --add safe\.directory '?([^'\n]+)'?$/m.exec(ex.stderr);
if (match && match.length === 3) {
const unsafeRepositoryPath = path.normalize(match[1]);
this.logger.trace(`Unsafe repository: ${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.set(unsafeRepositoryPath, match[2]);
return;
const unsafeRepositoryMatch = /^fatal: detected dubious ownership in repository at \'([^']+)\'[\s\S]*git config --global --add safe\.directory '?([^'\n]+)'?$/m.exec(err.stderr);
if (unsafeRepositoryMatch && unsafeRepositoryMatch.length === 3) {
return { repositoryRoot: path.normalize(unsafeRepositoryMatch[1]), unsafeRepositoryMatch };
}
// noop
this.logger.trace(`Opening repository for path='${repoPath}' failed; ex=${ex}`);
throw err;
}
}
@@ -737,6 +782,36 @@ export class Model implements IRemoteSourcePublisherRegistry, IPostCommitCommand
return [...this.pushErrorHandlers];
}
private async showParentRepositoryNotification(): Promise<void> {
const message = this.parentRepositories.size === 1 ?
workspace.workspaceFolders !== undefined ?
l10n.t('We found a git repository in one of the parent folders of this workspace. Would you like to open the repository?') :
l10n.t('We found a git repository in one of the parent folders of the open file(s). Would you like to open the repository?') :
workspace.workspaceFolders !== undefined ?
l10n.t('We found git repositories in one of the parent folders of this workspace. Would you like to open the repositories?') :
l10n.t('We found git repositories in one of the parent folders of the open file(s). Would you like to open the repositories?');
const yes = l10n.t('Yes');
const always = l10n.t('Always');
const never = l10n.t('Never');
const choice = await window.showWarningMessage(message, yes, always, never);
if (choice === yes) {
// Open Parent Repositories
commands.executeCommand('git.openRepositoriesInParentFolders');
} else if (choice === always || choice === never) {
// Update setting
const config = workspace.getConfiguration('git');
await config.update('openRepositoryInParentFolders', choice === always ? 'always' : 'never', true);
if (choice === always) {
for (const parentRepository of [...this.parentRepositories.keys()]) {
await this.openParentRepository(parentRepository);
}
}
}
}
private async showUnsafeRepositoryNotification(): Promise<void> {
// If no repositories are open, we will use a welcome view to inform the user
// that a potentially unsafe repository was found so we do not have to show