Skip to content

Commit

Permalink
Remove alias field in Tab, store visible name, remove `MsgSource::v…
Browse files Browse the repository at this point in the history
…isible_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`
  • Loading branch information
osa1 committed Jan 17, 2022
1 parent e8ab478 commit 5bcba53
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 34 deletions.
8 changes: 0 additions & 8 deletions crates/libtiny_common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions crates/libtiny_tui/src/tab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
};

pub(crate) struct Tab {
pub(crate) alias: Option<String>,
pub(crate) visible_name: String,
pub(crate) widget: MessagingUI,
pub(crate) src: MsgSource,
pub(crate) style: TabStyle,
Expand All @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions crates/libtiny_tui/src/tests/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
55 changes: 34 additions & 21 deletions crates/libtiny_tui/src/tui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>, 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);
Expand Down Expand Up @@ -347,44 +352,52 @@ impl TUI {
notifier: Notifier,
alias: Option<String>,
) {
let mut switch_keys: HashMap<char, i8> = 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<char, u16> = 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,
Expand Down

0 comments on commit 5bcba53

Please sign in to comment.