don't close branchlist every time (#550)

* do not close branchlist after branch rename 
* do not close branchlist after deleting a branch
* closes #543

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ![charcount](assets/char_count.gif)
 
 ### Fixed
+- don't close branchlist every time ([#550](https://github.com/extrawurst/gitui/issues/550))
 - fixed key binding for *external exitor* in vim key bindings [[@yanganto](https://github.com/yanganto)] ([#549](https://github.com/extrawurst/gitui/issues/549))
 - fix some potential errors when deleting files while they are being diffed ([#490](https://github.com/extrawurst/gitui/issues/490))
 - push defaults to 'origin' remote if it exists ([#494](https://github.com/extrawurst/gitui/issues/494))

asyncgit/src/sync/branch.rs

@@ -33,7 +33,7 @@ pub(crate) fn get_branch_name(repo_path: &str) -> Result<String> {
 }
 
 ///
-pub struct BranchForDisplay {
+pub struct BranchInfo {
  ///
  pub name: String,
  ///
@@ -48,12 +48,9 @@ pub struct BranchForDisplay {
  pub has_upstream: bool,
 }
 
-/// Used to return only the nessessary information for displaying a branch
-/// rather than an iterator over the actual branches
-pub fn get_branches_to_display(
- repo_path: &str,
-) -> Result<Vec<BranchForDisplay>> {
- scope_time!("get_branches_to_display");
+/// returns a list of `BranchInfo` with a simple summary of info about a single branch
+pub fn get_branches_info(repo_path: &str) -> Result<Vec<BranchInfo>> {
+ scope_time!("get_branches_info");
 
  let cur_repo = utils::repo(repo_path)?;
  let branches_for_display = cur_repo
@@ -62,7 +59,7 @@ pub fn get_branches_to_display(
  let branch = b?.0;
  let top_commit = branch.get().peel_to_commit()?;
 
- Ok(BranchForDisplay {
+ Ok(BranchInfo {
  name: bytes2string(branch.name_bytes()?)?,
  reference: bytes2string(branch.get().name_bytes())?,
  top_commit_message: bytes2string(
@@ -299,7 +296,7 @@ mod tests_branches {
  let repo_path = root.as_os_str().to_str().unwrap();
 
  assert_eq!(
- get_branches_to_display(repo_path)
+ get_branches_info(repo_path)
  .unwrap()
  .iter()
  .map(|b| b.name.clone())
@@ -317,7 +314,7 @@ mod tests_branches {
  create_branch(repo_path, "test").unwrap();
 
  assert_eq!(
- get_branches_to_display(repo_path)
+ get_branches_info(repo_path)
  .unwrap()
  .iter()
  .map(|b| b.name.clone())

asyncgit/src/sync/mod.rs

@@ -23,8 +23,8 @@ pub mod utils;
 
 pub use branch::{
  branch_compare_upstream, checkout_branch, create_branch,
- delete_branch, get_branches_to_display, rename_branch,
- BranchCompare, BranchForDisplay,
+ delete_branch, get_branches_info, rename_branch, BranchCompare,
+ BranchInfo,
 };
 pub use commit::{amend, commit, tag};
 pub use commit_details::{

src/app.rs

@@ -2,12 +2,13 @@ use crate::{
  accessors, any_popup_visible,
  cmdbar::CommandBar,
  components::{
- event_pump, CommandBlocking, CommandInfo, CommitComponent,
- Component, CreateBranchComponent, DrawableComponent,
+ event_pump, BranchListComponent, CommandBlocking,
+ CommandInfo, CommitComponent, Component,
+ CreateBranchComponent, DrawableComponent,
  ExternalEditorComponent, HelpComponent,
  InspectCommitComponent, MsgComponent, PushComponent,
- RenameBranchComponent, ResetComponent, SelectBranchComponent,
- StashMsgComponent, TagCommitComponent,
+ RenameBranchComponent, ResetComponent, StashMsgComponent,
+ TagCommitComponent,
  },
  draw_popups,
  input::{Input, InputEvent, InputState},
@@ -49,7 +50,7 @@ pub struct App {
  tag_commit_popup: TagCommitComponent,
  create_branch_popup: CreateBranchComponent,
  rename_branch_popup: RenameBranchComponent,
- select_branch_popup: SelectBranchComponent,
+ select_branch_popup: BranchListComponent,
  cmdbar: RefCell<CommandBar>,
  tab: usize,
  revlog: Revlog,
@@ -127,7 +128,7 @@ impl App {
  theme.clone(),
  key_config.clone(),
  ),
- select_branch_popup: SelectBranchComponent::new(
+ select_branch_popup: BranchListComponent::new(
  queue.clone(),
  theme.clone(),
  key_config.clone(),
@@ -510,7 +511,7 @@ impl App {
  )
  } else {
  flags.insert(NeedsUpdate::ALL);
- self.select_branch_popup.hide();
+ self.select_branch_popup.update_branches()?;
  }
  }
  Action::ForcePush(branch, force) => self

src/components/select_branch.rs → src/components/branchlist.rs

@@ -6,13 +6,11 @@ use crate::{
  components::ScrollType,
  keys::SharedKeyConfig,
  queue::{Action, InternalEvent, NeedsUpdate, Queue},
- strings,
+ strings, try_or_popup,
  ui::{self, calc_scroll_top},
 };
 use asyncgit::{
- sync::{
- checkout_branch, get_branches_to_display, BranchForDisplay,
- },
+ sync::{checkout_branch, get_branches_info, BranchInfo},
  CWD,
 };
 use crossterm::event::Event;
@@ -33,8 +31,8 @@ use anyhow::Result;
 use ui::style::SharedTheme;
 
 ///
-pub struct SelectBranchComponent {
- branch_names: Vec<BranchForDisplay>,
+pub struct BranchListComponent {
+ branch_names: Vec<BranchInfo>,
  visible: bool,
  selection: u16,
  scroll_top: Cell<usize>,
@@ -44,7 +42,7 @@ pub struct SelectBranchComponent {
  key_config: SharedKeyConfig,
 }
 
-impl DrawableComponent for SelectBranchComponent {
+impl DrawableComponent for BranchListComponent {
  fn draw<B: Backend>(
  &self,
  f: &mut Frame<B>,
@@ -104,7 +102,7 @@ impl DrawableComponent for SelectBranchComponent {
  }
 }
 
-impl Component for SelectBranchComponent {
+impl Component for BranchListComponent {
  fn commands(
  &self,
  out: &mut Vec<CommandInfo>,
@@ -166,15 +164,12 @@ impl Component for SelectBranchComponent {
  } else if e == self.key_config.page_up {
  return self.move_selection(ScrollType::PageUp);
  } else if e == self.key_config.enter {
- if let Err(e) = self.switch_to_selected_branch() {
- log::error!("switch branch error: {}", e);
- self.queue.borrow_mut().push_back(
- InternalEvent::ShowErrorMsg(format!(
- "switch branch error:\n{}",
- e
- )),
- );
- }
+ try_or_popup!(
+ self,
+ "switch branch error:",
+ self.switch_to_selected_branch()
+ );
+
  self.hide()
  } else if e == self.key_config.create_branch {
  self.queue
@@ -190,7 +185,8 @@ impl Component for SelectBranchComponent {
  cur_branch.name.clone(),
  ),
  );
- self.hide();
+
+ self.update_branches()?;
  } else if e == self.key_config.delete_branch
  && !self.selection_is_cur_branch()
  {
@@ -228,7 +224,7 @@ impl Component for SelectBranchComponent {
  }
 }
 
-impl SelectBranchComponent {
+impl BranchListComponent {
  pub fn new(
  queue: Queue,
  theme: SharedTheme,
@@ -245,10 +241,6 @@ impl SelectBranchComponent {
  current_height: Cell::new(0),
  }
  }
- /// Get all the names of the branches in the repo
- pub fn get_branch_names() -> Result<Vec<BranchForDisplay>> {
- Ok(get_branches_to_display(CWD)?)
- }
 
  ///
  pub fn open(&mut self) -> Result<()> {
@@ -258,14 +250,14 @@ impl SelectBranchComponent {
  Ok(())
  }
 
- ////
+ /// fetch list of branches
  pub fn update_branches(&mut self) -> Result<()> {
- self.branch_names = Self::get_branch_names()?;
+ self.branch_names = get_branches_info(CWD)?;
+ self.set_selection(self.selection)?;
  Ok(())
  }
 
- ///
- pub fn selection_is_cur_branch(&self) -> bool {
+ fn selection_is_cur_branch(&self) -> bool {
  self.branch_names
  .iter()
  .enumerate()
@@ -278,10 +270,7 @@ impl SelectBranchComponent {
 
  ///
  fn move_selection(&mut self, scroll: ScrollType) -> Result<bool> {
- let num_branches: u16 = self.branch_names.len().try_into()?;
- let num_branches = num_branches.saturating_sub(1);
-
- let mut new_selection = match scroll {
+ let new_selection = match scroll {
  ScrollType::Up => self.selection.saturating_add(1),
  ScrollType::Down => self.selection.saturating_sub(1),
  ScrollType::PageDown => self
@@ -293,15 +282,26 @@ impl SelectBranchComponent {
  _ => self.selection,
  };
 
- if new_selection > num_branches {
- new_selection = num_branches;
- }
-
- self.selection = new_selection;
+ self.set_selection(new_selection)?;
 
  Ok(true)
  }
 
+ fn set_selection(&mut self, selection: u16) -> Result<()> {
+ let num_branches: u16 = self.branch_names.len().try_into()?;
+ let num_branches = num_branches.saturating_sub(1);
+
+ let selection = if selection > num_branches {
+ num_branches
+ } else {
+ selection
+ };
+
+ self.selection = selection;
+
+ Ok(())
+ }
+
  /// Get branches to display
  fn get_text(
  &self,

src/components/mod.rs

@@ -1,3 +1,4 @@
+mod branchlist;
 mod changes;
 mod command;
 mod commit;
@@ -14,12 +15,12 @@ mod msg;
 mod push;
 mod rename_branch;
 mod reset;
-mod select_branch;
 mod stashmsg;
 mod tag_commit;
 mod textinput;
 mod utils;
 
+pub use branchlist::BranchListComponent;
 pub use changes::ChangesComponent;
 pub use command::{CommandInfo, CommandText};
 pub use commit::CommitComponent;
@@ -35,7 +36,6 @@ pub use msg::MsgComponent;
 pub use push::PushComponent;
 pub use rename_branch::RenameBranchComponent;
 pub use reset::ResetComponent;
-pub use select_branch::SelectBranchComponent;
 pub use stashmsg::StashMsgComponent;
 pub use tag_commit::TagCommitComponent;
 pub use textinput::{InputType, TextInputComponent};

src/components/utils/mod.rs

@@ -10,6 +10,7 @@ pub mod statustree;
 macro_rules! try_or_popup {
  ($self:ident, $msg:literal, $e:expr) => {
  if let Err(err) = $e {
+ ::log::error!("{} {}", $msg, err);
  $self.queue.borrow_mut().push_back(
  InternalEvent::ShowErrorMsg(format!(
  "{}\n{}",