compose: fix reply subject prefixes stripping original prefix

Unintelligent heuristic but should cover most cases?

Configurable subject response prefix #142
https://git.meli.delivery/meli/meli/issues/142

Closes #142

3 files changed, 154 insertions, 17 deletions

diff --git a/melib/src/thread.rs b/melib/src/thread.rs
index 925e8453..c5662631 100644
--- a/melib/src/thread.rs
+++ b/melib/src/thread.rs
@@ -179,7 +179,9 @@ macro_rules! make {
 /// use melib::thread::SubjectPrefix;
 ///
 /// let mut subject = "Re: RE: Res: Re: Res: Subject";
-/// assert_eq!(subject.strip_prefixes_from_list(<&str>::USUAL_PREFIXES), &"Subject");
+/// assert_eq!(subject.strip_prefixes_from_list(<&str>::USUAL_PREFIXES, None), &"Subject");
+/// let mut subject = "Re: RE: Res: Re: Res: Subject";
+/// assert_eq!(subject.strip_prefixes_from_list(<&str>::USUAL_PREFIXES, Some(1)), &"RE: Res: Re: Res: Subject");
 /// ```
 pub trait SubjectPrefix {
     const USUAL_PREFIXES: &'static [&'static str] = &[
@@ -279,7 +281,7 @@ pub trait SubjectPrefix {
     ];
     fn is_a_reply(&self) -> bool;
     fn strip_prefixes(&mut self) -> &mut Self;
-    fn strip_prefixes_from_list(&mut self, list: &[&str]) -> &mut Self;
+    fn strip_prefixes_from_list(&mut self, list: &[&str], times: Option<u8>) -> &mut Self;
 }
 
 impl SubjectPrefix for &[u8] {
@@ -335,10 +337,10 @@ impl SubjectPrefix for &[u8] {
         self
     }
 
-    fn strip_prefixes_from_list(&mut self, list: &[&str]) -> &mut Self {
+    fn strip_prefixes_from_list(&mut self, list: &[&str], mut times: Option<u8>) -> &mut Self {
         let result = {
             let mut slice = self.trim();
-            loop {
+            'outer: loop {
                 let len = slice.len();
                 for prefix in list.iter() {
                     if slice
@@ -347,10 +349,14 @@ impl SubjectPrefix for &[u8] {
                         .unwrap_or(false)
                     {
                         slice = &slice[prefix.len()..];
+                        slice = slice.trim();
+                        times = times.map(|u| u.saturating_sub(1));
+                        if times == Some(0) {
+                            break 'outer;
+                        }
                     }
-                    slice = slice.trim();
                 }
-                if slice.len() == len {
+                if slice.len() == len || times == Some(0) {
                     break;
                 }
             }
@@ -408,10 +414,10 @@ impl SubjectPrefix for &str {
         self
     }
 
-    fn strip_prefixes_from_list(&mut self, list: &[&str]) -> &mut Self {
+    fn strip_prefixes_from_list(&mut self, list: &[&str], mut times: Option<u8>) -> &mut Self {
         let result = {
             let mut slice = self.trim();
-            loop {
+            'outer: loop {
                 let len = slice.len();
                 for prefix in list.iter() {
                     if slice
@@ -420,10 +426,14 @@ impl SubjectPrefix for &str {
                         .unwrap_or(false)
                     {
                         slice = &slice[prefix.len()..];
+                        slice = slice.trim();
+                        times = times.map(|u| u.saturating_sub(1));
+                        if times == Some(0) {
+                            break 'outer;
+                        }
                     }
-                    slice = slice.trim();
                 }
-                if slice.len() == len {
+                if slice.len() == len || times == Some(0) {
                     break;
                 }
             }
diff --git a/src/components/mail/compose.rs b/src/components/mail/compose.rs
index 47a6766d..388b14ee 100644
--- a/src/components/mail/compose.rs
+++ b/src/components/mail/compose.rs
@@ -228,21 +228,21 @@ impl Composer {
             .as_ref()
             .map(|v| v.iter().map(String::as_str).collect::<Vec<&str>>())
             .unwrap_or_default();
-            let subject = subject
-                .as_ref()
-                .strip_prefixes_from_list(if prefix_list.is_empty() {
+            let subject_stripped = subject.as_ref().strip_prefixes_from_list(
+                if prefix_list.is_empty() {
                     <&str>::USUAL_PREFIXES
                 } else {
                     &prefix_list
-                })
-                .to_string();
+                },
+                Some(1),
+            ) == &subject.as_ref();
 
             let prefix =
                 account_settings!(context[ret.account_hash].composing.reply_prefix).as_str();
-            if !subject.starts_with(prefix) {
+            if subject_stripped {
                 format!("{prefix} {subject}", prefix = prefix, subject = subject)
             } else {
-                subject
+                subject.to_string()
             }
         };
         ret.draft.set_header("Subject", subject);
@@ -2390,3 +2390,61 @@ fn attribution_string(
     );
     melib::datetime::timestamp_to_string(date, Some(fmt.as_str()), posix)
 }
+
+#[test]
+fn test_compose_reply_subject_prefix() {
+    let raw_mail = r#"From: "some name" <some@example.com>
+To: "me" <myself@example.com>
+Cc:
+Subject: RE: your e-mail
+Message-ID: <h2g7f.z0gy2pgaen5m@example.com>
+Content-Type: text/plain
+
+hello world.
+"#;
+
+    let envelope = Envelope::from_bytes(raw_mail.as_bytes(), None).expect("Could not parse mail");
+    let mut context = Context::new_mock();
+    let account_hash = context.accounts[0].hash();
+    let mailbox_hash = 0;
+    let envelope_hash = envelope.hash();
+    context.accounts[0]
+        .collection
+        .insert(envelope, mailbox_hash);
+    let composer = Composer::reply_to(
+        (account_hash, mailbox_hash, envelope_hash),
+        String::new(),
+        &mut context,
+        false,
+    );
+    assert_eq!(&composer.draft.headers()["Subject"], "RE: your e-mail");
+    assert_eq!(
+        &composer.draft.headers()["To"],
+        r#"some name <some@example.com>"#
+    );
+    let raw_mail = r#"From: "some name" <some@example.com>
+To: "me" <myself@example.com>
+Cc:
+Subject: your e-mail
+Message-ID: <h2g7f.z0gy2pgaen5m@example.com>
+Content-Type: text/plain
+
+hello world.
+"#;
+    let envelope = Envelope::from_bytes(raw_mail.as_bytes(), None).expect("Could not parse mail");
+    let envelope_hash = envelope.hash();
+    context.accounts[0]
+        .collection
+        .insert(envelope, mailbox_hash);
+    let composer = Composer::reply_to(
+        (account_hash, mailbox_hash, envelope_hash),
+        String::new(),
+        &mut context,
+        false,
+    );
+    assert_eq!(&composer.draft.headers()["Subject"], "Re: your e-mail");
+    assert_eq!(
+        &composer.draft.headers()["To"],
+        r#"some name <some@example.com>"#
+    );
+}
diff --git a/src/state.rs b/src/state.rs
index 73f8c220..e72a4fba 100644
--- a/src/state.rs
+++ b/src/state.rs
@@ -163,6 +163,75 @@ impl Context {
         let idx = self.accounts.get_index_of(&account_hash).unwrap();
         self.is_online_idx(idx)
     }
+
+    #[cfg(test)]
+    pub fn new_mock() -> Self {
+        let (sender, receiver) =
+            crossbeam::channel::bounded(32 * ::std::mem::size_of::<ThreadEvent>());
+        let job_executor = Arc::new(JobExecutor::new(sender.clone()));
+        let input_thread = unbounded();
+        let input_thread_pipe = nix::unistd::pipe()
+            .map_err(|err| Box::new(err) as Box<dyn std::error::Error + Send + Sync + 'static>)
+            .unwrap();
+        let backends = Backends::new();
+        let settings = Box::new(Settings::new().unwrap());
+        let accounts = vec![{
+            let name = "test".to_string();
+            let mut account_conf = AccountConf::default();
+            account_conf.conf.format = "maildir".to_string();
+            account_conf.account.format = "maildir".to_string();
+            account_conf.account.root_mailbox = "/tmp/".to_string();
+            let sender = sender.clone();
+            let account_hash = {
+                use std::collections::hash_map::DefaultHasher;
+                use std::hash::Hasher;
+                let mut hasher = DefaultHasher::new();
+                hasher.write(name.as_bytes());
+                hasher.finish()
+            };
+            Account::new(
+                account_hash,
+                name,
+                account_conf,
+                &backends,
+                job_executor.clone(),
+                sender.clone(),
+                BackendEventConsumer::new(Arc::new(
+                    move |account_hash: AccountHash, ev: BackendEvent| {
+                        sender
+                            .send(ThreadEvent::UIEvent(UIEvent::BackendEvent(
+                                account_hash,
+                                ev,
+                            )))
+                            .unwrap();
+                    },
+                )),
+            )
+            .unwrap()
+        }];
+        let accounts = accounts.into_iter().map(|acc| (acc.hash(), acc)).collect();
+        let working = Arc::new(());
+        let control = Arc::downgrade(&working);
+        Context {
+            accounts,
+            settings,
+            dirty_areas: VecDeque::with_capacity(0),
+            replies: VecDeque::with_capacity(0),
+            temp_files: Vec::new(),
+            job_executor,
+            children: vec![],
+
+            input_thread: InputHandler {
+                pipe: input_thread_pipe,
+                rx: input_thread.1,
+                tx: input_thread.0,
+                control,
+                state_tx: sender.clone(),
+            },
+            sender,
+            receiver,
+        }
+    }
 }
 
 /// A State object to manage and own components and components of the UI. `State` is responsible for