Merge remote-tracking branch 'origin/master' into attachment-mime-types

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

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)]

src/ui/draw.rs

@@ -288,8 +288,20 @@ fn draw_messages<B: Backend>(f: &mut Frame<B>, 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<B: Backend>(f: &mut Frame<B>, 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();

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<Uuid, (String, Color)>,
     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<Item = MessageId>,
+    ) -> Self {
+        let mut names_and_colors: HashMap<Uuid, (String, Color)> = 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<str>, 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());