From b5f1e7645584b1fd2be26de791d858b9d0c4ec93 Mon Sep 17 00:00:00 2001 From: Stephan Dilly Date: Tue, 6 Apr 2021 18:16:08 +0200 Subject: [PATCH] Remote branches (#618) * allow checking out remote branch * set tracking branch on checking out remote * fix unittests by making branch list stable sorted by name --- CHANGELOG.md | 1 + assets/vim_style_key_config.ron | 1 + asyncgit/src/error.rs | 3 + asyncgit/src/sync/branch/mod.rs | 240 +++++++++++++++++++++++++++--- asyncgit/src/sync/commits_info.rs | 2 +- asyncgit/src/sync/remotes/mod.rs | 7 +- src/components/branchlist.rs | 126 ++++++++++------ src/keys.rs | 2 + src/strings.rs | 36 ++++- 9 files changed, 350 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aafbdd3..e37ecc07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `[w]` key to toggle between staging/workdir [[@terhechte](https://github.com/terhechte)] ([#595](https://github.com/extrawurst/gitui/issues/595)) +- view/checkout remote branches ([#617](https://github.com/extrawurst/gitui/issues/617)) ### Fixed - push branch to its tracking remote ([#597](https://github.com/extrawurst/gitui/issues/597)) diff --git a/assets/vim_style_key_config.ron b/assets/vim_style_key_config.ron index 6b447dac..ddb87d4f 100644 --- a/assets/vim_style_key_config.ron +++ b/assets/vim_style_key_config.ron @@ -70,6 +70,7 @@ rename_branch: ( code: Char('r'), modifiers: ( bits: 0,),), select_branch: ( code: Char('b'), modifiers: ( bits: 0,),), delete_branch: ( code: Char('D'), modifiers: ( bits: 1,),), + toggle_remote_branches: ( code: Char('t'), modifiers: ( bits: 0,),), push: ( code: Char('p'), modifiers: ( bits: 0,),), force_push: ( code: Char('P'), modifiers: ( bits: 1,),), pull: ( code: Char('f'), modifiers: ( bits: 0,),), diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index 0aebe1d8..0e70c8f8 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -18,6 +18,9 @@ pub enum Error { #[error("git: work dir error")] NoWorkDir, + #[error("git: uncommitted changes")] + UncommittedChanges, + #[error("io error:{0}")] Io(#[from] std::io::Error), diff --git a/asyncgit/src/sync/branch/mod.rs b/asyncgit/src/sync/branch/mod.rs index 7c55e5bd..8bbbc652 100644 --- a/asyncgit/src/sync/branch/mod.rs +++ b/asyncgit/src/sync/branch/mod.rs @@ -45,6 +45,27 @@ pub(crate) fn get_branch_name_repo( } /// +#[derive(Debug)] +pub struct LocalBranch { + /// + pub is_head: bool, + /// + pub has_upstream: bool, + /// + pub remote: Option, +} + +/// +#[derive(Debug)] +pub enum BranchDetails { + /// + Local(LocalBranch), + /// + Remote, +} + +/// +#[derive(Debug)] pub struct BranchInfo { /// pub name: String, @@ -55,20 +76,37 @@ pub struct BranchInfo { /// pub top_commit: CommitId, /// - pub is_head: bool, - /// - pub has_upstream: bool, - /// - pub remote: Option, + pub details: BranchDetails, } -/// returns a list of `BranchInfo` with a simple summary of info about a single branch -pub fn get_branches_info(repo_path: &str) -> Result> { +impl BranchInfo { + /// returns details about local branch or None + pub fn local_details(&self) -> Option<&LocalBranch> { + if let BranchDetails::Local(details) = &self.details { + return Some(details); + } + + None + } +} + +/// returns a list of `BranchInfo` with a simple summary on each branch +/// `local` filters for local branches otherwise remote branches will be returned +pub fn get_branches_info( + repo_path: &str, + local: bool, +) -> Result> { scope_time!("get_branches_info"); + let filter = if local { + BranchType::Local + } else { + BranchType::Remote + }; + let repo = utils::repo(repo_path)?; - let branches_for_display = repo - .branches(Some(BranchType::Local))? + let mut branches_for_display: Vec = repo + .branches(Some(filter))? .map(|b| { let branch = b?.0; let top_commit = branch.get().peel_to_commit()?; @@ -82,6 +120,16 @@ pub fn get_branches_info(repo_path: &str) -> Result> { .and_then(|buf| buf.as_str()) .map(String::from); + let details = if local { + BranchDetails::Local(LocalBranch { + is_head: branch.is_head(), + has_upstream: upstream.is_ok(), + remote, + }) + } else { + BranchDetails::Remote + }; + Ok(BranchInfo { name: bytes2string(branch.name_bytes()?)?, reference, @@ -89,14 +137,14 @@ pub fn get_branches_info(repo_path: &str) -> Result> { top_commit.summary_bytes().unwrap_or_default(), )?, top_commit: top_commit.id().into(), - is_head: branch.is_head(), - has_upstream: upstream.is_ok(), - remote, + details, }) }) .filter_map(Result::ok) .collect(); + branches_for_display.sort_by(|a, b| a.name.cmp(&b.name)); + Ok(branches_for_display) } @@ -212,12 +260,54 @@ pub fn checkout_branch( } Ok(()) } else { - Err(Error::Generic( - format!("Cannot change branch. There are unstaged/staged changes which have not been committed/stashed. There is {:?} changes preventing checking out a different branch.", statuses.len()), - )) + Err(Error::UncommittedChanges) } } +/// +pub fn checkout_remote_branch( + repo_path: &str, + branch: &BranchInfo, +) -> Result<()> { + scope_time!("checkout_remote_branch"); + + let repo = utils::repo(repo_path)?; + let cur_ref = repo.head()?; + + if !repo + .statuses(Some( + git2::StatusOptions::new().include_ignored(false), + ))? + .is_empty() + { + return Err(Error::UncommittedChanges); + } + + let name = if let Some(pos) = branch.name.rfind('/') { + branch.name[pos..].to_string() + } else { + branch.name.clone() + }; + + let commit = repo.find_commit(branch.top_commit.into())?; + let mut new_branch = repo.branch(&name, &commit, false)?; + new_branch.set_upstream(Some(&branch.name))?; + + repo.set_head( + bytes2string(new_branch.into_reference().name_bytes())? + .as_str(), + )?; + + if let Err(e) = repo.checkout_head(Some( + git2::build::CheckoutBuilder::new().force(), + )) { + // This is safe beacuse cur_ref was just found + repo.set_head(bytes2string(cur_ref.name_bytes())?.as_str())?; + return Err(Error::Git(e)); + } + Ok(()) +} + /// The user must not be on the branch for the branch to be deleted pub fn delete_branch( repo_path: &str, @@ -341,7 +431,7 @@ mod tests_branches { let repo_path = root.as_os_str().to_str().unwrap(); assert_eq!( - get_branches_info(repo_path) + get_branches_info(repo_path, true) .unwrap() .iter() .map(|b| b.name.clone()) @@ -359,7 +449,7 @@ mod tests_branches { create_branch(repo_path, "test").unwrap(); assert_eq!( - get_branches_info(repo_path) + get_branches_info(repo_path, true) .unwrap() .iter() .map(|b| b.name.clone()) @@ -405,7 +495,7 @@ mod tests_branches { ); //verify we got only master right now - let branches = get_branches_info(repo_path).unwrap(); + let branches = get_branches_info(repo_path, true).unwrap(); assert_eq!(branches.len(), 1); assert_eq!(branches[0].name, String::from("master")); @@ -423,10 +513,26 @@ mod tests_branches { "git checkout --track r2/r2branch", ); - let branches = get_branches_info(repo_path).unwrap(); + let branches = get_branches_info(repo_path, true).unwrap(); assert_eq!(branches.len(), 3); - assert_eq!(branches[1].remote.as_ref().unwrap(), "r1"); - assert_eq!(branches[2].remote.as_ref().unwrap(), "r2"); + assert_eq!( + branches[1] + .local_details() + .unwrap() + .remote + .as_ref() + .unwrap(), + "r1" + ); + assert_eq!( + branches[2] + .local_details() + .unwrap() + .remote + .as_ref() + .unwrap(), + "r2" + ); assert_eq!( get_branch_remote(repo_path, "r1branch") @@ -545,3 +651,95 @@ mod test_delete_branch { ); } } + +#[cfg(test)] +mod test_remote_branches { + use super::*; + use crate::sync::remotes::push::push; + use crate::sync::tests::{ + repo_clone, repo_init_bare, write_commit_file, + }; + + #[test] + fn test_remote_branches() { + let (r1_dir, _repo) = repo_init_bare().unwrap(); + + let (clone1_dir, clone1) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone1_dir = clone1_dir.path().to_str().unwrap(); + + // clone1 + + write_commit_file(&clone1, "test.txt", "test", "commit1"); + + push(clone1_dir, "origin", "master", false, None, None) + .unwrap(); + + create_branch(clone1_dir, "foo").unwrap(); + + write_commit_file(&clone1, "test.txt", "test2", "commit2"); + + push(clone1_dir, "origin", "foo", false, None, None).unwrap(); + + // clone2 + + let (clone2_dir, _clone2) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone2_dir = clone2_dir.path().to_str().unwrap(); + + let local_branches = + get_branches_info(clone2_dir, true).unwrap(); + + assert_eq!(local_branches.len(), 1); + + let branches = get_branches_info(clone2_dir, false).unwrap(); + assert_eq!(dbg!(&branches).len(), 3); + assert_eq!(&branches[0].name, "origin/HEAD"); + assert_eq!(&branches[1].name, "origin/foo"); + assert_eq!(&branches[2].name, "origin/master"); + } + + #[test] + fn test_checkout_remote_branch() { + let (r1_dir, _repo) = repo_init_bare().unwrap(); + + let (clone1_dir, clone1) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + let clone1_dir = clone1_dir.path().to_str().unwrap(); + + // clone1 + + write_commit_file(&clone1, "test.txt", "test", "commit1"); + push(clone1_dir, "origin", "master", false, None, None) + .unwrap(); + create_branch(clone1_dir, "foo").unwrap(); + write_commit_file(&clone1, "test.txt", "test2", "commit2"); + push(clone1_dir, "origin", "foo", false, None, None).unwrap(); + + // clone2 + + let (clone2_dir, _clone2) = + repo_clone(r1_dir.path().to_str().unwrap()).unwrap(); + + let clone2_dir = clone2_dir.path().to_str().unwrap(); + + let local_branches = + get_branches_info(clone2_dir, true).unwrap(); + + assert_eq!(local_branches.len(), 1); + + let branches = get_branches_info(clone2_dir, false).unwrap(); + + // checkout origin/foo + checkout_remote_branch(clone2_dir, &branches[1]).unwrap(); + + assert_eq!( + get_branches_info(clone2_dir, true).unwrap().len(), + 2 + ); + + assert_eq!(&get_branch_name(clone2_dir).unwrap(), "foo"); + } +} diff --git a/asyncgit/src/sync/commits_info.rs b/asyncgit/src/sync/commits_info.rs index d7bfcb8c..8891e938 100644 --- a/asyncgit/src/sync/commits_info.rs +++ b/asyncgit/src/sync/commits_info.rs @@ -21,7 +21,7 @@ impl CommitId { self.0 } - /// + /// 7 chars short hash pub fn get_short_string(&self) -> String { self.to_string().chars().take(7).collect() } diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index a2378b5e..36ad7366 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -3,11 +3,12 @@ pub(crate) mod push; pub(crate) mod tags; -use self::push::ProgressNotification; -use super::cred::BasicAuthCredential; use crate::{ error::{Error, Result}, - sync::utils, + sync::{ + cred::BasicAuthCredential, + remotes::push::ProgressNotification, utils, + }, }; use crossbeam_channel::Sender; use git2::{FetchOptions, Repository}; diff --git a/src/components/branchlist.rs b/src/components/branchlist.rs index ebe7acf3..6e4389ba 100644 --- a/src/components/branchlist.rs +++ b/src/components/branchlist.rs @@ -7,17 +7,18 @@ use crate::{ keys::SharedKeyConfig, queue::{Action, InternalEvent, NeedsUpdate, Queue}, strings, try_or_popup, - ui::{self, calc_scroll_top}, + ui::{self, calc_scroll_top, Size}, }; +use anyhow::Result; use asyncgit::{ - sync::{checkout_branch, get_branches_info, BranchInfo}, + sync::{ + branch::checkout_remote_branch, checkout_branch, + get_branches_info, BranchInfo, + }, CWD, }; use crossterm::event::Event; -use std::{ - cell::Cell, - convert::{TryFrom, TryInto}, -}; +use std::{cell::Cell, convert::TryInto}; use tui::{ backend::Backend, layout::{Alignment, Rect}, @@ -25,15 +26,13 @@ use tui::{ widgets::{Block, BorderType, Borders, Clear, Paragraph}, Frame, }; -use unicode_truncate::UnicodeTruncateStr; - -use crate::ui::Size; -use anyhow::Result; use ui::style::SharedTheme; +use unicode_truncate::UnicodeTruncateStr; /// pub struct BranchListComponent { - branch_names: Vec, + branches: Vec, + local: bool, visible: bool, selection: u16, scroll_top: Cell, @@ -80,7 +79,7 @@ impl DrawableComponent for BranchListComponent { )) .block( Block::default() - .title(strings::SELECT_BRANCH_POPUP_MSG) + .title(strings::title_branches(self.local)) .border_type(BorderType::Thick) .borders(Borders::ALL), ) @@ -92,11 +91,11 @@ impl DrawableComponent for BranchListComponent { f, area, &self.theme, - self.branch_names.len(), + self.branches.len(), self.scroll_top.get(), ); - self.current_height.set(u16::try_from(height_in_lines)?); + self.current_height.set(height_in_lines.try_into()?); } Ok(()) @@ -125,26 +124,43 @@ impl Component for BranchListComponent { )); out.push(CommandInfo::new( - strings::commands::open_branch_create_popup( - &self.key_config, - ), - true, - true, - )); - - out.push(CommandInfo::new( - strings::commands::delete_branch_popup( + strings::commands::select_branch_popup( &self.key_config, ), !self.selection_is_cur_branch(), true, )); + out.push(CommandInfo::new( + strings::commands::open_branch_create_popup( + &self.key_config, + ), + true, + self.local, + )); + + out.push(CommandInfo::new( + strings::commands::delete_branch_popup( + &self.key_config, + ), + !self.selection_is_cur_branch(), + self.local, + )); + out.push(CommandInfo::new( strings::commands::rename_branch_popup( &self.key_config, ), true, + self.local, + )); + + out.push(CommandInfo::new( + strings::commands::toggle_branch_popup( + &self.key_config, + self.local, + ), + true, true, )); } @@ -170,8 +186,6 @@ impl Component for BranchListComponent { "switch branch error:", self.switch_to_selected_branch() ); - - self.hide() } else if e == self.key_config.create_branch { self.queue .borrow_mut() @@ -179,7 +193,7 @@ impl Component for BranchListComponent { self.hide(); } else if e == self.key_config.rename_branch { let cur_branch = - &self.branch_names[self.selection as usize]; + &self.branches[self.selection as usize]; self.queue.borrow_mut().push_back( InternalEvent::RenameBranch( cur_branch.reference.clone(), @@ -194,13 +208,17 @@ impl Component for BranchListComponent { self.queue.borrow_mut().push_back( InternalEvent::ConfirmAction( Action::DeleteBranch( - self.branch_names + self.branches [self.selection as usize] .reference .clone(), ), ), ); + } else if e == self.key_config.toggle_remote_branches + { + self.local = !self.local; + self.update_branches()?; } } @@ -232,7 +250,8 @@ impl BranchListComponent { key_config: SharedKeyConfig, ) -> Self { Self { - branch_names: Vec::new(), + branches: Vec::new(), + local: true, visible: false, selection: 0, scroll_top: Cell::new(0), @@ -253,17 +272,22 @@ impl BranchListComponent { /// fetch list of branches pub fn update_branches(&mut self) -> Result<()> { - self.branch_names = get_branches_info(CWD)?; + self.branches = get_branches_info(CWD, self.local)?; self.set_selection(self.selection)?; Ok(()) } fn selection_is_cur_branch(&self) -> bool { - self.branch_names + self.branches .iter() .enumerate() .filter(|(index, b)| { - b.is_head && *index == self.selection as usize + b.local_details() + .map(|details| { + details.is_head + && *index == self.selection as usize + }) + .unwrap_or_default() }) .count() > 0 @@ -289,7 +313,7 @@ impl BranchListComponent { } fn set_selection(&mut self, selection: u16) -> Result<()> { - let num_branches: u16 = self.branch_names.len().try_into()?; + let num_branches: u16 = self.branches.len().try_into()?; let num_branches = num_branches.saturating_sub(1); let selection = if selection > num_branches { @@ -326,7 +350,7 @@ impl BranchListComponent { let mut txt = Vec::new(); for (i, displaybranch) in self - .branch_names + .branches .iter() .skip(self.scroll_top.get()) .take(height) @@ -354,9 +378,16 @@ impl BranchListComponent { let selected = self.selection as usize - self.scroll_top.get() == i; - let is_head_str = - if displaybranch.is_head { "*" } else { " " }; - let has_upstream_str = if displaybranch.has_upstream { + let is_head = displaybranch + .local_details() + .map(|details| details.is_head) + .unwrap_or_default(); + let is_head_str = if is_head { "*" } else { " " }; + let has_upstream_str = if displaybranch + .local_details() + .map(|details| details.has_upstream) + .unwrap_or_default() + { "\u{2191}" } else { " " @@ -383,7 +414,7 @@ impl BranchListComponent { branch_name, w = branch_name_length ), - theme.branch(selected, displaybranch.is_head), + theme.branch(selected, is_head), ); txt.push(Spans::from(vec![ @@ -398,11 +429,22 @@ impl BranchListComponent { } /// - fn switch_to_selected_branch(&self) -> Result<()> { - checkout_branch( - asyncgit::CWD, - &self.branch_names[self.selection as usize].reference, - )?; + fn switch_to_selected_branch(&mut self) -> Result<()> { + if self.local { + checkout_branch( + asyncgit::CWD, + &self.branches[self.selection as usize].reference, + )?; + self.hide() + } else { + checkout_remote_branch( + CWD, + &self.branches[self.selection as usize], + )?; + self.local = true; + self.update_branches()?; + } + self.queue .borrow_mut() .push_back(InternalEvent::Update(NeedsUpdate::ALL)); diff --git a/src/keys.rs b/src/keys.rs index 63073f8b..8b96b183 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -66,6 +66,7 @@ pub struct KeyConfig { pub rename_branch: KeyEvent, pub select_branch: KeyEvent, pub delete_branch: KeyEvent, + pub toggle_remote_branches: KeyEvent, pub push: KeyEvent, pub force_push: KeyEvent, pub pull: KeyEvent, @@ -121,6 +122,7 @@ impl Default for KeyConfig { rename_branch: KeyEvent { code: KeyCode::Char('r'), modifiers: KeyModifiers::NONE}, select_branch: KeyEvent { code: KeyCode::Char('b'), modifiers: KeyModifiers::NONE}, delete_branch: KeyEvent{code: KeyCode::Char('D'), modifiers: KeyModifiers::SHIFT}, + toggle_remote_branches: KeyEvent{code: KeyCode::Char('t'), modifiers: KeyModifiers::NONE}, push: KeyEvent { code: KeyCode::Char('p'), modifiers: KeyModifiers::empty()}, force_push: KeyEvent { code: KeyCode::Char('P'), modifiers: KeyModifiers::SHIFT}, pull: KeyEvent { code: KeyCode::Char('f'), modifiers: KeyModifiers::empty()}, diff --git a/src/strings.rs b/src/strings.rs index c84942d5..7b1a92ab 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -19,7 +19,14 @@ pub static PUSH_TAGS_STATES_FETCHING: &str = "fetching"; pub static PUSH_TAGS_STATES_PUSHING: &str = "pushing"; pub static PUSH_TAGS_STATES_DONE: &str = "done"; -pub static SELECT_BRANCH_POPUP_MSG: &str = "Switch Branch"; +pub fn title_branches(local: bool) -> String { + if local { + "Branches (local)" + } else { + "Branches (remote)" + } + .to_string() +} pub fn title_status(_key_config: &SharedKeyConfig) -> String { "Unstaged Changes".to_string() @@ -879,6 +886,33 @@ pub mod commands { CMD_GROUP_GENERAL, ) } + pub fn select_branch_popup( + key_config: &SharedKeyConfig, + ) -> CommandText { + CommandText::new( + format!( + "Checkout [{}]", + key_config.get_hint(key_config.enter), + ), + "checkout branch", + CMD_GROUP_GENERAL, + ) + } + pub fn toggle_branch_popup( + key_config: &SharedKeyConfig, + local: bool, + ) -> CommandText { + CommandText::new( + format!( + "{} Branches [{}]", + if local { "Remote" } else { "Local" }, + key_config + .get_hint(key_config.toggle_remote_branches), + ), + "toggle branch type (remote/local)", + CMD_GROUP_GENERAL, + ) + } pub fn open_branch_select_popup( key_config: &SharedKeyConfig, ) -> CommandText {