From eab5948f3be24bc6a177343f5ff785e637b0300f Mon Sep 17 00:00:00 2001 From: Sergio Date: Thu, 3 Oct 2024 12:22:28 +0200 Subject: [PATCH] handle BranchError in localGitProvider --- src/commands/git/branch.ts | 8 +- src/env/node/git/localGitProvider.ts | 111 +++++++++++++++------------ src/git/errors.ts | 2 +- 3 files changed, 69 insertions(+), 52 deletions(-) diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 7feca4ecb514a..35256b359743e 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -401,7 +401,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); } } } @@ -525,9 +525,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); } } } @@ -637,7 +637,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 45033ea5bb553..6712f64b86903 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -23,6 +23,7 @@ import { ApplyPatchCommitErrorReason, BlameIgnoreRevsFileBadRevisionError, BlameIgnoreRevsFileError, + BranchError, CherryPickError, CherryPickErrorReason, FetchError, @@ -182,17 +183,7 @@ import { countStringLength, filterMap } from '../../../system/array'; import { gate } from '../../../system/decorators/gate'; import { debug, log } from '../../../system/decorators/log'; import { debounce } from '../../../system/function'; -import { - filterMap as filterMapIterable, - find, - first, - groupByMap, - join, - last, - map, - skip, - some, -} from '../../../system/iterable'; +import { filterMap as filterMapIterable, find, first, join, last, map, skip, some } from '../../../system/iterable'; import { Logger } from '../../../system/logger'; import type { LogScope } from '../../../system/logger.scope'; import { getLogScope, setLogScopeExit } from '../../../system/logger.scope'; @@ -1241,13 +1232,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() @@ -1256,46 +1263,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 45a73c747389e..217ad694996e2 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`;