Remove get_summarytext_by_raw

Use `Summary.truncated_text()` instead.

Co-Authored-By: Floris Bruynooghe <flub@devork.be>

deltachat-ffi/src/lib.rs

@@ -15,7 +15,7 @@ extern crate num_traits;
 extern crate serde_json;
 
 use std::collections::BTreeMap;
-use std::convert::TryInto;
+use std::convert::TryFrom;
 use std::fmt::Write;
 use std::ops::Deref;
 use std::ptr;
@@ -3047,7 +3047,7 @@ pub unsafe extern "C" fn dc_msg_get_summary(
     let ffi_msg = &mut *msg;
     let ctx = &*ffi_msg.context;
 
-    let summary = block_on(async move { ffi_msg.message.get_summary(ctx, maybe_chat).await })
+    let summary = block_on(ffi_msg.message.get_summary(ctx, maybe_chat))
         .log_err(ctx, "dc_msg_get_summary failed")
         .unwrap_or_default();
     Box::into_raw(Box::new(summary.into()))
@@ -3065,12 +3065,13 @@ pub unsafe extern "C" fn dc_msg_get_summarytext(
     let ffi_msg = &mut *msg;
     let ctx = &*ffi_msg.context;
 
-    block_on({
-        ffi_msg
-            .message
-            .get_summarytext(ctx, approx_characters.try_into().unwrap_or_default())
-    })
-    .strdup()
+    let summary = block_on(ffi_msg.message.get_summary(ctx, None))
+        .log_err(ctx, "dc_msg_get_summarytext failed")
+        .unwrap_or_default();
+    match usize::try_from(approx_characters) {
+        Ok(chars) => summary.truncated_text(chars).strdup(),
+        Err(_) => summary.text.strdup(),
+    }
 }
 
 #[no_mangle]

src/lot.rs

@@ -162,6 +162,7 @@ impl From<MessageState> for LotState {
 
 impl From<Summary> for Lot {
     fn from(summary: Summary) -> Self {
+        let text2 = Some(summary.truncated_text(160).to_string());
         let (text1, text1_meaning) = match summary.prefix {
             None => (None, Meaning::None),
             Some(SummaryPrefix::Draft(text)) => (Some(text), Meaning::Text1Draft),
@@ -171,7 +172,7 @@ impl From<Summary> for Lot {
         Self {
             text1_meaning,
             text1,
-            text2: Some(summary.text),
+            text2,
             timestamp: summary.timestamp,
             state: summary.state.into(),
             ..Default::default()

src/message.rs

@@ -30,7 +30,7 @@ use crate::mimeparser::{parse_message_id, FailureReport, SystemMessage};
 use crate::param::{Param, Params};
 use crate::pgp::split_armored_data;
 use crate::stock_str;
-use crate::summary::{get_summarytext_by_raw, Summary};
+use crate::summary::Summary;
 
 /// Message ID, including reserved IDs.
 ///
@@ -592,7 +592,7 @@ impl Message {
     }
 
     /// Returns message summary for display in the search results.
-    pub async fn get_summary(&mut self, context: &Context, chat: Option<&Chat>) -> Result<Summary> {
+    pub async fn get_summary(&self, context: &Context, chat: Option<&Chat>) -> Result<Summary> {
         let chat_loaded: Chat;
         let chat = if let Some(chat) = chat {
             chat
@@ -616,18 +616,6 @@ impl Message {
         Ok(Summary::new(context, self, chat, contact.as_ref()).await)
     }
 
-    pub async fn get_summarytext(&self, context: &Context, approx_characters: usize) -> String {
-        get_summarytext_by_raw(
-            self.viewtype,
-            self.text.as_ref(),
-            self.is_forwarded(),
-            &self.param,
-            approx_characters,
-            context,
-        )
-        .await
-    }
-
     // It's a little unfortunate that the UI has to first call dc_msg_get_override_sender_name() and then if it was NULL, call
     // dc_contact_get_display_name() but this was the best solution:
     // - We could load a Contact struct from the db here to call get_display_name() instead of returning None, but then we had a db
@@ -871,7 +859,11 @@ impl Message {
             Param::Quote,
             if text.is_empty() {
                 // Use summary, similar to "Image" to avoid sending empty quote.
-                quote.get_summarytext(context, 500).await
+                quote
+                    .get_summary(context, None)
+                    .await?
+                    .truncated_text(500)
+                    .to_string()
             } else {
                 text
             },

src/mimefactory.rs

@@ -1157,7 +1157,11 @@ impl<'a> MimeFactory<'a> {
         {
             stock_str::encrypted_msg(context).await
         } else {
-            self.msg.get_summarytext(context, 32).await
+            self.msg
+                .get_summary(context, None)
+                .await?
+                .truncated_text(32)
+                .to_string()
         };
         let p2 = stock_str::read_rcpt_mail_body(context, p1).await;
         let message_text = format!("{}\r\n", format_flowed(&p2));

src/summary.rs

@@ -7,15 +7,12 @@ use crate::context::Context;
 use crate::dc_tools::dc_truncate;
 use crate::message::{Message, MessageState};
 use crate::mimeparser::SystemMessage;
-use crate::param::{Param, Params};
+use crate::param::Param;
 use crate::stock_str;
 use itertools::Itertools;
+use std::borrow::Cow;
 use std::fmt;
 
-// In practice, the user additionally cuts the string themselves
-// pixel-accurate.
-const SUMMARY_CHARACTERS: usize = 160;
-
 /// Prefix displayed before message and separated by ":" in the chatlist.
 #[derive(Debug)]
 pub enum SummaryPrefix {
@@ -85,15 +82,7 @@ impl Summary {
             }
         };
 
-        let mut text = get_summarytext_by_raw(
-            msg.viewtype,
-            msg.text.as_ref(),
-            msg.is_forwarded(),
-            &msg.param,
-            SUMMARY_CHARACTERS,
-            context,
-        )
-        .await;
+        let mut text = msg.get_summary_text(context).await;
 
         if text.is_empty() && msg.quoted_text().is_some() {
             text = stock_str::reply_noun(context).await
@@ -106,88 +95,87 @@ impl Summary {
             state: msg.state,
         }
     }
+
+    /// Returns the [`Summary::text`] attribute truncated to an approximate length.
+    pub fn truncated_text(&self, approx_chars: usize) -> Cow<str> {
+        dc_truncate(&self.text, approx_chars)
+    }
 }
 
-/// Returns a summary text.
-pub async fn get_summarytext_by_raw(
-    viewtype: Viewtype,
-    text: Option<impl AsRef<str>>,
-    was_forwarded: bool,
-    param: &Params,
-    approx_characters: usize,
-    context: &Context,
-) -> String {
-    let mut append_text = true;
-    let prefix = match viewtype {
-        Viewtype::Image => stock_str::image(context).await,
-        Viewtype::Gif => stock_str::gif(context).await,
-        Viewtype::Sticker => stock_str::sticker(context).await,
-        Viewtype::Video => stock_str::video(context).await,
-        Viewtype::Voice => stock_str::voice_message(context).await,
-        Viewtype::Audio | Viewtype::File => {
-            if param.get_cmd() == SystemMessage::AutocryptSetupMessage {
-                append_text = false;
-                stock_str::ac_setup_msg_subject(context).await
-            } else {
-                let file_name: String = param
-                    .get_path(Param::File, context)
-                    .unwrap_or(None)
-                    .and_then(|path| {
-                        path.file_name()
-                            .map(|fname| fname.to_string_lossy().into_owned())
-                    })
-                    .unwrap_or_else(|| String::from("ErrFileName"));
-                let label = if viewtype == Viewtype::Audio {
-                    stock_str::audio(context).await
+impl Message {
+    /// Returns a summary text.
+    async fn get_summary_text(&self, context: &Context) -> String {
+        let mut append_text = true;
+        let prefix = match self.viewtype {
+            Viewtype::Image => stock_str::image(context).await,
+            Viewtype::Gif => stock_str::gif(context).await,
+            Viewtype::Sticker => stock_str::sticker(context).await,
+            Viewtype::Video => stock_str::video(context).await,
+            Viewtype::Voice => stock_str::voice_message(context).await,
+            Viewtype::Audio | Viewtype::File => {
+                if self.param.get_cmd() == SystemMessage::AutocryptSetupMessage {
+                    append_text = false;
+                    stock_str::ac_setup_msg_subject(context).await
                 } else {
-                    stock_str::file(context).await
-                };
-                format!("{} – {}", label, file_name)
+                    let file_name: String = self
+                        .param
+                        .get_path(Param::File, context)
+                        .unwrap_or(None)
+                        .and_then(|path| {
+                            path.file_name()
+                                .map(|fname| fname.to_string_lossy().into_owned())
+                        })
+                        .unwrap_or_else(|| String::from("ErrFileName"));
+                    let label = if self.viewtype == Viewtype::Audio {
+                        stock_str::audio(context).await
+                    } else {
+                        stock_str::file(context).await
+                    };
+                    format!("{} – {}", label, file_name)
+                }
             }
-        }
-        Viewtype::VideochatInvitation => {
-            append_text = false;
-            stock_str::videochat_invitation(context).await
-        }
-        _ => {
-            if param.get_cmd() != SystemMessage::LocationOnly {
-                "".to_string()
-            } else {
+            Viewtype::VideochatInvitation => {
                 append_text = false;
-                stock_str::location(context).await
+                stock_str::videochat_invitation(context).await
             }
+            _ => {
+                if self.param.get_cmd() != SystemMessage::LocationOnly {
+                    "".to_string()
+                } else {
+                    append_text = false;
+                    stock_str::location(context).await
+                }
+            }
+        };
+
+        if !append_text {
+            return prefix;
         }
-    };
 
-    if !append_text {
-        return prefix;
-    }
-
-    let summary_content = if let Some(text) = text {
-        if text.as_ref().is_empty() {
-            prefix
-        } else if prefix.is_empty() {
-            dc_truncate(text.as_ref(), approx_characters).to_string()
+        let summary_content = if let Some(text) = &self.text {
+            if text.is_empty() {
+                prefix
+            } else if prefix.is_empty() {
+                text.to_string()
+            } else {
+                format!("{} – {}", prefix, text)
+            }
         } else {
-            let tmp = format!("{} – {}", prefix, text.as_ref());
-            dc_truncate(&tmp, approx_characters).to_string()
-        }
-    } else {
-        prefix
-    };
+            prefix
+        };
 
-    let summary = if was_forwarded {
-        let tmp = format!(
-            "{}: {}",
-            stock_str::forwarded(context).await,
+        let summary = if self.is_forwarded() {
+            format!(
+                "{}: {}",
+                stock_str::forwarded(context).await,
+                summary_content
+            )
+        } else {
             summary_content
-        );
-        dc_truncate(&tmp, approx_characters).to_string()
-    } else {
-        summary_content
-    };
+        };
 
-    summary.split_whitespace().join(" ")
+        summary.split_whitespace().join(" ")
+    }
 }
 
 #[cfg(test)]
@@ -196,7 +184,7 @@ mod tests {
     use crate::test_utils as test;
 
     #[async_std::test]
-    async fn test_get_summarytext_by_raw() {
+    async fn test_get_summary_text() {
         let d = test::TestContext::new().await;
         let ctx = &d.ctx;
 
@@ -204,190 +192,125 @@ mod tests {
         let empty_text = Some("".to_string());
         let no_text: Option<String> = None;
 
-        let mut some_file = Params::new();
-        some_file.set(Param::File, "foo.bar");
-
+        let mut msg = Message::new(Viewtype::Text);
+        msg.set_text(some_text.clone());
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Text,
-                some_text.as_ref(),
-                false,
-                &Params::new(),
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "bla bla" // for simple text, the type is not added to the summary
         );
 
+        let mut msg = Message::new(Viewtype::Image);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Image,
-                no_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Image" // file names are not added for images
         );
 
+        let mut msg = Message::new(Viewtype::Video);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Video,
-                no_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Video" // file names are not added for videos
         );
 
+        let mut msg = Message::new(Viewtype::Gif);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(Viewtype::Gif, no_text.as_ref(), false, &some_file, 50, ctx,)
-                .await,
+            msg.get_summary_text(ctx).await,
             "GIF" // file names are not added for GIFs
         );
 
+        let mut msg = Message::new(Viewtype::Sticker);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Sticker,
-                no_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx,
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Sticker" // file names are not added for stickers
         );
 
+        let mut msg = Message::new(Viewtype::Voice);
+        msg.set_text(empty_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Voice,
-                empty_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx,
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Voice message" // file names are not added for voice messages, empty text is skipped
         );
 
+        let mut msg = Message::new(Viewtype::Voice);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Voice,
-                no_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Voice message" // file names are not added for voice messages
         );
 
+        let mut msg = Message::new(Viewtype::Voice);
+        msg.set_text(some_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Voice,
-                some_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Voice message \u{2013} bla bla" // `\u{2013}` explicitly checks for "EN DASH"
         );
 
+        let mut msg = Message::new(Viewtype::Audio);
+        msg.set_text(no_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Audio,
-                no_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Audio \u{2013} foo.bar" // file name is added for audio
         );
 
+        let mut msg = Message::new(Viewtype::Audio);
+        msg.set_text(empty_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Audio,
-                empty_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx,
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Audio \u{2013} foo.bar" // file name is added for audio, empty text is not added
         );
 
+        let mut msg = Message::new(Viewtype::Audio);
+        msg.set_text(some_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Audio,
-                some_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Audio \u{2013} foo.bar \u{2013} bla bla" // file name and text added for audio
         );
 
+        let mut msg = Message::new(Viewtype::File);
+        msg.set_text(some_text.clone());
+        msg.set_file("foo.bar", None);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::File,
-                some_text.as_ref(),
-                false,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "File \u{2013} foo.bar \u{2013} bla bla" // file name is added for files
         );
 
         // Forwarded
+        let mut msg = Message::new(Viewtype::Text);
+        msg.set_text(some_text.clone());
+        msg.param.set_int(Param::Forwarded, 1);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::Text,
-                some_text.as_ref(),
-                true,
-                &Params::new(),
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Forwarded: bla bla" // for simple text, the type is not added to the summary
         );
+
+        let mut msg = Message::new(Viewtype::File);
+        msg.set_text(some_text.clone());
+        msg.set_file("foo.bar", None);
+        msg.param.set_int(Param::Forwarded, 1);
         assert_eq!(
-            get_summarytext_by_raw(
-                Viewtype::File,
-                some_text.as_ref(),
-                true,
-                &some_file,
-                50,
-                ctx
-            )
-            .await,
+            msg.get_summary_text(ctx).await,
             "Forwarded: File \u{2013} foo.bar \u{2013} bla bla"
         );
 
-        let mut asm_file = Params::new();
-        asm_file.set(Param::File, "foo.bar");
-        asm_file.set_cmd(SystemMessage::AutocryptSetupMessage);
+        let mut msg = Message::new(Viewtype::File);
+        msg.set_text(no_text.clone());
+        msg.param.set(Param::File, "foo.bar");
+        msg.param.set_cmd(SystemMessage::AutocryptSetupMessage);
         assert_eq!(
-            get_summarytext_by_raw(Viewtype::File, no_text.as_ref(), false, &asm_file, 50, ctx)
-                .await,
+            msg.get_summary_text(ctx).await,
             "Autocrypt Setup Message" // file name is not added for autocrypt setup messages
         );
     }