Skip to content

Commit

Permalink
Refactors branch creation and renaming
Browse files Browse the repository at this point in the history
 - 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
  • Loading branch information
eamodio committed Sep 27, 2024
1 parent 5b5d0de commit 28e7774
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 57 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 16 additions & 2 deletions src/commands/git/branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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');
}
}
}
}
Expand Down Expand Up @@ -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');
}
}
}

Expand Down
16 changes: 6 additions & 10 deletions src/env/node/git/localGitProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import {
WorktreeDeleteErrorReason,
} from '../../../git/errors';
import type {
GitBranchOptions,
GitCaches,
GitDir,
GitProvider,
Expand Down Expand Up @@ -1232,16 +1231,13 @@ export class LocalGitProvider implements GitProvider, Disposable {
}

@log()
async branch(repoPath: string, options: GitBranchOptions): Promise<void> {
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<void> {
await this.git.branch(repoPath, name, ref);
}

throw new Error('Invalid branch options');
@log()
async renameBranch(repoPath: string, oldName: string, newName: string): Promise<void> {
await this.git.branch(repoPath, '-m', oldName, newName);
}

@log()
Expand Down
16 changes: 4 additions & 12 deletions src/git/gitProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
renameBranch?(repoPath: string, oldName: string, newName: string): Promise<void>;

addRemote?(repoPath: string, name: string, url: string, options?: { fetch?: boolean }): Promise<void>;
pruneRemote?(repoPath: string, name: string): Promise<void>;
removeRemote?(repoPath: string, name: string): Promise<void>;

applyUnreachableCommitForPatch?(
repoPath: string,
ref: string,
Expand Down Expand Up @@ -488,7 +481,6 @@ export interface GitProvider extends GitProviderRepository, Disposable {
getWorkingUri(repoPath: string, uri: Uri): Promise<Uri | undefined>;

applyChangesToWorkingFile?(uri: GitUri, ref1?: string, ref2?: string): Promise<void>;
branch(_repoPath: string, _options: GitBranchOptions): Promise<void>;
clone?(url: string, parentPath: string): Promise<string | undefined>;
/**
* Returns the blame of a file
Expand Down
34 changes: 8 additions & 26 deletions src/git/gitProviderService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -1341,35 +1339,19 @@ export class GitProviderService implements Disposable {
}

@log()
branchCreate(repoPath: string, name: string, startRef: string): Promise<void> {
createBranch(repoPath: string | Uri, name: string, ref: string): Promise<void> {
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<void> {
renameBranch(repoPath: string | Uri, oldName: string, newName: string): Promise<void> {
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()
Expand Down
2 changes: 0 additions & 2 deletions src/git/models/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ export type RepoGitProviderService = Pick<
[K in keyof GitProviderService]: RemoveFirstArg<GitProviderService[K]>;
},
| keyof GitProviderRepository
| 'branchCreate'
| 'branchRename'
| 'getBestRemoteWithIntegration'
| 'getBranch'
| 'getRemote'
Expand Down
4 changes: 0 additions & 4 deletions src/plus/integrations/providers/github/githubGitProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
import { Features } from '../../../../features';
import { GitSearchError } from '../../../../git/errors';
import type {
GitBranchOptions,
GitCaches,
GitProvider,
LeftRightCommitCountResult,
Expand Down Expand Up @@ -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<void> {}

@log()
async branchContainsCommit(_repoPath: string, _name: string, _ref: string): Promise<boolean> {
return false;
Expand Down

0 comments on commit 28e7774

Please sign in to comment.