Changes delete branch command to avoid terminal (#4359)

Author: eamodio

src/commands/git/branch.ts

@@ -1,11 +1,13 @@
-import { QuickInputButtons } from 'vscode';
+import { QuickInputButtons, window } from 'vscode';
 import type { Container } from '../../container';
+import { BranchError, BranchErrorReason } from '../../git/errors';
 import type { IssueShape } from '../../git/models/issue';
 import type { GitBranchReference, GitReference } from '../../git/models/reference';
 import { Repository } from '../../git/models/repository';
 import type { GitWorktree } from '../../git/models/worktree';
 import { addAssociatedIssueToBranch } from '../../git/utils/-webview/branch.issue.utils';
 import { getWorktreesByBranch } from '../../git/utils/-webview/worktree.utils';
+import { getBranchNameAndRemote } from '../../git/utils/branch.utils';
 import {
 	getReferenceLabel,
 	getReferenceNameWithoutRemote,
@@ -430,7 +432,7 @@ export class BranchGitCommand extends QuickCommand {
 				try {
 					await state.repo.git.branches.createBranch?.(state.name, state.reference.ref);
 				} catch (ex) {
-					Logger.error(ex);
+					Logger.error(ex, context.title);
 					// TODO likely need some better error handling here
 					return showGenericErrorMessage('Unable to create branch');
 				}
@@ -571,10 +573,47 @@ export class BranchGitCommand extends QuickCommand {
 			state.flags = result;
 
 			endSteps(state);
-			state.repo.branchDelete(state.references, {
-				force: state.flags.includes('--force'),
-				remote: state.flags.includes('--remotes'),
-			});
+
+			for (const ref of state.references) {
+				const [name, remote] = getBranchNameAndRemote(ref);
+				try {
+					if (ref.remote) {
+						await state.repo.git.branches.deleteRemoteBranch?.(name, remote!);
+					} else {
+						await state.repo.git.branches.deleteLocalBranch?.(name, {
+							force: state.flags.includes('--force'),
+						});
+						if (state.flags.includes('--remotes') && remote) {
+							await state.repo.git.branches.deleteRemoteBranch?.(name, remote);
+						}
+					}
+				} catch (ex) {
+					if (BranchError.is(ex, BranchErrorReason.BranchNotFullyMerged)) {
+						const confirm = { title: 'Delete Branch' };
+						const cancel = { title: 'Cancel', isCloseAffordance: true };
+						const result = await window.showWarningMessage(
+							`Unable to delete branch '${name}' as it is not fully merged. Do you want to delete it anyway?`,
+							{ modal: true },
+							confirm,
+							cancel,
+						);
+
+						if (result === confirm) {
+							try {
+								await state.repo.git.branches.deleteLocalBranch?.(name, { force: true });
+							} catch (ex) {
+								Logger.error(ex, context.title);
+								void showGenericErrorMessage(ex);
+							}
+						}
+
+						continue;
+					}
+
+					Logger.error(ex, context.title);
+					void showGenericErrorMessage(ex);
+				}
+			}
 		}
 	}
 
@@ -680,7 +719,7 @@ export class BranchGitCommand extends QuickCommand {
 			try {
 				await state.repo.git.branches.renameBranch?.(state.reference.ref, state.name);
 			} catch (ex) {
-				Logger.error(ex);
+				Logger.error(ex, context.title);
 				// TODO likely need some better error handling here
 				return showGenericErrorMessage('Unable to rename branch');
 			}

src/commands/git/tag.ts

@@ -383,6 +383,7 @@ export class TagGitCommand extends QuickCommand<State> {
 			if (result === StepResultBreak) continue;
 
 			endSteps(state);
+
 			for (const { ref } of state.references) {
 				try {
 					await state.repo.git.tags.deleteTag?.(ref);

src/env/node/git/git.ts

@@ -16,6 +16,8 @@ import { GitErrorHandling } from '../../../git/commandOptions';
 import {
 	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
+	BranchError,
+	BranchErrorReason,
 	FetchError,
 	FetchErrorReason,
 	PullError,
@@ -75,6 +77,8 @@ export const GitErrors = {
 	alreadyExists: /already exists/i,
 	ambiguousArgument: /fatal:\s*ambiguous argument ['"].+['"]: unknown revision or path not in the working tree/i,
 	badRevision: /bad revision '(.*?)'/i,
+	branchAlreadyExists: /fatal: A branch named '.+?' already exists/i,
+	branchNotFullyMerged: /error: The branch '.+?' is not fully merged/i,
 	cantLockRef: /cannot lock ref|unable to update local ref/i,
 	changesWouldBeOverwritten:
 		/Your local changes to the following files would be overwritten|Your local changes would be overwritten/i,
@@ -84,13 +88,15 @@ export const GitErrors = {
 	emptyPreviousCherryPick: /The previous cherry-pick is now empty/i,
 	entryNotUpToDate: /error:\s*Entry ['"].+['"] not uptodate\. Cannot merge\./i,
 	failedToDeleteDirectoryNotEmpty: /failed to delete '(.*?)': Directory not empty/i,
+	invalidBranchName: /fatal: '.+?' is not a valid branch name/i,
 	invalidLineCount: /file .+? has only (\d+) lines/i,
 	invalidObjectName: /invalid object name: (.*)\s/i,
 	invalidObjectNameList: /could not open object name list: (.*)\s/i,
 	invalidTagName: /invalid tag name/i,
 	mainWorkingTree: /is a main working tree/i,
 	noFastForward: /\(non-fast-forward\)/i,
 	noMergeBase: /no merge base/i,
+	noRemoteReference: /unable to delete '.+?': remote ref does not exist/i,
 	noRemoteRepositorySpecified: /No remote repository specified\./i,
 	noUpstream: /^fatal: The current branch .* has no upstream branch/i,
 	notAValidObjectName: /Not a valid object name/i,
@@ -169,15 +175,14 @@ const uniqueCounterForStdin = getScopedCounter();
 type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true };
 export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never };
 
-const tagErrorAndReason: [RegExp, TagErrorReason][] = [
-	[GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists],
-	[GitErrors.tagNotFound, TagErrorReason.TagNotFound],
-	[GitErrors.invalidTagName, TagErrorReason.InvalidTagName],
-	[GitErrors.permissionDenied, TagErrorReason.PermissionDenied],
-	[GitErrors.remoteRejected, TagErrorReason.RemoteRejected],
+const branchErrorsToReasons: [RegExp, BranchErrorReason][] = [
+	[GitErrors.noRemoteReference, BranchErrorReason.NoRemoteReference],
+	[GitErrors.invalidBranchName, BranchErrorReason.InvalidBranchName],
+	[GitErrors.branchAlreadyExists, BranchErrorReason.BranchAlreadyExists],
+	[GitErrors.branchNotFullyMerged, BranchErrorReason.BranchNotFullyMerged],
 ];
 
-const resetErrorAndReason: [RegExp, ResetErrorReason][] = [
+const resetErrorsToReasons: [RegExp, ResetErrorReason][] = [
 	[GitErrors.ambiguousArgument, ResetErrorReason.AmbiguousArgument],
 	[GitErrors.changesWouldBeOverwritten, ResetErrorReason.ChangesWouldBeOverwritten],
 	[GitErrors.detachedHead, ResetErrorReason.DetachedHead],
@@ -187,6 +192,14 @@ const resetErrorAndReason: [RegExp, ResetErrorReason][] = [
 	[GitErrors.unmergedChanges, ResetErrorReason.UnmergedChanges],
 ];
 
+const tagErrorsToReasons: [RegExp, TagErrorReason][] = [
+	[GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists],
+	[GitErrors.tagNotFound, TagErrorReason.TagNotFound],
+	[GitErrors.invalidTagName, TagErrorReason.InvalidTagName],
+	[GitErrors.permissionDenied, TagErrorReason.PermissionDenied],
+	[GitErrors.remoteRejected, TagErrorReason.RemoteRejected],
+];
+
 export class GitError extends Error {
 	readonly cmd: string | undefined;
 	readonly exitCode: number | string | undefined;
@@ -705,6 +718,21 @@ export class Git implements Disposable {
 		}
 	}
 
+	async branch(repoPath: string, ...args: string[]): Promise<GitResult<string>> {
+		try {
+			const result = await this.exec({ cwd: repoPath }, 'branch', ...args);
+			return result;
+		} catch (ex) {
+			const msg: string = ex?.toString() ?? '';
+			for (const [error, reason] of branchErrorsToReasons) {
+				if (error.test(msg) || error.test(ex.stderr ?? '')) {
+					throw new BranchError(reason, ex);
+				}
+			}
+			throw new BranchError(BranchErrorReason.Other, ex);
+		}
+	}
+
 	async branchOrTag__containsOrPointsAt(
 		repoPath: string,
 		refs: string[],
@@ -991,6 +1019,10 @@ export class Git implements Disposable {
 			publish?: boolean;
 			remote?: string;
 			upstream?: string;
+			delete?: {
+				remote: string;
+				branch: string;
+			};
 		},
 	): Promise<void> {
 		const params = ['push'];
@@ -1018,6 +1050,8 @@ export class Git implements Disposable {
 			}
 		} else if (options.remote) {
 			params.push(options.remote);
+		} else if (options.delete) {
+			params.push(options.delete.remote, `:${options.delete.branch}`);
 		}
 
 		try {
@@ -1038,12 +1072,16 @@ export class Git implements Disposable {
 						/! \[rejected\].*\(remote ref updated since checkout\)/m.test(ex.stderr || '')
 					) {
 						reason = PushErrorReason.PushRejectedWithLeaseIfIncludes;
+					} else if (/error: unable to delete '(.*?)': remote ref does not exist/m.test(ex.stderr || '')) {
+						reason = PushErrorReason.PushRejectedRefNotExists;
 					} else {
 						reason = PushErrorReason.PushRejected;
 					}
 				} else {
 					reason = PushErrorReason.PushRejected;
 				}
+			} else if (/error: unable to delete '(.*?)': remote ref does not exist/m.test(ex.stderr || '')) {
+				reason = PushErrorReason.PushRejectedRefNotExists;
 			} else if (GitErrors.permissionDenied.test(msg) || GitErrors.permissionDenied.test(ex.stderr ?? '')) {
 				reason = PushErrorReason.PermissionDenied;
 			} else if (GitErrors.remoteConnection.test(msg) || GitErrors.remoteConnection.test(ex.stderr ?? '')) {
@@ -1052,7 +1090,12 @@ export class Git implements Disposable {
 				reason = PushErrorReason.NoUpstream;
 			}
 
-			throw new PushError(reason, ex, options?.branch, options?.remote);
+			throw new PushError(
+				reason,
+				ex,
+				options?.branch || options?.delete?.branch,
+				options?.remote || options?.delete?.remote,
+			);
 		}
 	}
 
@@ -1126,7 +1169,7 @@ export class Git implements Disposable {
 			await this.exec({ cwd: repoPath }, 'reset', '-q', ...flags, '--', ...pathspecs);
 		} catch (ex) {
 			const msg: string = ex?.toString() ?? '';
-			for (const [error, reason] of resetErrorAndReason) {
+			for (const [error, reason] of resetErrorsToReasons) {
 				if (error.test(msg) || error.test(ex.stderr ?? '')) {
 					throw new ResetError(reason, ex);
 				}
@@ -1542,7 +1585,7 @@ export class Git implements Disposable {
 			return result;
 		} catch (ex) {
 			const msg: string = ex?.toString() ?? '';
-			for (const [error, reason] of tagErrorAndReason) {
+			for (const [error, reason] of tagErrorsToReasons) {
 				if (error.test(msg) || error.test(ex.stderr ?? '')) {
 					throw new TagError(reason, ex);
 				}

src/env/node/git/sub-providers/branches.ts

@@ -3,6 +3,7 @@ import type { Container } from '../../../../container';
 import { CancellationError, isCancellationError } from '../../../../errors';
 import type { GitCache } from '../../../../git/cache';
 import { GitErrorHandling } from '../../../../git/commandOptions';
+import { BranchError } from '../../../../git/errors';
 import type {
 	BranchContributionsOverview,
 	GitBranchesSubProvider,
@@ -441,7 +442,41 @@ export class BranchesGitSubProvider implements GitBranchesSubProvider {
 
 	@log()
 	async createBranch(repoPath: string, name: string, sha: string): Promise<void> {
-		await this.git.exec({ cwd: repoPath }, 'branch', name, sha);
+		try {
+			await this.git.branch(repoPath, name, sha);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.update({ branch: name, action: 'create' });
+			}
+
+			throw ex;
+		}
+	}
+
+	@log()
+	async deleteLocalBranch(repoPath: string, name: string, options?: { force?: boolean }): Promise<void> {
+		try {
+			await this.git.branch(repoPath, options?.force ? '-D' : '-d', name);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.update({ branch: name, action: 'delete' });
+			}
+
+			throw ex;
+		}
+	}
+
+	@log()
+	async deleteRemoteBranch(repoPath: string, name: string, remote: string): Promise<void> {
+		try {
+			await this.git.exec({ cwd: repoPath }, 'push', '-d', remote, name);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.update({ branch: name, action: 'delete' });
+			}
+
+			throw ex;
+		}
 	}
 
 	@log()
@@ -781,7 +816,15 @@ export class BranchesGitSubProvider implements GitBranchesSubProvider {
 
 	@log()
 	async renameBranch(repoPath: string, oldName: string, newName: string): Promise<void> {
-		await this.git.exec({ cwd: repoPath }, 'branch', '-m', oldName, newName);
+		try {
+			await this.git.branch(repoPath, 'branch', '-m', oldName, newName);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.update({ branch: oldName, action: 'rename' });
+			}
+
+			throw ex;
+		}
 	}
 
 	@log()

src/env/node/git/sub-providers/tags.ts

@@ -134,7 +134,7 @@ export class TagsGitSubProvider implements GitTagsSubProvider {
 			await this.git.tag(repoPath, name, sha, ...(message != null && message.length > 0 ? ['-m', message] : []));
 		} catch (ex) {
 			if (ex instanceof TagError) {
-				throw ex.withTag(name).withAction('create');
+				throw ex.update({ tag: name, action: 'create' });
 			}
 
 			throw ex;
@@ -147,7 +147,7 @@ export class TagsGitSubProvider implements GitTagsSubProvider {
 			await this.git.tag(repoPath, '-d', name);
 		} catch (ex) {
 			if (ex instanceof TagError) {
-				throw ex.withTag(name).withAction('delete');
+				throw ex.update({ tag: name, action: 'delete' });
 			}
 
 			throw ex;