don't close branchlist every time (#550)

* do not close branchlist after branch rename * do not close branchlist after deleting a branch * closes #543
2026-05-23 17:08:21 +00:00 · 2021-02-25 15:54:18 +01:00 · 2021-02-25 15:54:18 +01:00 · 8d4c1ca26e
commit 8d4c1ca26e
parent dc3775caa2
7 changed files with 57 additions and 57 deletions
--- a/CHANGELOG.md
+++ b/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))
--- a/asyncgit/src/sync/branch.rs
+++ b/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())
--- a/asyncgit/src/sync/mod.rs
+++ b/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::{
--- a/src/app.rs
+++ b/src/app.rs
@ -2,12 +2,13 @@ use crate::{
    accessors,
    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,
    },
    input::{Input, InputEvent, InputState},
    keys::{KeyConfig, SharedKeyConfig},
@ -47,7 +48,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,
@ -125,7 +126,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(),
@ -491,7 +492,7 @@ impl App {
                        )
                    } else {
                        flags.insert(NeedsUpdate::ALL);
-                        self.select_branch_popup.hide();
+                        self.select_branch_popup.update_branches()?;
                    }
                }
                Action::ForcePush(branch, force) => self
--- a/src/components/select_branch.rs
+++ b/src/components/select_branch.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,
--- a/src/components/mod.rs
+++ b/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};
--- a/src/components/utils/mod.rs
+++ b/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{}",