Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add git delete branch command #25862

Merged
merged 4 commits into from
May 24, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions extensions/git/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@
"title": "%command.branch%",
"category": "Git"
},
{
"command": "git.deleteBranch",
"title": "%command.deleteBranch%",
"category": "Git"
},
{
"command": "git.pull",
"title": "%command.pull%",
Expand Down Expand Up @@ -298,6 +303,10 @@
"command": "git.branch",
"when": "config.git.enabled && scmProvider == git && gitState == idle"
},
{
"command": "git.deleteBranch",
"when": "config.git.enabled && scmProvider == git && gitState == idle"
},
{
"command": "git.pull",
"when": "config.git.enabled && scmProvider == git && gitState == idle"
Expand Down
1 change: 1 addition & 0 deletions extensions/git/package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"command.undoCommit": "Undo Last Commit",
"command.checkout": "Checkout to...",
"command.branch": "Create Branch...",
"command.deleteBranch": "Delete Branch...",
"command.pull": "Pull",
"command.pullRebase": "Pull (Rebase)",
"command.push": "Push",
Expand Down
39 changes: 39 additions & 0 deletions extensions/git/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,26 @@ class CheckoutRemoteHeadItem extends CheckoutItem {
}
}

class BranchDeleteItem implements QuickPickItem {

protected get shortCommit(): string { return (this.ref.commit || '').substr(0, 8); }
protected get treeish(): string | undefined { return this.ref.name; }
get label(): string { return this.ref.name || this.shortCommit; }
get description(): string { return this.shortCommit; }

constructor(protected ref: Ref) { }

async run(model: Model): Promise<void> {
const ref = this.treeish;

if (!ref) {
return;
}

await model.deleteBranch(ref);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A common error from this operation happens when the branch isn't yet merged:

image

It would be great if the underlying git library would catch this situation, wrap it neatly in a GitError with a new GitErrorCode and we try catch it here, prompting the user Hey, the branch isn't yet merge. Do you still want to delete it?.

}
}

interface Command {
commandId: string;
key: string;
Expand Down Expand Up @@ -671,6 +691,25 @@ export class CommandCenter {
await this.model.branch(name);
}

@command('git.deleteBranch')
async deleteBranch(branchName: string): Promise<void> {
if (typeof branchName === 'string') {
return await this.model.deleteBranch(branchName);
}
const currentHead = this.model.HEAD && this.model.HEAD.name;
const heads = this.model.refs.filter(ref => ref.type === RefType.Head && ref.name !== currentHead)
.map(ref => new BranchDeleteItem(ref));

const placeHolder = 'Select a branch to delete';
const choice = await window.showQuickPick<BranchDeleteItem>(heads, { placeHolder });

if (!choice) {
return;
}

await choice.run(this.model);
}

@command('git.pull')
async pull(): Promise<void> {
const remotes = this.model.remotes;
Expand Down
5 changes: 5 additions & 0 deletions extensions/git/src/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,11 @@ export class Repository {
await this.run(args);
}

async deleteBranch(name: string): Promise<void> {
const args = ['branch', '-d', name];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should be explicit with the branch name and use its full form refs/heads/${name} instead of simply name.

await this.run(args);
}

async clean(paths: string[]): Promise<void> {
const pathsByGroup = groupBy(paths, p => path.dirname(p));
const groups = Object.keys(pathsByGroup).map(k => pathsByGroup[k]);
Expand Down
7 changes: 6 additions & 1 deletion extensions/git/src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ export enum Operation {
Init = 1 << 12,
Show = 1 << 13,
Stage = 1 << 14,
GetCommitTemplate = 1 << 15
GetCommitTemplate = 1 << 15,
DeleteBranch = 1 << 16
}

// function getOperationName(operation: Operation): string {
Expand Down Expand Up @@ -454,6 +455,10 @@ export class Model implements Disposable {
await this.run(Operation.Branch, () => this.repository.branch(name, true));
}

async deleteBranch(name: string): Promise<void> {
await this.run(Operation.DeleteBranch, () => this.repository.deleteBranch(name));
}

async checkout(treeish: string): Promise<void> {
await this.run(Operation.Checkout, () => this.repository.checkout(treeish, []));
}
Expand Down