3528 Change branch terminal-run commands into normal commands w/ proper error handling

src/commands/git/branch.ts

@@ -1,5 +1,6 @@
 import { QuickInputButtons } from 'vscode';
 import type { Container } from '../../container';
+import { BranchError, BranchErrorReason } from '../../git/errors';
 import type { GitBranchReference, GitReference } from '../../git/models/reference';
 import {
 	getNameWithoutRemote,
@@ -10,7 +11,7 @@ import {
 import { Repository } from '../../git/models/repository';
 import type { GitWorktree } from '../../git/models/worktree';
 import { getWorktreesByBranch } from '../../git/models/worktree';
-import { showGenericErrorMessage } from '../../messages';
+import { showGenericErrorMessage, showGitBranchNotFullyMergedPrompt } from '../../messages';
 import type { QuickPickItemOfT } from '../../quickpicks/items/common';
 import { createQuickPickSeparator } from '../../quickpicks/items/common';
 import type { FlagsQuickPickItem } from '../../quickpicks/items/flags';
@@ -427,7 +428,7 @@ export class BranchGitCommand extends QuickCommand {
 				} catch (ex) {
 					Logger.error(ex);
 					// TODO likely need some better error handling here
-					return showGenericErrorMessage('Unable to create branch');
+					return showGenericErrorMessage(ex);
 				}
 			}
 		}
@@ -558,10 +559,35 @@ 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) {
+				try {
+					await state.repo.git.deleteBranch(ref, {
+						force: state.flags.includes('--force'),
+						remote: state.flags.includes('--remotes'),
+					});
+				} catch (ex) {
+					// TODO likely need some better error handling here
+					Logger.error(ex);
+					if (ex instanceof BranchError && ex.reason === BranchErrorReason.BranchNotFullyMerged) {
+						const shouldRetryWithForce = await showGitBranchNotFullyMergedPrompt(ref.name);
+						if (shouldRetryWithForce) {
+							try {
+								await state.repo.git.deleteBranch(ref, {
+									force: true,
+									remote: state.flags.includes('--remotes'),
+								});
+							} catch (ex) {
+								Logger.error(ex);
+								await showGenericErrorMessage(ex);
+							}
+						}
+						continue;
+					}
+
+					await showGenericErrorMessage(ex);
+				}
+			}
 		}
 	}
 
@@ -669,7 +695,7 @@ export class BranchGitCommand extends QuickCommand {
 			} catch (ex) {
 				Logger.error(ex);
 				// TODO likely need some better error handling here
-				return showGenericErrorMessage('Unable to rename branch');
+				return showGenericErrorMessage(ex);
 			}
 		}
 	}

src/env/node/git/git.ts

@@ -12,6 +12,8 @@ import { GitErrorHandling } from '../../../git/commandOptions';
 import {
 	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
+	BranchError,
+	BranchErrorReason,
 	CherryPickError,
 	CherryPickErrorReason,
 	FetchError,
@@ -73,6 +75,10 @@ const textDecoder = new TextDecoder('utf8');
 const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
 
 export const GitErrors = {
+	noRemoteReference: /unable to delete '.+?': remote ref does not exist/i,
+	invalidBranchName: /fatal: '.+?' is not a valid branch name/i,
+	branchAlreadyExists: /fatal: A branch named '.+?' already exists/i,
+	branchNotFullyMerged: /error: The branch '.+?' is not fully merged/i,
 	badRevision: /bad revision '(.*?)'/i,
 	cantLockRef: /cannot lock ref|unable to update local ref/i,
 	changesWouldBeOverwritten: /Your local changes to the following files would be overwritten/i,
@@ -165,6 +171,13 @@ function getStdinUniqueKey(): number {
 type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true };
 export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never };
 
+const branchErrorAndReason: [RegExp, BranchErrorReason][] = [
+	[GitErrors.noRemoteReference, BranchErrorReason.NoRemoteReference],
+	[GitErrors.invalidBranchName, BranchErrorReason.InvalidBranchName],
+	[GitErrors.branchAlreadyExists, BranchErrorReason.BranchAlreadyExists],
+	[GitErrors.branchNotFullyMerged, BranchErrorReason.BranchNotFullyMerged],
+];
+
 const tagErrorAndReason: [RegExp, TagErrorReason][] = [
 	[GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists],
 	[GitErrors.tagNotFound, TagErrorReason.TagNotFound],
@@ -520,8 +533,18 @@ export class Git {
 		}
 	}
 
-	branch(repoPath: string, ...args: string[]) {
-		return this.git<string>({ cwd: repoPath }, 'branch', ...args);
+	async branch(repoPath: string, ...args: string[]): Promise<void> {
+		try {
+			await this.git<string>({ cwd: repoPath }, 'branch', ...args);
+		} catch (ex) {
+			const msg: string = ex?.toString() ?? '';
+			for (const [error, reason] of branchErrorAndReason) {
+				if (error.test(msg) || error.test(ex.stderr ?? '')) {
+					throw new BranchError(reason, ex);
+				}
+			}
+			throw new BranchError(BranchErrorReason.Other, ex);
+		}
 	}
 
 	branch__set_upstream(repoPath: string, branch: string, remote: string, remoteBranch: string) {
@@ -976,6 +999,10 @@ export class Git {
 			publish?: boolean;
 			remote?: string;
 			upstream?: string;
+			delete?: {
+				remote: string;
+				branch: string;
+			};
 		},
 	): Promise<void> {
 		const params = ['push'];
@@ -1003,6 +1030,8 @@ export class Git {
 			}
 		} else if (options.remote) {
 			params.push(options.remote);
+		} else if (options.delete) {
+			params.push('-d', options.delete.remote, options.delete.branch);
 		}
 
 		try {
@@ -1023,9 +1052,13 @@ export class Git {
 						/! \[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 if (/error: unable to delete '(.*?)': remote ref does not exist/m.test(ex.stderr || '')) {
+					reason = PushErrorReason.PushRejectedRefNotExists;
 				} else {
 					reason = PushErrorReason.PushRejected;
 				}
@@ -1037,7 +1070,12 @@ export class Git {
 				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,
+			);
 		}
 	}
 
@@ -1591,7 +1629,12 @@ export class Git {
 	async rev_list(
 		repoPath: string,
 		ref: string,
-		options?: { all?: boolean; maxParents?: number; since?: string },
+		options?: {
+			all?: boolean;
+			maxParents?: number;
+			maxResults?: number;
+            since?: string;
+		},
 	): Promise<string[] | undefined> {
 		const params = ['rev-list'];
 		if (options?.all) {
@@ -1602,6 +1645,10 @@ export class Git {
 			params.push(`--max-parents=${options.maxParents}`);
 		}
 
+		if (options?.maxResults != null) {
+			params.push(`-n ${options.maxResults}`);
+		}
+
 		if (options?.since) {
 			params.push(`--since="${options.since}"`, '--date-order');
 		}
@@ -1871,6 +1918,10 @@ export class Git {
 		return data.length === 0 ? undefined : data.trim();
 	}
 
+	async update_ref(repoPath: string, ...args: string[]): Promise<void> {
+		await this.git<string>({ cwd: repoPath }, 'update-ref', ...args);
+	}
+
 	show(
 		repoPath: string,
 		options?: { cancellation?: CancellationToken; configs?: readonly string[] },

src/env/node/git/localGitProvider.ts

@@ -24,6 +24,7 @@ import {
 	ApplyPatchCommitErrorReason,
 	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
+	BranchError,
 	CherryPickError,
 	CherryPickErrorReason,
 	FetchError,
@@ -1277,12 +1278,86 @@ export class LocalGitProvider implements GitProvider, Disposable {
 
 	@log()
 	async createBranch(repoPath: string, name: string, ref: string): Promise<void> {
-		await this.git.branch(repoPath, name, ref);
+		try {
+			await this.git.branch(repoPath, name, ref);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.WithBranch(name);
+			}
+
+			throw ex;
+		}
 	}
 
 	@log()
 	async renameBranch(repoPath: string, oldName: string, newName: string): Promise<void> {
-		await this.git.branch(repoPath, '-m', oldName, newName);
+		try {
+			await this.git.branch(repoPath, '-m', oldName, newName);
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.WithBranch(oldName);
+			}
+
+			throw ex;
+		}
+	}
+
+	@log()
+	async deleteBranch(
+		repoPath: string,
+		branch: GitBranchReference,
+		options: { force?: boolean; remote?: boolean },
+	): Promise<void> {
+		try {
+			if (branch.remote) {
+				await this.git.push(repoPath, {
+					delete: {
+						remote: getRemoteNameFromBranchName(branch.name),
+						branch: branch.remote ? getBranchNameWithoutRemote(branch.name) : branch.name,
+					},
+				});
+				return;
+			}
+
+			const args = ['--delete'];
+			if (options.force) {
+				args.push('--force');
+			}
+
+			if (!options.remote || !branch.upstream) {
+				await this.git.branch(repoPath, ...args, branch.ref);
+				return;
+			}
+
+			const remote = getRemoteNameFromBranchName(branch.upstream.name);
+			const remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, {
+				maxResults: 1,
+			});
+
+			await this.git.branch(repoPath, '--delete', '--remotes', `${remote}/${branch.ref}`);
+
+			try {
+				await this.git.branch(repoPath, ...args, branch.ref);
+			} catch (ex) {
+				// If it fails, restore the remote branch
+				await this.git.update_ref(repoPath, `refs/remotes/${remote}/${branch.ref}`, remoteCommit?.[0] ?? '');
+				await this.git.branch__set_upstream(repoPath, branch.name, remote, branch.ref);
+				throw ex;
+			}
+
+			await this.git.push(repoPath, {
+				delete: {
+					remote: remote,
+					branch: getBranchNameWithoutRemote(branch.upstream.name),
+				},
+			});
+		} catch (ex) {
+			if (ex instanceof BranchError) {
+				throw ex.WithBranch(branch.name);
+			}
+
+			throw ex;
+		}
 	}
 
 	@log()

src/git/errors.ts

@@ -148,6 +148,7 @@ export const enum PushErrorReason {
 	PushRejected,
 	PushRejectedWithLease,
 	PushRejectedWithLeaseIfIncludes,
+	PushRejectedRefNotExists,
 	PermissionDenied,
 	RemoteConnection,
 	NoUpstream,
@@ -197,6 +198,11 @@ export class PushError extends Error {
 						remote ? ` to ${remote}` : ''
 					} because some refs failed to push or the push was rejected. The tip of the remote-tracking branch has been updated since the last checkout. Try pulling first.`;
 					break;
+				case PushErrorReason.PushRejectedRefNotExists:
+					message = `Unable to delete branch${branch ? ` '${branch}'` : ''}${
+						remote ? ` on ${remote}` : ''
+					}, the remote reference does not exist`;
+					break;
 				case PushErrorReason.PermissionDenied:
 					message = `${baseMessage} because you don't have permission to push to this remote repository.`;
 					break;
@@ -218,6 +224,61 @@ export class PushError extends Error {
 	}
 }
 
+export const enum BranchErrorReason {
+	BranchAlreadyExists,
+	BranchNotFullyMerged,
+	NoRemoteReference,
+	InvalidBranchName,
+	Other,
+}
+
+export class BranchError extends Error {
+	static is(ex: unknown, reason?: BranchErrorReason): ex is BranchError {
+		return ex instanceof BranchError && (reason == null || ex.reason === reason);
+	}
+
+	readonly original?: Error;
+	readonly reason: BranchErrorReason | undefined;
+
+	constructor(reason?: BranchErrorReason, original?: Error, branch?: string);
+	constructor(message?: string, original?: Error);
+	constructor(messageOrReason: string | BranchErrorReason | undefined, original?: Error, branch?: string) {
+		let reason: BranchErrorReason | undefined;
+		if (typeof messageOrReason !== 'string') {
+			reason = messageOrReason as BranchErrorReason;
+		}
+
+		const message =
+			typeof messageOrReason === 'string' ? messageOrReason : BranchError.buildBranchErrorMessage(reason, branch);
+		super(message);
+
+		this.original = original;
+		this.reason = reason;
+		Error.captureStackTrace?.(this, BranchError);
+	}
+
+	private static buildBranchErrorMessage(reason?: BranchErrorReason, branch?: string): string {
+		const baseMessage = `Unable to perform action ${branch ? `with branch '${branch}'` : 'on branch'}`;
+		switch (reason) {
+			case BranchErrorReason.BranchAlreadyExists:
+				return `${baseMessage} because it already exists`;
+			case BranchErrorReason.BranchNotFullyMerged:
+				return `${baseMessage} because it is not fully merged`;
+			case BranchErrorReason.NoRemoteReference:
+				return `${baseMessage} because the remote reference does not exist`;
+			case BranchErrorReason.InvalidBranchName:
+				return `${baseMessage} because the branch name is invalid`;
+			default:
+				return baseMessage;
+		}
+	}
+
+	WithBranch(branchName: string): this {
+		this.message = BranchError.buildBranchErrorMessage(this.reason, branchName);
+		return this;
+	}
+}
+
 export const enum PullErrorReason {
 	Conflict,
 	GitIdentity,