From 21c72c105fed379baa577aa5259d110f12ab54cc Mon Sep 17 00:00:00 2001 From: boxdot Date: Thu, 21 Dec 2023 10:19:42 +0100 Subject: [PATCH] feat: utilize name offset space better (#258) Also fix rendering of empty user names. Also update these when ensuring that the user is known. --- src/app.rs | 17 +++--- src/data.rs | 4 ++ src/ui/draw.rs | 25 ++++++--- src/ui/name_resolver.rs | 113 +++++++++++++++++----------------------- 4 files changed, 80 insertions(+), 79 deletions(-) diff --git a/src/app.rs b/src/app.rs index 501bd31..ebf0340 100644 --- a/src/app.rs +++ b/src/app.rs @@ -160,7 +160,7 @@ impl App { // 2. User id is in presage's signal manager (that is, it is a known contact from our address // book) => use it, // 3. User id is in the gurk's user name table (custom name) => use it, - // 4. give up with "Unknown User" + // 4. give up with UUID as user name pub fn name_by_id(&self, id: Uuid) -> String { if self.user_id == id { // it's me @@ -174,12 +174,12 @@ impl App { { // user is known via our contact list contact.name - } else if let Some(name) = self.storage.name(id) { + } else if let Some(name) = self.storage.name(id).filter(|name| !name.is_empty()) { // user should be at least known via their profile or phone number name.into_owned() } else { // give up - "Unknown User".to_string() + id.to_string() } } @@ -1079,13 +1079,18 @@ impl App { async fn ensure_user_is_known(&mut self, uuid: Uuid, profile_key: ProfileKeyBytes) { // is_known <=> - // * in names, or - // * is not a phone numbers, or + // * in names, and + // * is not empty + // * is not a phone numbers, and // * is not their uuid let is_known = self .storage .name(uuid) - .filter(|name| !util::is_phone_number(name) && Uuid::parse_str(name) != Ok(uuid)) + .filter(|name| { + !name.is_empty() + && !util::is_phone_number(name) + && Uuid::parse_str(name) != Ok(uuid) + }) .is_some(); if !is_known { if let Some(name) = self diff --git a/src/data.rs b/src/data.rs index 5762444..d5ba3a0 100644 --- a/src/data.rs +++ b/src/data.rs @@ -120,6 +120,10 @@ impl ChannelId { let group_id = secret_params.get_group_identifier(); Ok(Self::Group(group_id)) } + + pub(crate) fn is_user(&self) -> bool { + matches!(self, ChannelId::User(_)) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] diff --git a/src/ui/draw.rs b/src/ui/draw.rs index 6ee9103..3495e34 100644 --- a/src/ui/draw.rs +++ b/src/ui/draw.rs @@ -288,8 +288,20 @@ fn draw_messages(f: &mut Frame, app: &mut App, area: Rect) { } else { messages.rendered.offset }; - - let names = NameResolver::compute_for_channel(app, &channel); + let messages_to_render = messages + .items + .iter() + .rev() + .skip(offset) + .take(height) + .copied(); + + let names = NameResolver::compute( + app, + messages_to_render + .clone() + .map(|arrived_at| MessageId::new(channel_id, arrived_at)), + ); let max_username_width = names.max_name_width(); // message display options @@ -302,21 +314,18 @@ fn draw_messages(f: &mut Frame, app: &mut App, area: Rect) { let prefix = " ".repeat(prefix_width); // The day of the message at the bottom of the viewport - let messages_to_render = messages.items.iter().rev().skip(offset).copied(); let mut previous_msg_day = utc_timestamp_msec_to_local(messages_to_render.clone().next().unwrap_or_default()) .num_days_from_ce(); let messages_from_offset = messages_to_render .flat_map(|arrived_at| { - let msg = app - .storage - .message(MessageId::new(channel_id, arrived_at)) - .expect("non-existent message"); + let Some(msg) = app.storage.message(MessageId::new(channel_id, arrived_at)) else { + return [None, None]; + }; let date_division = display_date_line(msg.arrived_at, &mut previous_msg_day, width); let show_receipt = ShowReceipt::from_msg(&msg, app.user_id, app.config.show_receipts); let msg = display_message(&names, &msg, &prefix, width, height, show_receipt); - [date_division, msg] }) .flatten(); diff --git a/src/ui/name_resolver.rs b/src/ui/name_resolver.rs index d45e88d..6bea351 100644 --- a/src/ui/name_resolver.rs +++ b/src/ui/name_resolver.rs @@ -1,77 +1,57 @@ use std::borrow::Cow; +use std::collections::HashMap; use ratatui::style::Color; use unicode_width::UnicodeWidthStr; use uuid::Uuid; use crate::app::App; -use crate::data::{Channel, ChannelId}; +use crate::storage::MessageId; /// Once constructed for a channel, resolves uuid to name and color /// /// Construction takes time, lookup (resolving) is fast -// TODO: Cache in the app pub struct NameResolver<'a> { app: Option<&'a App>, - // invariant: sorted by Uuid - names_and_colors: Vec<(Uuid, String, Color)>, + names_and_colors: HashMap, max_name_width: usize, } impl<'a> NameResolver<'a> { - /// Constructs the resolver for channel - pub fn compute_for_channel<'b>(app: &'a App, channel: &'b Channel) -> Self { - let first_name_only = app.config.first_name_only; - let mut names_and_colors: Vec<(Uuid, String, Color)> = - if let Some(group_data) = channel.group_data.as_ref() { - // group channel - group_data - .members - .iter() - .map(|&uuid| { - let name = app.name_by_id(uuid); - let color = user_color(&name); - let name = displayed_name(name, first_name_only); - (uuid, name, color) - }) - .collect() - } else { - // direct message channel - let user_id = app.user_id; - let user_name = app.name_by_id(user_id); - let mut self_color = user_color(&user_name); - let user_name = displayed_name(user_name, first_name_only); - - let contact_uuid = match channel.id { - ChannelId::User(uuid) => uuid, - _ => unreachable!("logic error"), - }; - - if contact_uuid == user_id { - vec![(user_id, user_name, self_color)] - } else { - let contact_name = app.name_by_id(contact_uuid); - let contact_color = user_color(&contact_name); - let contact_name = displayed_name(contact_name, first_name_only); - - if self_color == contact_color { + pub fn compute( + app: &'a App, + relevant_message_ids: impl IntoIterator, + ) -> Self { + let mut names_and_colors: HashMap = Default::default(); + names_and_colors.insert(app.user_id, app.name_and_color(app.user_id)); + for message_id in relevant_message_ids { + if let Some(message) = app.storage.message(message_id) { + names_and_colors + .entry(message.from_id) + .or_insert_with(|| app.name_and_color(message.from_id)); + if message_id.channel_id.is_user() { + if message_id.channel_id == app.user_id { + break; // amortize notes channel + } else if message.from_id != app.user_id { // use different color for our user name - if let Some(idx) = USER_COLORS.iter().position(|&c| c == self_color) { - self_color = USER_COLORS[(idx + 1) % USER_COLORS.len()]; + let &(_, contact_color) = + names_and_colors.get(&message.from_id).expect("logic error"); + let (_, self_color) = + names_and_colors.get_mut(&app.user_id).expect("logic error"); + if self_color == &contact_color { + if let Some(idx) = USER_COLORS.iter().position(|&c| c == *self_color) { + *self_color = USER_COLORS[(idx + 1) % USER_COLORS.len()]; + } } + break; // amortize direct channel } - - vec![ - (user_id, user_name, self_color), - (contact_uuid, contact_name, contact_color), - ] } - }; - names_and_colors.sort_unstable_by_key(|&(id, _, _)| id); + } + } let max_name_width = names_and_colors - .iter() - .map(|(_, name, _)| name.width()) + .values() + .map(|(name, _)| name.width()) .max() .unwrap_or(0); @@ -84,19 +64,13 @@ impl<'a> NameResolver<'a> { /// Returns name and color for the given id pub fn resolve(&self, id: Uuid) -> (Cow, Color) { - match self - .names_and_colors - .binary_search_by_key(&id, |&(id, _, _)| id) - { - Ok(idx) => { - let (_, from, from_color) = &self.names_and_colors[idx]; - (from.into(), *from_color) - } - Err(_) => ( - self.app.expect("logic error").name_by_id(id).into(), - Color::Magenta, - ), - } + self.names_and_colors + .get(&id) + .map(|(name, color)| (name.into(), *color)) + .unwrap_or_else(|| { + let name = self.app.expect("logic error").name_by_id(id).into(); + (name, Color::Magenta) + }) } /// Returns the char width of the longest name @@ -109,12 +83,21 @@ impl<'a> NameResolver<'a> { pub fn single_user(user_id: Uuid, username: String, color: Color) -> NameResolver<'static> { NameResolver { app: None, - names_and_colors: vec![(user_id, username, color)], + names_and_colors: [(user_id, (username, color))].into_iter().collect(), max_name_width: 6, } } } +impl App { + fn name_and_color(&self, id: Uuid) -> (String, Color) { + let name = self.name_by_id(id); + let color = user_color(&name); + let name = displayed_name(name, self.config.first_name_only); + (name, color) + } +} + fn displayed_name(name: String, first_name_only: bool) -> String { if first_name_only { let space_pos = name.find(' ').unwrap_or(name.len());