From 09b08cb56e78c6f15a2d5b3561a1903dc5a91fe0 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Thu, 26 Sep 2024 17:27:42 -0400 Subject: [PATCH] Refactors branch creation and renaming - Moves error handling outside of GitProviderService (to allow callers control over handling) - Splits unified branch command into specific calls and makes them optional Refs #3528 --- CHANGELOG.md | 2 +- src/commands/git/branch.ts | 18 ++++++++-- src/env/node/git/localGitProvider.ts | 16 ++++----- src/git/gitProvider.ts | 16 +++------ src/git/gitProviderService.ts | 34 +++++-------------- src/git/models/repository.ts | 2 -- .../providers/github/githubGitProvider.ts | 4 --- 7 files changed, 35 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3c621964078f..e02f4e0af7e88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Changed -- Change `branch create` and `branch rename` terminal-run commands into normal commands +- Changes how GitLens handles creating and renaming branches to avoid using the terminal — refs [#3528](https://github.com/gitkraken/vscode-gitlens/issues/3528) ### Fixed diff --git a/src/commands/git/branch.ts b/src/commands/git/branch.ts index 448d3c485adc8..8712422e81e89 100644 --- a/src/commands/git/branch.ts +++ b/src/commands/git/branch.ts @@ -5,11 +5,13 @@ import { getNameWithoutRemote, getReferenceLabel, isRevisionReference } from '.. import { Repository } from '../../git/models/repository'; import type { GitWorktree } from '../../git/models/worktree'; import { getWorktreesByBranch } from '../../git/models/worktree'; +import { showGenericErrorMessage } from '../../messages'; import type { QuickPickItemOfT } from '../../quickpicks/items/common'; import { createQuickPickSeparator } from '../../quickpicks/items/common'; import type { FlagsQuickPickItem } from '../../quickpicks/items/flags'; import { createFlagsQuickPickItem } from '../../quickpicks/items/flags'; import { ensureArray } from '../../system/array'; +import { Logger } from '../../system/logger'; import { pluralize } from '../../system/string'; import type { ViewsWithRepositoryFolders } from '../../views/viewBase'; import type { @@ -393,7 +395,13 @@ export class BranchGitCommand extends QuickCommand { if (state.flags.includes('--switch')) { await state.repo.switch(state.reference.ref, { createBranch: state.name }); } else { - await state.repo.git.branchCreate(state.name, state.reference.ref); + try { + await state.repo.git.createBranch(state.name, state.reference.ref); + } catch (ex) { + Logger.error(ex); + // TODO likely need some better error handling here + return showGenericErrorMessage('Unable to create branch'); + } } } } @@ -614,7 +622,13 @@ export class BranchGitCommand extends QuickCommand { state.flags = result; endSteps(state); - await state.repo.git.branchRename(state.reference.ref, state.name); + try { + await state.repo.git.renameBranch(state.reference.ref, state.name); + } catch (ex) { + Logger.error(ex); + // TODO likely need some better error handling here + return showGenericErrorMessage('Unable to rename branch'); + } } } diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index da8089b6327e5..23a51475eb05f 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -39,7 +39,6 @@ import { WorktreeDeleteErrorReason, } from '../../../git/errors'; import type { - GitBranchOptions, GitCaches, GitDir, GitProvider, @@ -1232,16 +1231,13 @@ export class LocalGitProvider implements GitProvider, Disposable { } @log() - async branch(repoPath: string, options: GitBranchOptions): Promise { - if (options?.create != null) { - return this.git.branch(repoPath, options.create.name, options.create.startRef); - } - - if (options?.rename != null) { - return this.git.branch(repoPath, '-m', options.rename.old, options.rename.new); - } + async createBranch(repoPath: string, name: string, ref: string): Promise { + await this.git.branch(repoPath, name, ref); + } - throw new Error('Invalid branch options'); + @log() + async renameBranch(repoPath: string, oldName: string, newName: string): Promise { + await this.git.branch(repoPath, '-m', oldName, newName); } @log() diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index d19b5b0ede4bc..62fe2e32b697d 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -112,21 +112,14 @@ export interface RepositoryVisibilityInfo { remotesHash?: string; } -export type GitBranchOptions = { - rename?: { - old: string; - new: string; - }; - create?: { - name: string; - startRef: string; - }; -}; - export interface GitProviderRepository { + createBranch?(repoPath: string, name: string, ref: string): Promise; + renameBranch?(repoPath: string, oldName: string, newName: string): Promise; + addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise; pruneRemote?(repoPath: string, name: string): Promise; removeRemote?(repoPath: string, name: string): Promise; + applyUnreachableCommitForPatch?( repoPath: string, ref: string, @@ -488,7 +481,6 @@ export interface GitProvider extends GitProviderRepository, Disposable { getWorkingUri(repoPath: string, uri: Uri): Promise; applyChangesToWorkingFile?(uri: GitUri, ref1?: string, ref2?: string): Promise; - branch(_repoPath: string, _options: GitBranchOptions): Promise; clone?(url: string, parentPath: string): Promise; /** * Returns the blame of a file diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 0b16924eb63e7..1bf5c1144bec7 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -19,7 +19,6 @@ import { SubscriptionPlanId } from '../constants.subscription'; import type { Container } from '../container'; import { AccessDeniedError, CancellationError, ProviderNotFoundError, ProviderNotSupportedError } from '../errors'; import type { FeatureAccess, Features, PlusFeatures, RepoFeatureAccess } from '../features'; -import { showGenericErrorMessage } from '../messages'; import { getApplicablePromo } from '../plus/gk/account/promos'; import type { Subscription } from '../plus/gk/account/subscription'; import { isSubscriptionPaidPlan } from '../plus/gk/account/subscription'; @@ -45,7 +44,6 @@ import { configuration } from '../system/vscode/configuration'; import { setContext } from '../system/vscode/context'; import { getBestPath } from '../system/vscode/path'; import type { - GitBranchOptions, GitCaches, GitDir, GitProvider, @@ -1341,35 +1339,19 @@ export class GitProviderService implements Disposable { } @log() - branchCreate(repoPath: string, name: string, startRef: string): Promise { + createBranch(repoPath: string | Uri, name: string, ref: string): Promise { const { provider, path } = this.getProvider(repoPath); - try { - return provider.branch(path, { - create: { - name: name, - startRef: startRef, - }, - }); - } catch (ex) { - Logger.error(ex); - return showGenericErrorMessage('Unable to create branch'); - } + if (provider.createBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); + + return provider.createBranch(path, name, ref); } @log() - branchRename(repoPath: string, oldName: string, newName: string): Promise { + renameBranch(repoPath: string | Uri, oldName: string, newName: string): Promise { const { provider, path } = this.getProvider(repoPath); - try { - return provider.branch(path, { - rename: { - old: oldName, - new: newName, - }, - }); - } catch (ex) { - Logger.error(ex); - return showGenericErrorMessage('Unable to rename branch'); - } + if (provider.renameBranch == null) throw new ProviderNotSupportedError(provider.descriptor.name); + + return provider.renameBranch(path, oldName, newName); } @log() diff --git a/src/git/models/repository.ts b/src/git/models/repository.ts index bfade5fc1f9c6..633462a3d414d 100644 --- a/src/git/models/repository.ts +++ b/src/git/models/repository.ts @@ -39,8 +39,6 @@ export type RepoGitProviderService = Pick< [K in keyof GitProviderService]: RemoveFirstArg; }, | keyof GitProviderRepository - | 'branchCreate' - | 'branchRename' | 'getBestRemoteWithIntegration' | 'getBranch' | 'getRemote' diff --git a/src/plus/integrations/providers/github/githubGitProvider.ts b/src/plus/integrations/providers/github/githubGitProvider.ts index 30b2b79479ff2..b740e239f7b8b 100644 --- a/src/plus/integrations/providers/github/githubGitProvider.ts +++ b/src/plus/integrations/providers/github/githubGitProvider.ts @@ -26,7 +26,6 @@ import { import { Features } from '../../../../features'; import { GitSearchError } from '../../../../git/errors'; import type { - GitBranchOptions, GitCaches, GitProvider, LeftRightCommitCountResult, @@ -444,9 +443,6 @@ export class GitHubGitProvider implements GitProvider, Disposable { return this.createVirtualUri(repoPath, undefined, uri.path); } - @log() - async branch(_repoPath: string, _options: GitBranchOptions): Promise {} - @log() async branchContainsCommit(_repoPath: string, _name: string, _ref: string): Promise { return false;