From c3639e78054b250e80f3b49838fde3814ed9d218 Mon Sep 17 00:00:00 2001 From: Sergio Date: Fri, 27 Sep 2024 13:46:51 +0200 Subject: [PATCH 01/12] add deleteBranch # Conflicts: # src/env/node/git/localGitProvider.ts --- src/commands/git/branch.ts | 15 +++++--- src/env/node/git/git.ts | 12 +++++-- src/env/node/git/localGitProvider.ts | 51 ++++++++++++++++++++++++++++ src/git/gitProvider.ts | 5 +++ src/git/gitProviderService.ts | 19 +++++++++++ src/git/models/repository.ts | 49 ++------------------------ 6 files changed, 99 insertions(+), 52 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 67f61509e8ba0..019aefb97e25f 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -558,10 +558,17 @@ 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'), - }); + + try { + await state.repo.git.deleteBranch(state.references, { + force: state.flags.includes('--force'), + remote: state.flags.includes('--remotes'), + }); + } catch (ex) { + Logger.error(ex); + // TODO likely need some better error handling here + return showGenericErrorMessage('Unable to delete branch'); + } } } diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 6f9b571ea868c..16acd25f004ab 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -12,6 +12,8 @@ import { GitErrorHandling } from '../../../git/commandOptions'; import { BlameIgnoreRevsFileBadRevisionError, BlameIgnoreRevsFileError, + BranchError, + BranchErrorReason, CherryPickError, CherryPickErrorReason, FetchError, @@ -520,7 +522,7 @@ export class Git { } } - branch(repoPath: string, ...args: string[]) { + async branch(repoPath: string, ...args: string[]): Promise { return this.git({ cwd: repoPath }, 'branch', ...args); } @@ -976,6 +978,10 @@ export class Git { publish?: boolean; remote?: string; upstream?: string; + delete?: { + remote: string; + branches: string[]; + }; }, ): Promise { const params = ['push']; @@ -1001,8 +1007,10 @@ export class Git { } else { params.push(options.remote, options.branch); } - } else if (options.remote) { + } else if (options.remote != null) { params.push(options.remote); + } else if (options.delete != null) { + params.push('-d', options.delete.remote, ...options.delete.branches); } try { diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 75f5e9b1cbfd3..8dca2e0c15259 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1285,6 +1285,57 @@ export class LocalGitProvider implements GitProvider, Disposable { await this.git.branch(repoPath, '-m', oldName, newName); } + @log() + async deleteBranch( + repoPath: string, + branches: GitBranchReference[], + options: { force?: boolean; remote?: boolean }, + ): Promise { + const localBranches = branches.filter((b: GitBranchReference) => !b.remote); + if (localBranches.length !== 0) { + const args = ['--delete']; + if (options.force) { + args.push('--force'); + } + + await this.git.branch(repoPath, ...args, ...branches.map((b: GitBranchReference) => b.ref)); + + if (options.remote) { + const trackingBranches = localBranches.filter(b => b.upstream != null); + if (trackingBranches.length !== 0) { + const branchesByOrigin = groupByMap(trackingBranches, b => + getRemoteNameFromBranchName(b.upstream!.name), + ); + + for (const [remote, branches] of branchesByOrigin.entries()) { + await this.git.push(repoPath, { + delete: { + remote: remote, + branches: branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), + }, + }); + } + } + } + } + + const remoteBranches = branches.filter((b: GitBranchReference) => b.remote); + if (remoteBranches.length !== 0) { + const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); + + for (const [remote, branches] of branchesByOrigin.entries()) { + await this.git.push(repoPath, { + delete: { + remote: remote, + branches: branches.map((b: GitBranchReference) => + b.remote ? getBranchNameWithoutRemote(b.name) : b.name, + ), + }, + }); + } + } + } + @log() async createTag(repoPath: string, name: string, ref: string, message?: string): Promise { try { diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 162279a814baf..b0c10dcb21d18 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -120,6 +120,11 @@ export interface BranchContributorOverview { export interface GitProviderRepository { createBranch?(repoPath: string, name: string, ref: string): Promise; renameBranch?(repoPath: string, oldName: string, newName: string): Promise; + deleteBranch?( + repoPath: string, + branches: GitBranchReference | GitBranchReference[], + options?: { force?: boolean; remote?: boolean }, + ): Promise; createTag?(repoPath: string, name: string, ref: string, message?: string): Promise; deleteTag?(repoPath: string, name: string): Promise; addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 479657d170dd1..296584e6a349f 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1377,6 +1377,25 @@ export class GitProviderService implements Disposable { return provider.renameBranch(path, oldName, newName); } + @log() + deleteBranch( + repoPath: string, + branches: GitBranchReference | GitBranchReference[], + options?: { force?: boolean; remote?: boolean }, + ): Promise { + const { provider, path } = this.getProvider(repoPath); + if (provider.deleteBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); + + if (!Array.isArray(branches)) { + branches = [branches]; + } + + return provider.deleteBranch(path, branches, { + force: options?.force, + remote: options?.remote, + }); + } + @log() createTag(repoPath: string | Uri, name: string, ref: string, message?: string): Promise { const { provider, path } = this.getProvider(repoPath); diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index 49c0a3c4a972c..990314bcdc3a8 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -15,7 +15,7 @@ import { debug, log, logName } from '../../system/decorators/log'; import { memoize } from '../../system/decorators/memoize'; import type { Deferrable } from '../../system/function'; import { debounce } from '../../system/function'; -import { filter, groupByMap, join, map, min, some } from '../../system/iterable'; +import { filter, join, map, min, some } from '../../system/iterable'; import { getLoggableName, Logger } from '../../system/logger'; import { getLogScope } from '../../system/logger.scope'; import { updateRecordValue } from '../../system/object'; @@ -26,9 +26,9 @@ import { configuration } from '../../system/vscode/configuration'; import type { GitProviderDescriptor, GitProviderRepository } from '../gitProvider'; import type { GitProviderService } from '../gitProviderService'; import type { GitBranch } from './branch'; -import { getBranchNameWithoutRemote, getRemoteNameFromBranchName } from './branch'; -import type { GitBranchReference, GitReference } from './reference'; +import type { GitBranchReference, GitReference, GitTagReference } from './reference'; import { getNameWithoutRemote, isBranchReference } from './reference'; +import { getBranchNameWithoutRemote, getRemoteNameFromBranchName } from './branch'; import type { GitRemote } from './remote'; import type { GitWorktree } from './worktree'; @@ -591,49 +591,6 @@ export class Repository implements Disposable { return remote; } - @log() - branchDelete(branches: GitBranchReference | GitBranchReference[], options?: { force?: boolean; remote?: boolean }) { - if (!Array.isArray(branches)) { - branches = [branches]; - } - - const localBranches = branches.filter(b => !b.remote); - if (localBranches.length !== 0) { - const args = ['--delete']; - if (options?.force) { - args.push('--force'); - } - void this.runTerminalCommand('branch', ...args, ...branches.map(b => b.ref)); - - if (options?.remote) { - const trackingBranches = localBranches.filter(b => b.upstream != null); - if (trackingBranches.length !== 0) { - const branchesByOrigin = groupByMap(trackingBranches, b => - getRemoteNameFromBranchName(b.upstream!.name), - ); - - for (const [remote, branches] of branchesByOrigin.entries()) { - void this.runTerminalCommand( - 'push', - '-d', - remote, - ...branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), - ); - } - } - } - } - - const remoteBranches = branches.filter(b => b.remote); - if (remoteBranches.length !== 0) { - const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); - - for (const [remote, branches] of branchesByOrigin.entries()) { - void this.runTerminalCommand('push', '-d', remote, ...branches.map(b => getNameWithoutRemote(b))); - } - } - } - @log() cherryPick(...args: string[]) { void this.runTerminalCommand('cherry-pick', ...args); From 0413e1c9e24ec2be9cac860e8402cbfd9c18bfc9 Mon Sep 17 00:00:00 2001 From: Sergio Date: Fri, 27 Sep 2024 15:45:17 +0200 Subject: [PATCH 02/12] add BranchError and BranchErrorReason --- src/env/node/git/git.ts | 33 ++++++++++++++++++++++++- src/git/errors.ts | 54 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 16acd25f004ab..508e155261cc2 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -75,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, @@ -523,7 +527,34 @@ export class Git { } async branch(repoPath: string, ...args: string[]): Promise { - return this.git({ cwd: repoPath }, 'branch', ...args); + try { + await this.git({ cwd: repoPath }, 'branch', ...args); + } catch (ex) { + const msg: string = ex?.toString() ?? ''; + let reason: BranchErrorReason = BranchErrorReason.Other; + switch (true) { + case GitErrors.noRemoteReference.test(msg) || GitErrors.noRemoteReference.test(ex.stderr ?? ''): + reason = BranchErrorReason.NoRemoteReference; + break; + case GitErrors.invalidBranchName.test(msg) || GitErrors.invalidBranchName.test(ex.stderr ?? ''): + reason = BranchErrorReason.InvalidBranchName; + break; + case GitErrors.branchAlreadyExists.test(msg) || GitErrors.branchAlreadyExists.test(ex.stderr ?? ''): + reason = BranchErrorReason.BranchAlreadyExists; + break; + case GitErrors.branchNotFullyMerged.test(msg) || GitErrors.branchNotFullyMerged.test(ex.stderr ?? ''): + reason = BranchErrorReason.BranchNotFullyMerged; + break; + case GitErrors.branchNotYetBorn.test(msg) || GitErrors.branchNotYetBorn.test(ex.stderr ?? ''): + reason = BranchErrorReason.BranchNotYetBorn; + break; + case GitErrors.branchFastForwardRejected.test(msg) || + GitErrors.branchFastForwardRejected.test(ex.stderr ?? ''): + reason = BranchErrorReason.BranchFastForwardRejected; + break; + } + throw new BranchError(reason, ex); + } } branch__set_upstream(repoPath: string, branch: string, remote: string, remoteBranch: string) { diff --git a/src/git/errors.ts b/src/git/errors.ts index e1ef081fdfb25..03dc3ec51202c 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -218,6 +218,60 @@ 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 message; + const baseMessage = `Unable to perform action on branch${branch ? ` '${branch}'` : ''}`; + let reason: BranchErrorReason | undefined; + if (messageOrReason == null) { + message = baseMessage; + } else if (typeof messageOrReason === 'string') { + message = messageOrReason; + reason = undefined; + } else { + reason = messageOrReason; + switch (reason) { + case BranchErrorReason.BranchAlreadyExists: + message = `${baseMessage} because it already exists.`; + break; + case BranchErrorReason.BranchNotFullyMerged: + message = `${baseMessage} because it is not fully merged.`; + break; + case BranchErrorReason.NoRemoteReference: + message = `${baseMessage} because the remote reference does not exist.`; + break; + case BranchErrorReason.InvalidBranchName: + message = `${baseMessage} because the branch name is invalid.`; + break; + default: + message = baseMessage; + } + } + super(message); + + this.original = original; + this.reason = reason; + Error.captureStackTrace?.(this, BranchError); + } +} + export const enum PullErrorReason { Conflict, GitIdentity, From 10d0ea93a8c12bd78189fc8b3c45078bae43cd60 Mon Sep 17 00:00:00 2001 From: Sergio Date: Mon, 30 Sep 2024 16:27:08 +0200 Subject: [PATCH 03/12] refactor code --- src/commands/git/branch.ts | 9 ++++++--- src/env/node/git/git.ts | 40 +++++++++++++++----------------------- src/git/errors.ts | 8 ++++---- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 019aefb97e25f..ed73fa9390079 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -1,5 +1,6 @@ import { QuickInputButtons } from 'vscode'; import type { Container } from '../../container'; +import { BranchError } from '../../git/errors'; import type { GitBranchReference, GitReference } from '../../git/models/reference'; import { getNameWithoutRemote, @@ -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(new BranchError(ex.reason, ex, state.name).message); } } } @@ -567,7 +568,9 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage('Unable to delete branch'); + return showGenericErrorMessage( + new BranchError(ex.reason, ex, state.references.map(r => r.name).join(', ')).message, + ); } } } @@ -676,7 +679,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(new BranchError(ex.reason, ex, state.name).message); } } } diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 508e155261cc2..cc55be87f9ac4 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -171,6 +171,15 @@ function getStdinUniqueKey(): number { type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true }; export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never }; +const branchErrorAndReason = [ + [GitErrors.noRemoteReference, BranchErrorReason.NoRemoteReference], + [GitErrors.invalidBranchName, BranchErrorReason.InvalidBranchName], + [GitErrors.branchAlreadyExists, BranchErrorReason.BranchAlreadyExists], + [GitErrors.branchNotFullyMerged, BranchErrorReason.BranchNotFullyMerged], + [GitErrors.branchNotYetBorn, BranchErrorReason.BranchNotYetBorn], + [GitErrors.branchFastForwardRejected, BranchErrorReason.BranchFastForwardRejected], +]; + const tagErrorAndReason: [RegExp, TagErrorReason][] = [ [GitErrors.tagAlreadyExists, TagErrorReason.TagAlreadyExists], [GitErrors.tagNotFound, TagErrorReason.TagNotFound], @@ -531,29 +540,12 @@ export class Git { await this.git({ cwd: repoPath }, 'branch', ...args); } catch (ex) { const msg: string = ex?.toString() ?? ''; - let reason: BranchErrorReason = BranchErrorReason.Other; - switch (true) { - case GitErrors.noRemoteReference.test(msg) || GitErrors.noRemoteReference.test(ex.stderr ?? ''): - reason = BranchErrorReason.NoRemoteReference; - break; - case GitErrors.invalidBranchName.test(msg) || GitErrors.invalidBranchName.test(ex.stderr ?? ''): - reason = BranchErrorReason.InvalidBranchName; - break; - case GitErrors.branchAlreadyExists.test(msg) || GitErrors.branchAlreadyExists.test(ex.stderr ?? ''): - reason = BranchErrorReason.BranchAlreadyExists; - break; - case GitErrors.branchNotFullyMerged.test(msg) || GitErrors.branchNotFullyMerged.test(ex.stderr ?? ''): - reason = BranchErrorReason.BranchNotFullyMerged; - break; - case GitErrors.branchNotYetBorn.test(msg) || GitErrors.branchNotYetBorn.test(ex.stderr ?? ''): - reason = BranchErrorReason.BranchNotYetBorn; - break; - case GitErrors.branchFastForwardRejected.test(msg) || - GitErrors.branchFastForwardRejected.test(ex.stderr ?? ''): - reason = BranchErrorReason.BranchFastForwardRejected; - break; + for (const [error, reason] of branchErrorAndReason) { + if (error.test(msg) || error.test(ex.stderr ?? '')) { + throw new BranchError(reason, ex); + } } - throw new BranchError(reason, ex); + throw new BranchError(BranchErrorReason.Other, ex); } } @@ -1038,9 +1030,9 @@ export class Git { } else { params.push(options.remote, options.branch); } - } else if (options.remote != null) { + } else if (options.remote) { params.push(options.remote); - } else if (options.delete != null) { + } else if (options.delete) { params.push('-d', options.delete.remote, ...options.delete.branches); } diff --git a/src/git/errors.ts b/src/git/errors.ts index 03dc3ec51202c..d1c39b032ee76 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -249,16 +249,16 @@ export class BranchError extends Error { reason = messageOrReason; switch (reason) { case BranchErrorReason.BranchAlreadyExists: - message = `${baseMessage} because it already exists.`; + message = `${baseMessage} because it already exists`; break; case BranchErrorReason.BranchNotFullyMerged: - message = `${baseMessage} because it is not fully merged.`; + message = `${baseMessage} because it is not fully merged`; break; case BranchErrorReason.NoRemoteReference: - message = `${baseMessage} because the remote reference does not exist.`; + message = `${baseMessage} because the remote reference does not exist`; break; case BranchErrorReason.InvalidBranchName: - message = `${baseMessage} because the branch name is invalid.`; + message = `${baseMessage} because the branch name is invalid`; break; default: message = baseMessage; From e8ff53bda54e901b222d2021a7cbf3b3e31d2394 Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 1 Oct 2024 11:12:29 +0200 Subject: [PATCH 04/12] rename deleteBranch to deleteBranches --- src/commands/git/branch.ts | 2 +- src/env/node/git/localGitProvider.ts | 3 +-- src/git/gitProvider.ts | 2 +- src/git/gitProviderService.ts | 6 +++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index ed73fa9390079..d66278ce0470c 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -561,7 +561,7 @@ export class BranchGitCommand extends QuickCommand { endSteps(state); try { - await state.repo.git.deleteBranch(state.references, { + await state.repo.git.deleteBranches(state.references, { force: state.flags.includes('--force'), remote: state.flags.includes('--remotes'), }); diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 8dca2e0c15259..8f897b4a069dd 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1286,7 +1286,7 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - async deleteBranch( + async deleteBranches( repoPath: string, branches: GitBranchReference[], options: { force?: boolean; remote?: boolean }, @@ -1319,7 +1319,6 @@ export class LocalGitProvider implements GitProvider, Disposable { } } - const remoteBranches = branches.filter((b: GitBranchReference) => b.remote); if (remoteBranches.length !== 0) { const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index b0c10dcb21d18..f71df2767816c 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -120,7 +120,7 @@ export interface BranchContributorOverview { export interface GitProviderRepository { createBranch?(repoPath: string, name: string, ref: string): Promise; renameBranch?(repoPath: string, oldName: string, newName: string): Promise; - deleteBranch?( + deleteBranches?( repoPath: string, branches: GitBranchReference | GitBranchReference[], options?: { force?: boolean; remote?: boolean }, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 296584e6a349f..af7915b1797ab 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1378,19 +1378,19 @@ export class GitProviderService implements Disposable { } @log() - deleteBranch( + deleteBranches( repoPath: string, branches: GitBranchReference | GitBranchReference[], options?: { force?: boolean; remote?: boolean }, ): Promise { const { provider, path } = this.getProvider(repoPath); - if (provider.deleteBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); + if (provider.deleteBranches == null) throw new ProviderNotSupportedError(provider.descriptor.name); if (!Array.isArray(branches)) { branches = [branches]; } - return provider.deleteBranch(path, branches, { + return provider.deleteBranches(path, branches, { force: options?.force, remote: options?.remote, }); From 3f2d168545b7ceb305ef4edd8662434fa72aaecf Mon Sep 17 00:00:00 2001 From: Sergio Date: Tue, 1 Oct 2024 17:37:53 +0200 Subject: [PATCH 05/12] safe delete branch # Conflicts: # src/env/node/git/git.ts --- src/env/node/git/git.ts | 15 ++++++++++- src/env/node/git/localGitProvider.ts | 38 +++++++++++++++++++++++----- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index cc55be87f9ac4..c5092c8150d15 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -1622,7 +1622,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 { const params = ['rev-list']; if (options?.all) { @@ -1633,6 +1638,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'); } @@ -1902,6 +1911,10 @@ export class Git { return data.length === 0 ? undefined : data.trim(); } + async update_ref(repoPath: string, ...args: string[]): Promise { + await this.git({ cwd: repoPath }, 'update-ref', ...args); + } + show( repoPath: string, options?: { cancellation?: CancellationToken; configs?: readonly string[] }, diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 8f897b4a069dd..438248c3529ca 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1298,8 +1298,6 @@ export class LocalGitProvider implements GitProvider, Disposable { args.push('--force'); } - await this.git.branch(repoPath, ...args, ...branches.map((b: GitBranchReference) => b.ref)); - if (options.remote) { const trackingBranches = localBranches.filter(b => b.upstream != null); if (trackingBranches.length !== 0) { @@ -1308,17 +1306,43 @@ export class LocalGitProvider implements GitProvider, Disposable { ); for (const [remote, branches] of branchesByOrigin.entries()) { - await this.git.push(repoPath, { - delete: { - remote: remote, - branches: branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), - }, + const remoteCommitByBranch: Map = {}; + branches.forEach(async b => { + remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${b.ref}`, { + maxResults: 1, + }); + remoteCommitByBranch[b.ref] = remoteCommit; }); + + await this.git.branch( + repoPath, + '--delete', + '--remotes', + ...branches.map((b: GitBranchReference) => `${remote}/${b.ref}`), + ); + + try { + await this.git.branch(repoPath, ...args, ...branches.map((b: GitBranchReference) => b.ref)); + await this.git.push(repoPath, { + delete: { + remote: remote, + branches: branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), + }, + }); + } catch (ex) { + // If it fails, restore the remote branches + remoteCommitByBranch.forEach(async (branch, commit) => { + await this.git.update_ref(repoPath, `refs/remotes/${remote}/${branch}`, commit); + await this.git.branch__set_upstream(repoPath, branch, remote, branch); + }); + throw ex; + } } } } } + const remoteBranches = branches.filter((b: GitBranchReference) => b.remote); if (remoteBranches.length !== 0) { const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); From 3cb86258b02d92546832842f27e7172395436914 Mon Sep 17 00:00:00 2001 From: Sergio Date: Wed, 2 Oct 2024 15:42:29 +0200 Subject: [PATCH 06/12] add message builder to BranchError --- src/git/errors.ts | 51 ++++++++++++++++++++++++----------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/git/errors.ts b/src/git/errors.ts index d1c39b032ee76..f9df6a6ca32d6 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -237,39 +237,40 @@ export class BranchError extends Error { constructor(reason?: BranchErrorReason, original?: Error, branch?: string); constructor(message?: string, original?: Error); constructor(messageOrReason: string | BranchErrorReason | undefined, original?: Error, branch?: string) { - let message; - const baseMessage = `Unable to perform action on branch${branch ? ` '${branch}'` : ''}`; let reason: BranchErrorReason | undefined; - if (messageOrReason == null) { - message = baseMessage; - } else if (typeof messageOrReason === 'string') { - message = messageOrReason; - reason = undefined; - } else { - reason = messageOrReason; - switch (reason) { - case BranchErrorReason.BranchAlreadyExists: - message = `${baseMessage} because it already exists`; - break; - case BranchErrorReason.BranchNotFullyMerged: - message = `${baseMessage} because it is not fully merged`; - break; - case BranchErrorReason.NoRemoteReference: - message = `${baseMessage} because the remote reference does not exist`; - break; - case BranchErrorReason.InvalidBranchName: - message = `${baseMessage} because the branch name is invalid`; - break; - default: - message = baseMessage; - } + 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 on branch${branch ? ` '${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 { From cdec8c9299d6aa3ca09bee2d9c6d19237bec2a1d Mon Sep 17 00:00:00 2001 From: Sergio Date: Wed, 2 Oct 2024 15:47:00 +0200 Subject: [PATCH 07/12] convert deleteBranches into deleteBranch and call multiple times --- src/commands/git/branch.ts | 26 ++++---- src/env/node/git/git.ts | 4 +- src/env/node/git/localGitProvider.ts | 98 ++++++++++------------------ src/git/gitProvider.ts | 4 +- src/git/gitProviderService.ts | 12 ++-- 5 files changed, 57 insertions(+), 87 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index d66278ce0470c..8100d7210c620 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -428,7 +428,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage(new BranchError(ex.reason, ex, state.name).message); + return showGenericErrorMessage(ex.WithBranch(state.name)); } } } @@ -560,17 +560,17 @@ export class BranchGitCommand extends QuickCommand { endSteps(state); - try { - await state.repo.git.deleteBranches(state.references, { - force: state.flags.includes('--force'), - remote: state.flags.includes('--remotes'), - }); - } catch (ex) { - Logger.error(ex); - // TODO likely need some better error handling here - return showGenericErrorMessage( - new BranchError(ex.reason, ex, state.references.map(r => r.name).join(', ')).message, - ); + 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) { + Logger.error(ex); + // TODO likely need some better error handling here + return showGenericErrorMessage(ex.WithBranch(ref.name)); + } } } } @@ -679,7 +679,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage(new BranchError(ex.reason, ex, state.name).message); + return showGenericErrorMessage(ex.WithBranch(state.name)); } } } diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index c5092c8150d15..518105772c537 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -1003,7 +1003,7 @@ export class Git { upstream?: string; delete?: { remote: string; - branches: string[]; + branch: string; }; }, ): Promise { @@ -1033,7 +1033,7 @@ export class Git { } else if (options.remote) { params.push(options.remote); } else if (options.delete) { - params.push('-d', options.delete.remote, ...options.delete.branches); + params.push('-d', options.delete.remote, options.delete.branch); } try { diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 438248c3529ca..b2942ff1ad048 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1286,77 +1286,51 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - async deleteBranches( + async deleteBranch( repoPath: string, - branches: GitBranchReference[], + branch: GitBranchReference, options: { force?: boolean; remote?: boolean }, ): Promise { - const localBranches = branches.filter((b: GitBranchReference) => !b.remote); - if (localBranches.length !== 0) { - const args = ['--delete']; - if (options.force) { - args.push('--force'); - } - - if (options.remote) { - const trackingBranches = localBranches.filter(b => b.upstream != null); - if (trackingBranches.length !== 0) { - const branchesByOrigin = groupByMap(trackingBranches, b => - getRemoteNameFromBranchName(b.upstream!.name), - ); - - for (const [remote, branches] of branchesByOrigin.entries()) { - const remoteCommitByBranch: Map = {}; - branches.forEach(async b => { - remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${b.ref}`, { - maxResults: 1, - }); - remoteCommitByBranch[b.ref] = remoteCommit; - }); + if (branch.remote) { + return this.git.push(repoPath, { + delete: { + remote: getRemoteNameFromBranchName(branch.name), + branch: branch.remote ? getBranchNameWithoutRemote(branch.name) : branch.name, + }, + }); + } - await this.git.branch( - repoPath, - '--delete', - '--remotes', - ...branches.map((b: GitBranchReference) => `${remote}/${b.ref}`), - ); + const args = ['--delete']; + if (options.force) { + args.push('--force'); + } - try { - await this.git.branch(repoPath, ...args, ...branches.map((b: GitBranchReference) => b.ref)); - await this.git.push(repoPath, { - delete: { - remote: remote, - branches: branches.map(b => getBranchNameWithoutRemote(b.upstream!.name)), - }, - }); - } catch (ex) { - // If it fails, restore the remote branches - remoteCommitByBranch.forEach(async (branch, commit) => { - await this.git.update_ref(repoPath, `refs/remotes/${remote}/${branch}`, commit); - await this.git.branch__set_upstream(repoPath, branch, remote, branch); - }); - throw ex; - } - } - } - } + if (!options.remote || !branch.upstream) { + return this.git.branch(repoPath, ...args, branch.ref); } - const remoteBranches = branches.filter((b: GitBranchReference) => b.remote); - if (remoteBranches.length !== 0) { - const branchesByOrigin = groupByMap(remoteBranches, b => getRemoteNameFromBranchName(b.name)); + const remote = getRemoteNameFromBranchName(branch.upstream.name); + remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { + maxResults: 1, + }); - for (const [remote, branches] of branchesByOrigin.entries()) { - await this.git.push(repoPath, { - delete: { - remote: remote, - branches: branches.map((b: GitBranchReference) => - b.remote ? getBranchNameWithoutRemote(b.name) : b.name, - ), - }, - }); - } + 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}`, commit); + await this.git.branch__set_upstream(repoPath, branch, remote, branch); + throw ex; } + + await this.git.push(repoPath, { + delete: { + remote: remote, + branch: getBranchNameWithoutRemote(branch.upstream.name), + }, + }); } @log() diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index f71df2767816c..543347130c621 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -120,9 +120,9 @@ export interface BranchContributorOverview { export interface GitProviderRepository { createBranch?(repoPath: string, name: string, ref: string): Promise; renameBranch?(repoPath: string, oldName: string, newName: string): Promise; - deleteBranches?( + deleteBranch?( repoPath: string, - branches: GitBranchReference | GitBranchReference[], + branches: GitBranchReference, options?: { force?: boolean; remote?: boolean }, ): Promise; createTag?(repoPath: string, name: string, ref: string, message?: string): Promise; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index af7915b1797ab..e6d018e302460 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -1378,19 +1378,15 @@ export class GitProviderService implements Disposable { } @log() - deleteBranches( + deleteBranch( repoPath: string, - branches: GitBranchReference | GitBranchReference[], + branch: GitBranchReference, options?: { force?: boolean; remote?: boolean }, ): Promise { const { provider, path } = this.getProvider(repoPath); - if (provider.deleteBranches == null) throw new ProviderNotSupportedError(provider.descriptor.name); + if (provider.deleteBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); - if (!Array.isArray(branches)) { - branches = [branches]; - } - - return provider.deleteBranches(path, branches, { + return provider.deleteBranch(path, branch, { force: options?.force, remote: options?.remote, }); From 33bfb9d4bae98ffb6b562a5db28245a6bc520140 Mon Sep 17 00:00:00 2001 From: Sergio Date: Thu, 3 Oct 2024 12:19:52 +0200 Subject: [PATCH 08/12] add message & sort supress keys # Conflicts: # src/config.ts --- package.json | 11 ++++++----- src/messages.ts | 12 ++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 5fdf08bb21c93..b39cf1a5d9153 100644 --- a/package.json +++ b/package.json @@ -4717,22 +4717,23 @@ "gitlens.advanced.messages": { "type": "object", "default": { + "suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false, + "suppressBlameInvalidIgnoreRevsFileWarning": false, "suppressCommitHasNoPreviousCommitWarning": false, "suppressCommitNotFoundWarning": false, "suppressCreatePullRequestPrompt": false, "suppressDebugLoggingWarning": false, "suppressFileNotUnderSourceControlWarning": false, + "suppressGitBranchNotFullyMergedWarning": false, "suppressGitDisabledWarning": false, "suppressGitMissingWarning": false, "suppressGitVersionWarning": false, - "suppressLineUncommittedWarning": false, - "suppressNoRepositoryWarning": false, - "suppressRebaseSwitchToTextWarning": false, "suppressIntegrationDisconnectedTooManyFailedRequestsWarning": false, "suppressIntegrationRequestFailed500Warning": false, "suppressIntegrationRequestTimedOutWarning": false, - "suppressBlameInvalidIgnoreRevsFileWarning": false, - "suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false + "suppressLineUncommittedWarning": false, + "suppressNoRepositoryWarning": false, + "suppressRebaseSwitchToTextWarning": false }, "properties": { "suppressCommitHasNoPreviousCommitWarning": { diff --git a/src/messages.ts b/src/messages.ts index 5027cd29eac8b..500a824c965af 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -138,6 +138,18 @@ export function showGitVersionUnsupportedErrorMessage( ); } +export async function showGitBranchNotFullyMergedPrompt(branchName: string) { + const confirm = { title: 'Retry with --force flag' }; + const result = await showMessage( + 'warn', + `Unable to delete branch '${branchName}'. It is not fully merged.`, + 'suppressGitBranchNotFullyMergedWarning', + { title: "Don't Show Again" }, + confirm, + ); + return result === confirm; +} + export async function showPreReleaseExpiredErrorMessage(version: string) { const upgrade = { title: 'Upgrade' }; const switchToRelease = { title: 'Switch to Release Version' }; From 1279905783b3cddabd287a73362afc8f9e8c306f Mon Sep 17 00:00:00 2001 From: Sergio Date: Thu, 3 Oct 2024 12:22:28 +0200 Subject: [PATCH 09/12] handle BranchError in localGitProvider # Conflicts: # src/env/node/git/localGitProvider.ts --- src/commands/git/branch.ts | 8 +-- src/env/node/git/localGitProvider.ts | 99 ++++++++++++++++++---------- src/git/errors.ts | 2 +- 3 files changed, 68 insertions(+), 41 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 8100d7210c620..ffc92965399dc 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -428,7 +428,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage(ex.WithBranch(state.name)); + return showGenericErrorMessage(ex); } } } @@ -567,9 +567,9 @@ export class BranchGitCommand extends QuickCommand { remote: state.flags.includes('--remotes'), }); } catch (ex) { - Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage(ex.WithBranch(ref.name)); + Logger.error(ex); + return showGenericErrorMessage(ex); } } } @@ -679,7 +679,7 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { Logger.error(ex); // TODO likely need some better error handling here - return showGenericErrorMessage(ex.WithBranch(state.name)); + return showGenericErrorMessage(ex); } } } diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index b2942ff1ad048..b3b19b2de0185 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -24,6 +24,7 @@ import { ApplyPatchCommitErrorReason, BlameIgnoreRevsFileBadRevisionError, BlameIgnoreRevsFileError, + BranchError, CherryPickError, CherryPickErrorReason, FetchError, @@ -1276,13 +1277,29 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - async createBranch(repoPath: string, name: string, ref: string): Promise { - await this.git.branch(repoPath, name, ref); + createBranch(repoPath: string, name: string, ref: string): Promise { + try { + return void this.git.branch(repoPath, name, ref); + } catch (ex) { + if (ex instanceof BranchError) { + throw ex.WithBranch(branch.name); + } + + throw ex; + } } @log() - async renameBranch(repoPath: string, oldName: string, newName: string): Promise { - await this.git.branch(repoPath, '-m', oldName, newName); + renameBranch(repoPath: string, oldName: string, newName: string): Promise { + try { + return void this.git.branch(repoPath, '-m', oldName, newName); + } catch (ex) { + if (ex instanceof BranchError) { + throw ex.WithBranch(branch.name); + } + + throw ex; + } } @log() @@ -1291,46 +1308,56 @@ export class LocalGitProvider implements GitProvider, Disposable { branch: GitBranchReference, options: { force?: boolean; remote?: boolean }, ): Promise { - if (branch.remote) { - return this.git.push(repoPath, { - delete: { - remote: getRemoteNameFromBranchName(branch.name), - branch: branch.remote ? getBranchNameWithoutRemote(branch.name) : branch.name, - }, - }); - } + 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'); - } + const args = ['--delete']; + if (options.force) { + args.push('--force'); + } - if (!options.remote || !branch.upstream) { - return this.git.branch(repoPath, ...args, branch.ref); - } + if (!options.remote || !branch.upstream) { + await this.git.branch(repoPath, ...args, branch.ref); + return; + } - const remote = getRemoteNameFromBranchName(branch.upstream.name); - remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { - maxResults: 1, - }); + const remote = getRemoteNameFromBranchName(branch.upstream.name); + remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { + maxResults: 1, + }); - await this.git.branch(repoPath, '--delete', '--remotes', `${remote}/${branch.ref}`); + await this.git.branch(repoPath, '--delete', '--remotes', `${remote}/${branch.ref}`); - try { - await this.git.branch(repoPath, ...args, 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}`, commit); + await this.git.branch__set_upstream(repoPath, branch, remote, branch); + throw ex; + } + + await this.git.push(repoPath, { + delete: { + remote: remote, + branch: getBranchNameWithoutRemote(branch.upstream.name), + }, + }); } catch (ex) { - // If it fails, restore the remote branch - await this.git.update_ref(repoPath, `refs/remotes/${remote}/${branch.ref}`, commit); - await this.git.branch__set_upstream(repoPath, branch, remote, branch); + if (ex instanceof BranchError) { + throw ex.WithBranch(branch.name); + } + throw ex; } - - await this.git.push(repoPath, { - delete: { - remote: remote, - branch: getBranchNameWithoutRemote(branch.upstream.name), - }, - }); } @log() diff --git a/src/git/errors.ts b/src/git/errors.ts index f9df6a6ca32d6..e363610982d27 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -252,7 +252,7 @@ export class BranchError extends Error { } private static buildBranchErrorMessage(reason?: BranchErrorReason, branch?: string): string { - const baseMessage = `Unable to perform action on branch${branch ? ` '${branch}'` : ''}`; + const baseMessage = `Unable to perform action ${branch ? `with branch '${branch}'` : 'on branch'}`; switch (reason) { case BranchErrorReason.BranchAlreadyExists: return `${baseMessage} because it already exists`; From 149bdcbc9144151a3d523fe50c04b081252c5452 Mon Sep 17 00:00:00 2001 From: Sergio Date: Thu, 3 Oct 2024 15:02:32 +0200 Subject: [PATCH 10/12] prompt to retry with force --- src/commands/git/branch.ts | 22 +++++++++++++++++++--- src/messages.ts | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index ffc92965399dc..e251884f02983 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -1,6 +1,6 @@ import { QuickInputButtons } from 'vscode'; import type { Container } from '../../container'; -import { BranchError } from '../../git/errors'; +import { BranchError, BranchErrorReason } from '../../git/errors'; import type { GitBranchReference, GitReference } from '../../git/models/reference'; import { getNameWithoutRemote, @@ -11,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'; @@ -569,7 +569,23 @@ export class BranchGitCommand extends QuickCommand { } catch (ex) { // TODO likely need some better error handling here Logger.error(ex); - return showGenericErrorMessage(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); } } } diff --git a/src/messages.ts b/src/messages.ts index 500a824c965af..b6aba9a0ca3a7 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -138,7 +138,7 @@ export function showGitVersionUnsupportedErrorMessage( ); } -export async function showGitBranchNotFullyMergedPrompt(branchName: string) { +export async function showGitBranchNotFullyMergedPrompt(branchName: string): Promise { const confirm = { title: 'Retry with --force flag' }; const result = await showMessage( 'warn', From 0d9b1d0199e7be8e88426fc414c05ad1aaa43ecd Mon Sep 17 00:00:00 2001 From: Sergio Date: Wed, 23 Oct 2024 12:37:33 +0200 Subject: [PATCH 11/12] rebase, lint & code fixes --- package.json | 11 +++++------ src/env/node/git/git.ts | 4 +--- src/env/node/git/localGitProvider.ts | 18 +++++++++--------- src/git/models/repository.ts | 5 ++--- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index b39cf1a5d9153..5fdf08bb21c93 100644 --- a/package.json +++ b/package.json @@ -4717,23 +4717,22 @@ "gitlens.advanced.messages": { "type": "object", "default": { - "suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false, - "suppressBlameInvalidIgnoreRevsFileWarning": false, "suppressCommitHasNoPreviousCommitWarning": false, "suppressCommitNotFoundWarning": false, "suppressCreatePullRequestPrompt": false, "suppressDebugLoggingWarning": false, "suppressFileNotUnderSourceControlWarning": false, - "suppressGitBranchNotFullyMergedWarning": false, "suppressGitDisabledWarning": false, "suppressGitMissingWarning": false, "suppressGitVersionWarning": false, + "suppressLineUncommittedWarning": false, + "suppressNoRepositoryWarning": false, + "suppressRebaseSwitchToTextWarning": false, "suppressIntegrationDisconnectedTooManyFailedRequestsWarning": false, "suppressIntegrationRequestFailed500Warning": false, "suppressIntegrationRequestTimedOutWarning": false, - "suppressLineUncommittedWarning": false, - "suppressNoRepositoryWarning": false, - "suppressRebaseSwitchToTextWarning": false + "suppressBlameInvalidIgnoreRevsFileWarning": false, + "suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false }, "properties": { "suppressCommitHasNoPreviousCommitWarning": { diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 518105772c537..d92cdf81f7afb 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -171,13 +171,11 @@ function getStdinUniqueKey(): number { type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true }; export type PushForceOptions = { withLease: true; ifIncludes?: boolean } | { withLease: false; ifIncludes?: never }; -const branchErrorAndReason = [ +const branchErrorAndReason: [RegExp, BranchErrorReason][] = [ [GitErrors.noRemoteReference, BranchErrorReason.NoRemoteReference], [GitErrors.invalidBranchName, BranchErrorReason.InvalidBranchName], [GitErrors.branchAlreadyExists, BranchErrorReason.BranchAlreadyExists], [GitErrors.branchNotFullyMerged, BranchErrorReason.BranchNotFullyMerged], - [GitErrors.branchNotYetBorn, BranchErrorReason.BranchNotYetBorn], - [GitErrors.branchFastForwardRejected, BranchErrorReason.BranchFastForwardRejected], ]; const tagErrorAndReason: [RegExp, TagErrorReason][] = [ diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index b3b19b2de0185..a329335ddefef 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -1277,12 +1277,12 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - createBranch(repoPath: string, name: string, ref: string): Promise { + async createBranch(repoPath: string, name: string, ref: string): Promise { try { - return void this.git.branch(repoPath, name, ref); + await this.git.branch(repoPath, name, ref); } catch (ex) { if (ex instanceof BranchError) { - throw ex.WithBranch(branch.name); + throw ex.WithBranch(name); } throw ex; @@ -1290,12 +1290,12 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - renameBranch(repoPath: string, oldName: string, newName: string): Promise { + async renameBranch(repoPath: string, oldName: string, newName: string): Promise { try { - return void this.git.branch(repoPath, '-m', oldName, newName); + await this.git.branch(repoPath, '-m', oldName, newName); } catch (ex) { if (ex instanceof BranchError) { - throw ex.WithBranch(branch.name); + throw ex.WithBranch(oldName); } throw ex; @@ -1330,7 +1330,7 @@ export class LocalGitProvider implements GitProvider, Disposable { } const remote = getRemoteNameFromBranchName(branch.upstream.name); - remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { + const remoteCommit = await this.git.rev_list(repoPath, `refs/remotes/${remote}/${branch.ref}`, { maxResults: 1, }); @@ -1340,8 +1340,8 @@ export class LocalGitProvider implements GitProvider, Disposable { 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}`, commit); - await this.git.branch__set_upstream(repoPath, branch, 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; } diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index 990314bcdc3a8..ecb236631f355 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -26,9 +26,8 @@ import { configuration } from '../../system/vscode/configuration'; import type { GitProviderDescriptor, GitProviderRepository } from '../gitProvider'; import type { GitProviderService } from '../gitProviderService'; import type { GitBranch } from './branch'; -import type { GitBranchReference, GitReference, GitTagReference } from './reference'; -import { getNameWithoutRemote, isBranchReference } from './reference'; -import { getBranchNameWithoutRemote, getRemoteNameFromBranchName } from './branch'; +import type { GitBranchReference, GitReference } from './reference'; +import { isBranchReference } from './reference'; import type { GitRemote } from './remote'; import type { GitWorktree } from './worktree'; From f502b4947aa13dffb07ca56d8ca2fed7d9c2ba26 Mon Sep 17 00:00:00 2001 From: Sergio Date: Mon, 11 Nov 2024 15:25:38 +0100 Subject: [PATCH 12/12] handle delete remote ref that does not exist err --- src/env/node/git/git.ts | 11 ++++++++++- src/git/errors.ts | 6 ++++++ src/messages.ts | 4 ++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index d92cdf81f7afb..cc9939057dbfd 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -1052,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; } @@ -1066,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, + ); } } diff --git a/src/git/errors.ts b/src/git/errors.ts index e363610982d27..16a4e5de4e496 100644 --- a/src/git/errors.ts +++ b/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; diff --git a/src/messages.ts b/src/messages.ts index b6aba9a0ca3a7..6702cd2f39dde 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -143,8 +143,8 @@ export async function showGitBranchNotFullyMergedPrompt(branchName: string): Pro const result = await showMessage( 'warn', `Unable to delete branch '${branchName}'. It is not fully merged.`, - 'suppressGitBranchNotFullyMergedWarning', - { title: "Don't Show Again" }, + undefined, + null, confirm, ); return result === confirm;