diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 660f2f7612294..9de075258145d 100644 --- a/src/commands/git/branch.ts +++ b/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'); } diff --git a/src/commands/git/tag.ts b/src/commands/git/tag.ts index 0c3a80c19a1d0..c4032d1393d69 100644 --- a/src/commands/git/tag.ts +++ b/src/commands/git/tag.ts @@ -383,6 +383,7 @@ export class TagGitCommand extends QuickCommand { if (result === StepResultBreak) continue; endSteps(state); + for (const { ref } of state.references) { try { await state.repo.git.tags.deleteTag?.(ref); diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 2ea6875a624f4..6d8be77bf3b54 100644 --- a/src/env/node/git/git.ts +++ b/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,6 +88,7 @@ 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, @@ -91,6 +96,7 @@ export const GitErrors = { 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> { + 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 { 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); } diff --git a/src/env/node/git/sub-providers/branches.ts b/src/env/node/git/sub-providers/branches.ts index 717e9ef91dad4..07748ffcae4a3 100644 --- a/src/env/node/git/sub-providers/branches.ts +++ b/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 { - 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 { + 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 { + 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 { - 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() diff --git a/src/env/node/git/sub-providers/tags.ts b/src/env/node/git/sub-providers/tags.ts index f6edfcbcfd427..2582790cfc6a0 100644 --- a/src/env/node/git/sub-providers/tags.ts +++ b/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; diff --git a/src/git/errors.ts b/src/git/errors.ts index c6f5362de4a7b..1acbe896e147e 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -149,6 +149,7 @@ export const enum PushErrorReason { RemoteAhead, TipBehind, PushRejected, + PushRejectedRefNotExists, PushRejectedWithLease, PushRejectedWithLeaseIfIncludes, PermissionDenied, @@ -194,6 +195,11 @@ export class PushError extends Error { case PushErrorReason.PushRejected: message = `${baseMessage} because some refs failed to push or the push was rejected. Try pulling first.`; break; + case PushErrorReason.PushRejectedRefNotExists: + message = `Unable to delete remote branch${branch ? ` '${branch}'` : ''}${ + remote ? ` from ${remote}` : '' + }, the remote reference does not exist`; + break; case PushErrorReason.PushRejectedWithLease: case PushErrorReason.PushRejectedWithLeaseIfIncludes: message = `Unable to force push${branch ? ` branch '${branch}'` : ''}${ @@ -496,6 +502,84 @@ export class WorktreeDeleteError 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; + private _branch?: string; + get branch(): string | undefined { + return this._branch; + } + private _action?: string; + get action(): string | undefined { + return this._action; + } + + constructor(reason?: BranchErrorReason, original?: Error, branch?: string, action?: string); + constructor(message?: string, original?: Error); + constructor( + messageOrReason: string | BranchErrorReason | undefined, + original?: Error, + branch?: string, + action?: string, + ) { + let message; + let reason: BranchErrorReason | undefined; + if (typeof messageOrReason !== 'string') { + reason = messageOrReason as BranchErrorReason; + message = BranchError.buildErrorMessage(reason, branch, action); + } else { + message = messageOrReason; + } + super(message); + + this.original = original; + this.reason = reason; + this._branch = branch; + this._action = action; + Error.captureStackTrace?.(this, BranchError); + } + + private static buildErrorMessage(reason?: BranchErrorReason, branch?: string, action?: string): string { + let baseMessage: string; + if (action != null) { + baseMessage = `Unable to ${action} branch ${branch ? `'${branch}'` : ''}`; + } else { + 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; + } + } + + update(changes: { branch?: string; action?: string }): this { + this._branch = changes.branch === null ? undefined : changes.branch ?? this._branch; + this._action = changes.action === null ? undefined : changes.action ?? this._action; + this.message = BranchError.buildErrorMessage(this.reason, this._branch, this._action); + return this; + } +} + export const enum TagErrorReason { TagAlreadyExists, TagNotFound, @@ -512,10 +596,36 @@ export class TagError extends Error { readonly original?: Error; readonly reason: TagErrorReason | undefined; - action?: string; - tag?: string; + private _tag?: string; + get tag(): string | undefined { + return this._tag; + } + private _action?: string; + get action(): string | undefined { + return this._action; + } + + constructor(reason?: TagErrorReason, original?: Error, tag?: string, action?: string); + constructor(message?: string, original?: Error); + constructor(messageOrReason: string | TagErrorReason | undefined, original?: Error, tag?: string, action?: string) { + let message; + let reason: TagErrorReason | undefined; + if (typeof messageOrReason !== 'string') { + reason = messageOrReason as TagErrorReason; + message = TagError.buildErrorMessage(reason, tag, action); + } else { + message = messageOrReason; + } + super(message); - private static buildTagErrorMessage(reason?: TagErrorReason, tag?: string, action?: string): string { + this.original = original; + this.reason = reason; + this._tag = tag; + this._action = action; + Error.captureStackTrace?.(this, TagError); + } + + private static buildErrorMessage(reason?: TagErrorReason, tag?: string, action?: string): string { let baseMessage: string; if (action != null) { baseMessage = `Unable to ${action} tag ${tag ? `'${tag}'` : ''}`; @@ -539,37 +649,10 @@ export class TagError extends Error { } } - constructor(reason?: TagErrorReason, original?: Error, tag?: string, action?: string); - constructor(message?: string, original?: Error); - constructor(messageOrReason: string | TagErrorReason | undefined, original?: Error, tag?: string, action?: string) { - let reason: TagErrorReason | undefined; - if (typeof messageOrReason !== 'string') { - reason = messageOrReason as TagErrorReason; - } else { - super(messageOrReason); - } - const message = - typeof messageOrReason === 'string' - ? messageOrReason - : TagError.buildTagErrorMessage(messageOrReason as TagErrorReason, tag, action); - super(message); - - this.original = original; - this.reason = reason; - this.tag = tag; - this.action = action; - Error.captureStackTrace?.(this, TagError); - } - - withTag(tag: string): this { - this.tag = tag; - this.message = TagError.buildTagErrorMessage(this.reason, tag, this.action); - return this; - } - - withAction(action: string): this { - this.action = action; - this.message = TagError.buildTagErrorMessage(this.reason, this.tag, action); + update(changes: { tag?: string; action?: string }): this { + this._tag = changes.tag === null ? undefined : changes.tag ?? this._tag; + this._action = changes.action === null ? undefined : changes.action ?? this._action; + this.message = TagError.buildErrorMessage(this.reason, this._tag, this._action); return this; } } diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 0bcedd9df56f9..155ee5c7f075b 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -244,6 +244,8 @@ export interface GitBranchesSubProvider { ): Promise; createBranch?(repoPath: string, name: string, sha: string): Promise; + deleteLocalBranch?(repoPath: string, name: string, options?: { force?: boolean }): Promise; + deleteRemoteBranch?(repoPath: string, name: string, remote: string): Promise; /** * Returns whether a branch has been merged into another branch * @param repoPath The repository path