Skip to content

Feature #875: Delete referring branches #935

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Added
- support rebasing branches with conflicts ([#895](https://github.com/extrawurst/gitui/issues/895))
- add supporting deletion of referring branches [[@alessandroasm](https://github.com/alessandroasm)] ([#875](https://github.com/extrawurst/gitui/issues/875))
- add a key binding to stage / unstage items [[@alessandroasm](https://github.com/alessandroasm)] ([#909](https://github.com/extrawurst/gitui/issues/909))
- switch to status tab after merging or rebasing with conflicts ([#926](https://github.com/extrawurst/gitui/issues/926))

Expand Down
35 changes: 35 additions & 0 deletions asyncgit/src/sync/branch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,41 @@ pub fn get_branches_info(
Ok(branches_for_display)
}

/// returns the local branches that track the provided remote branch
pub fn get_branch_trackers(
repo_path: &str,
branch_name: &str,
) -> Result<HashSet<String>> {
let repo = utils::repo(repo_path)?;

let local_branches: HashSet<_> = repo
.branches(Some(BranchType::Local))?
.filter_map(|b| {
let branch = b.ok()?.0;

branch
.upstream()
.ok()
.filter(|upstream| {
let upstream_name =
bytes2string(upstream.get().name_bytes())
.ok();
upstream_name.map_or(false, |upstream_name| {
branch_name == upstream_name
})
})
.and_then(|_| {
branch
.name_bytes()
.ok()
.and_then(|bn| bytes2string(bn).ok())
})
})
.collect();

Ok(local_branches)
}

///
#[derive(Debug, Default)]
pub struct BranchCompare {
Expand Down
3 changes: 2 additions & 1 deletion asyncgit/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub use blame::{blame_file, BlameHunk, FileBlame};
pub use branch::{
branch_compare_upstream, checkout_branch, config_is_pull_rebase,
create_branch, delete_branch, get_branch_remote,
get_branches_info, merge_commit::merge_upstream_commit,
get_branch_trackers, get_branches_info,
merge_commit::merge_upstream_commit,
merge_ff::branch_merge_upstream_fastforward,
merge_rebase::merge_upstream_rebase, rename::rename_branch,
validate_branch_name, BranchCompare, BranchInfo,
Expand Down
98 changes: 74 additions & 24 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,56 @@ impl App {
sync::discard_lines(CWD, &path, &lines)?;
flags.insert(NeedsUpdate::ALL);
}
Action::DeleteTag(tag_name) => {
if let Err(error) = sync::delete_tag(CWD, &tag_name) {
self.queue.push(InternalEvent::ShowErrorMsg(
error.to_string(),
));
} else {
flags.insert(NeedsUpdate::ALL);
self.tags_popup.update_tags()?;
}
}
Action::ForcePush(branch, force) => {
self.queue
.push(InternalEvent::Push(branch, force, false));
}
Action::PullMerge { rebase, .. } => {
self.pull_popup.try_conflict_free_merge(rebase);
flags.insert(NeedsUpdate::ALL);
}
Action::AbortMerge => {
self.status_tab.abort_merge();
flags.insert(NeedsUpdate::ALL);
}
Action::AbortRebase => {
self.status_tab.abort_rebase();
flags.insert(NeedsUpdate::ALL);
}
Action::DeleteBranch(_, _)
| Action::DeleteUpstreamBranch(_)
| Action::DeleteTrackingBranches(_) => {
return self.process_branch_delete_confirmed_action(
action, flags,
);
}
};

Ok(())
}

fn process_branch_delete_confirmed_action(
&mut self,
action: Action,
flags: &mut NeedsUpdate,
) -> Result<()> {
match action {
Action::DeleteBranch(branch_ref, true) => {
let upstream =
sync::get_branch_remote(CWD, &branch_ref)
.ok()
.flatten();

if let Err(e) = sync::delete_branch(CWD, &branch_ref)
{
self.queue.push(InternalEvent::ShowErrorMsg(
Expand All @@ -777,6 +826,13 @@ impl App {
}
flags.insert(NeedsUpdate::ALL);
self.select_branch_popup.update_branches()?;

// If we have a remote, we must ask if user wants to delete it
if let Some(upstream) = upstream {
self.queue.push(InternalEvent::ConfirmAction(
Action::DeleteUpstreamBranch(upstream),
));
}
}
Action::DeleteBranch(branch_ref, false) => {
self.queue.push(
Expand All @@ -799,33 +855,27 @@ impl App {
flags.insert(NeedsUpdate::ALL);
self.select_branch_popup.update_branches()?;
}
Action::DeleteTag(tag_name) => {
if let Err(error) = sync::delete_tag(CWD, &tag_name) {
self.queue.push(InternalEvent::ShowErrorMsg(
error.to_string(),
));
} else {
flags.insert(NeedsUpdate::ALL);
self.tags_popup.update_tags()?;
Action::DeleteUpstreamBranch(upstream) => {
self.queue.push(InternalEvent::ConfirmedAction(
Action::DeleteBranch(upstream, false),
));
}
Action::DeleteTrackingBranches(branch_refs) => {
for branch_ref in &branch_refs {
if let Err(e) =
sync::delete_branch(CWD, branch_ref)
{
self.queue.push(InternalEvent::ShowErrorMsg(
e.to_string(),
));
}
}
}
Action::ForcePush(branch, force) => {
self.queue
.push(InternalEvent::Push(branch, force, false));
}
Action::PullMerge { rebase, .. } => {
self.pull_popup.try_conflict_free_merge(rebase);
flags.insert(NeedsUpdate::ALL);
}
Action::AbortMerge => {
self.status_tab.abort_merge();
flags.insert(NeedsUpdate::ALL);
}
Action::AbortRebase => {
self.status_tab.abort_rebase();

flags.insert(NeedsUpdate::ALL);
self.select_branch_popup.update_branches()?;
}
};
_ => {}
}

Ok(())
}
Expand Down
34 changes: 32 additions & 2 deletions src/components/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
CommandInfo, Component, DrawableComponent, EventState,
},
keys::SharedKeyConfig,
queue::{InternalEvent, Queue},
queue::{Action, InternalEvent, Queue},
strings,
ui::{self, style::SharedTheme},
};
Expand All @@ -15,13 +15,14 @@ use asyncgit::{
extract_username_password, need_username_password,
BasicAuthCredential,
},
get_branch_remote, get_default_remote,
get_branch_remote, get_branch_trackers, get_default_remote,
},
AsyncGitNotification, AsyncPush, PushRequest, RemoteProgress,
RemoteProgressState, CWD,
};
use crossbeam_channel::Sender;
use crossterm::event::Event;
use std::collections::HashSet;
use tui::{
backend::Backend,
layout::Rect,
Expand Down Expand Up @@ -60,6 +61,7 @@ pub struct PushComponent {
theme: SharedTheme,
key_config: SharedKeyConfig,
input_cred: CredComponent,
tracking_branches: Option<Vec<String>>,
}

impl PushComponent {
Expand All @@ -84,6 +86,7 @@ impl PushComponent {
),
theme,
key_config,
tracking_branches: None,
}
}

Expand Down Expand Up @@ -141,6 +144,19 @@ impl PushComponent {
remote
};

self.tracking_branches = if self.modifier.delete() {
let remote_ref =
format!("refs/remotes/{}/{}", remote, self.branch);
Some(
get_branch_trackers(CWD, &remote_ref)
.unwrap_or_else(|_| HashSet::new())
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need an empty hashset here? can't we go with None in case there was a problem?

.into_iter()
.collect(),
)
} else {
None
};

self.pending = true;
self.progress = None;
self.git_push.request(PushRequest {
Expand Down Expand Up @@ -175,6 +191,20 @@ impl PushComponent {
self.queue.push(InternalEvent::ShowErrorMsg(
format!("push failed:\n{}", err),
));
} else if self.modifier.delete() {
// Check if we need to delete the tracking branches
let tracking_branches = self
.tracking_branches
.take()
.unwrap_or_else(Vec::new);

if !tracking_branches.is_empty() {
self.queue.push(InternalEvent::ConfirmAction(
Action::DeleteTrackingBranches(
tracking_branches.into_iter().collect(),
),
));
}
}
self.hide();
}
Expand Down
18 changes: 18 additions & 0 deletions src/components/reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,24 @@ impl ConfirmComponent {
branch_ref,
),
),
Action::DeleteUpstreamBranch(upstream) => (
strings::confirm_title_delete_remote_branch(
&self.key_config,
),
strings::confirm_msg_delete_referring_remote_branch(
&self.key_config,
upstream,
),
),
Action::DeleteTrackingBranches(local_branches) => (
strings::confirm_title_delete_branch(
&self.key_config,
),
strings::confirm_msg_delete_tracking_branches(
&self.key_config,
local_branches,
),
),
Action::DeleteTag(tag_name) => (
strings::confirm_title_delete_tag(
&self.key_config,
Expand Down
2 changes: 2 additions & 0 deletions src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub enum Action {
StashDrop(Vec<CommitId>),
StashPop(CommitId),
DeleteBranch(String, bool),
DeleteUpstreamBranch(String),
DeleteTrackingBranches(Vec<String>),
DeleteTag(String),
ForcePush(String, bool),
PullMerge { incoming: usize, rebase: bool },
Expand Down
18 changes: 18 additions & 0 deletions src/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,15 @@ pub fn confirm_msg_delete_branch(
) -> String {
format!("Confirm deleting branch: '{}' ?", branch_ref)
}
pub fn confirm_msg_delete_tracking_branches(
_key_config: &SharedKeyConfig,
branches_ref: &[String],
) -> String {
format!(
"Do you want to delete the referring tracking branches: {} ?",
Copy link
Collaborator

Choose a reason for hiding this comment

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

since it is a list and we have a multiline message box, lets list the branch names one per line after the question

branches_ref.join(", ")
)
}
pub fn confirm_title_delete_remote_branch(
_key_config: &SharedKeyConfig,
) -> String {
Expand All @@ -218,6 +227,15 @@ pub fn confirm_msg_delete_remote_branch(
) -> String {
format!("Confirm deleting remote branch: '{}' ?", branch_ref)
}
pub fn confirm_msg_delete_referring_remote_branch(
_key_config: &SharedKeyConfig,
branch_ref: &str,
) -> String {
format!(
"Do you want to delete the referring remote branch: '{}' ?",
branch_ref
)
}
pub fn confirm_title_delete_tag(
_key_config: &SharedKeyConfig,
) -> String {
Expand Down