From 5bcba53d4dc9fc0cb63e5efb635a815e26eff05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Mon, 17 Jan 2022 11:57:52 +0300 Subject: [PATCH] Remove alias field in `Tab`, store visible name, remove `MsgSource::visible_name` - TUI does not care about tab aliases, it just needs to know how a tab will be shown in the tab bar. - Remove `MsgSource::visible_name`: - This method is used in one place, inlining it makes the code easier to understand - On its own this method is not useful, as "visible name" of a tab also depends on the server alias when the tab is for a server. - Minor refactoring in `TUI::new_tab` --- crates/libtiny_common/src/lib.rs | 8 ---- crates/libtiny_tui/src/tab.rs | 7 +--- crates/libtiny_tui/src/tests/layout.rs | 15 +++++++ crates/libtiny_tui/src/tui.rs | 55 ++++++++++++++++---------- 4 files changed, 51 insertions(+), 34 deletions(-) diff --git a/crates/libtiny_common/src/lib.rs b/crates/libtiny_common/src/lib.rs index 6655d147..3dc50bf8 100644 --- a/crates/libtiny_common/src/lib.rs +++ b/crates/libtiny_common/src/lib.rs @@ -196,14 +196,6 @@ impl MsgSource { MsgSource::User { serv, nick } => MsgTarget::User { serv, nick }, } } - - pub fn visible_name(&self) -> &str { - match self { - MsgSource::Serv { serv, .. } => serv, - MsgSource::Chan { chan, .. } => chan.display(), - MsgSource::User { nick, .. } => nick, - } - } } // NOTE: Keep the variants sorted in increasing significance, to avoid updating diff --git a/crates/libtiny_tui/src/tab.rs b/crates/libtiny_tui/src/tab.rs index 56f706bc..e60b9b30 100644 --- a/crates/libtiny_tui/src/tab.rs +++ b/crates/libtiny_tui/src/tab.rs @@ -10,7 +10,7 @@ use crate::{ }; pub(crate) struct Tab { - pub(crate) alias: Option, + pub(crate) visible_name: String, pub(crate) widget: MessagingUI, pub(crate) src: MsgSource, pub(crate) style: TabStyle, @@ -30,10 +30,7 @@ fn tab_style(style: TabStyle, colors: &Colors) -> Style { impl Tab { pub(crate) fn visible_name(&self) -> &str { - match self.alias { - Some(ref alias) => alias, - None => self.src.visible_name(), - } + &self.visible_name } pub(crate) fn set_style(&mut self, style: TabStyle) { diff --git a/crates/libtiny_tui/src/tests/layout.rs b/crates/libtiny_tui/src/tests/layout.rs index 7c92be0b..236bbb2b 100644 --- a/crates/libtiny_tui/src/tests/layout.rs +++ b/crates/libtiny_tui/src/tests/layout.rs @@ -68,3 +68,18 @@ fn test_alignment_long_string() { expect_screen(screen, &tui.get_front_buffer(), 40, 5, Location::caller()); } + +#[test] +fn test_mnemonic_generation() { + let mut tui = TUI::new_test(10, 10); + tui.new_chan_tab("s1", ChanNameRef::new("#ab")); + tui.new_chan_tab("s2", ChanNameRef::new("#ab")); + tui.new_chan_tab("s3", ChanNameRef::new("#ab")); + tui.new_chan_tab("s4", ChanNameRef::new("#ab")); + let tabs = tui.get_tabs(); + assert_eq!(tabs.len(), 9); // mentions, 4 servers, 4 channels + assert_eq!(tabs[2].switch, Some('a')); + assert_eq!(tabs[4].switch, Some('b')); + assert_eq!(tabs[6].switch, Some('a')); + assert_eq!(tabs[8].switch, Some('b')); +} diff --git a/crates/libtiny_tui/src/tui.rs b/crates/libtiny_tui/src/tui.rs index d178f629..80c08f22 100644 --- a/crates/libtiny_tui/src/tui.rs +++ b/crates/libtiny_tui/src/tui.rs @@ -142,6 +142,11 @@ impl TUI { &self.tabs[self.active_idx].src } + #[cfg(test)] + pub(crate) fn get_tabs(&self) -> &[Tab] { + &self.tabs + } + fn new_tb(config_path: Option, tb: Termbox) -> TUI { // This is now done in reload_config() below // tb.set_clear_attributes(colors.clear.fg as u8, colors.clear.bg as u8); @@ -347,44 +352,52 @@ impl TUI { notifier: Notifier, alias: Option, ) { - let mut switch_keys: HashMap = HashMap::with_capacity(self.tabs.len()); - for tab in &self.tabs { - if let Some(key) = tab.switch { - switch_keys.entry(key).and_modify(|e| *e += 1).or_insert(1); - } - } + let visible_name = alias.unwrap_or_else(|| match &src { + MsgSource::Serv { serv } => serv.to_owned(), + MsgSource::Chan { chan, .. } => chan.display().to_owned(), + MsgSource::User { nick, .. } => nick.to_owned(), + }); let switch = { - let mut ret = None; - let mut n = 0; - let visible_name = match alias.as_ref() { - None => src.visible_name(), - Some(alias) => alias, - }; + // Maps a switch key to number of times it's used + let mut switch_keys: HashMap = HashMap::with_capacity(self.tabs.len()); + + for tab in &self.tabs { + if let Some(key) = tab.switch { + *switch_keys.entry(key).or_default() += 1; + } + } + + // From the characters in tab name, find the one that is used the least + let mut new_tab_switch_char: Option<(char, u16)> = None; for ch in visible_name.chars() { if !ch.is_alphabetic() { continue; } - match switch_keys.get(&ch) { + match switch_keys.get(&ch).copied() { None => { - ret = Some(ch); + new_tab_switch_char = Some((ch, 0)); break; } - Some(n_) => { - if ret == None || n > *n_ { - ret = Some(ch); - n = *n_; + Some(n_uses) => match new_tab_switch_char { + None => { + new_tab_switch_char = Some((ch, n_uses)); } - } + Some((_, new_tab_switch_char_n_uses)) => { + if new_tab_switch_char_n_uses > n_uses { + new_tab_switch_char = Some((ch, n_uses)); + } + } + }, } } - ret + new_tab_switch_char.map(|(ch, _)| ch) }; self.tabs.insert( idx, Tab { - alias, + visible_name, widget: MessagingUI::new( self.width, self.height - 1,