[geary/wip/713150-conversations] More notes, fine-tuning

[Jim Nelson <jnelson src gnome org>]

commit b2f32e7ef6ab2d44adcfe1718f2caedf9f4e7250
Author: Jim Nelson <jim yorba org>
Date:   Tue Feb 24 16:15:42 2015 -0800

    More notes, fine-tuning

 src/engine/api/geary-associated-emails.vala        |    2 +
 src/engine/app/app-conversation-monitor.vala       |   79 +++++++++----------
 src/engine/app/app-conversation.vala               |   23 ++----
 .../conversation-monitor/app-conversation-set.vala |    2 +
 4 files changed, 48 insertions(+), 58 deletions(-)
---
diff --git a/src/engine/api/geary-associated-emails.vala b/src/engine/api/geary-associated-emails.vala
index c51f74a..5581f2f 100644
--- a/src/engine/api/geary-associated-emails.vala
+++ b/src/engine/api/geary-associated-emails.vala
@@ -12,6 +12,8 @@
  * also not updated as new email arrives and email is removed.
  *
  * @see Account.local_search_associated_emails_async
+ *
+ * TODO: Necessary?
  */
 
 public class Geary.AssociatedEmails : BaseObject {
diff --git a/src/engine/app/app-conversation-monitor.vala b/src/engine/app/app-conversation-monitor.vala
index 1fb0bca..a61d6df 100644
--- a/src/engine/app/app-conversation-monitor.vala
+++ b/src/engine/app/app-conversation-monitor.vala
@@ -337,11 +337,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
     
     internal async void local_load_async() {
         debug("ConversationMonitor seeding with local email for %s", folder.to_string());
-        try {
-            yield load_by_id_async(null, min_window_count, Folder.ListFlags.LOCAL_ONLY, cancellable_monitor);
-        } catch (Error e) {
-            debug("Error loading local messages: %s", e.message);
-        }
+        yield load_by_id_async(null, min_window_count, Folder.ListFlags.LOCAL_ONLY, cancellable_monitor);
         debug("ConversationMonitor seeded for %s", folder.to_string());
     }
     
@@ -386,15 +382,13 @@ public class Geary.App.ConversationMonitor : BaseObject {
     }
     
     private async void load_by_id_async(Geary.EmailIdentifier? initial_id, int count,
-        Geary.Folder.ListFlags flags, Cancellable? cancellable) throws Error {
+        Geary.Folder.ListFlags flags, Cancellable? cancellable) {
         notify_scan_started();
         try {
             yield process_email_async(yield folder.list_email_by_id_async(initial_id, count,
                 required_fields, flags, cancellable));
         } catch (Error err) {
             notify_scan_error(err);
-            
-            throw err;
         } finally {
             notify_scan_completed();
         }
@@ -478,20 +472,25 @@ public class Geary.App.ConversationMonitor : BaseObject {
     // NOTE: This is called from a background thread.
     private bool search_associated_predicate(EmailIdentifier email_id, bool only_partial,
         Gee.Collection<FolderPath?> known_paths, EmailFlags flags) {
+        // don't want partial emails
         if (only_partial)
             return false;
         
+        // if email is in this path, it's not blacklisted (i.e. if viewing the Spam folder, don't
+        // blacklist because it's in the Spam folder)
         if (known_paths.contains(folder.path))
             return true;
         
+        // Don't add drafts (unless in Drafts folder, above)
+        if (flags.contains(EmailFlags.DRAFT))
+            return false;
+        
+        // If in a blacklisted path, don't add
         foreach (FolderPath? blacklist_path in search_path_blacklist) {
             if (known_paths.contains(blacklist_path))
                 return false;
         }
         
-        if (flags.contains(EmailFlags.DRAFT))
-            return false;
-        
         return true;
     }
     
@@ -633,22 +632,29 @@ public class Geary.App.ConversationMonitor : BaseObject {
             if (removed_email == null)
                 continue;
             
-            Gee.Set<RFC822.MessageID>? removed_message_ids = conversation.remove(removed_email);
-            if (removed_message_ids != null) {
-                foreach (RFC822.MessageID removed_message_id in removed_message_ids)
-                    message_id_to_conversation.unset(removed_message_id);
-            }
+            // TODO: If removed message is still available in other paths, don't remove from
+            // conversation, simply remove from the path
+            bool removed = conversation.remove(removed_email, folder.path);
             
-            if (conversation.get_count() == 0) {
+            if (conversation.get_count() == 0 || !conversation.any_in_folder_path(folder.path)) {
                 conversations.remove(conversation);
                 removed_conversations.add(conversation);
-            } else {
+            } else if (removed) {
                 trimmed_conversations.set(conversation, removed_email);
             }
+            
+            // TODO: cleanup/remove message_id_to_conversation map
+            /*
+            if (removed_message_ids != null) {
+                foreach (RFC822.MessageID removed_message_id in removed_message_ids)
+                    message_id_to_conversation.unset(removed_message_id);
+            }
+            */
         }
         
         // Look for trimmed conversations no longer holding messages in this folder;
         // those are then evaporated themselves
+        /*
         int evaporated_count = 0;
         foreach (Conversation conversation in trimmed_conversations.get_keys().to_array()) {
             if (conversation.any_in_folder_path(folder.path))
@@ -666,6 +672,7 @@ public class Geary.App.ConversationMonitor : BaseObject {
             debug("Evaporated %d conversations from %s due to no in-folder messages",
                 evaporated_count, folder.to_string());
         }
+        */
         
         if (trimmed_conversations.size > 0) {
             debug("Trimmed %d conversations of %d emails from %s", trimmed_conversations.get_keys().size,
@@ -726,20 +733,16 @@ public class Geary.App.ConversationMonitor : BaseObject {
     
     internal async void reseed_async(string why) {
         Geary.EmailIdentifier? earliest_id = yield get_lowest_email_id_async(null);
-        try {
-            if (earliest_id != null) {
-                debug("ConversationMonitor (%s) reseeding starting from Email ID %s on opened %s", why,
-                    earliest_id.to_string(), folder.to_string());
-                yield load_by_id_async(earliest_id, int.MAX,
-                    Geary.Folder.ListFlags.OLDEST_TO_NEWEST | Geary.Folder.ListFlags.INCLUDING_ID,
-                    cancellable_monitor);
-            } else {
-                debug("ConversationMonitor (%s) reseeding latest %d emails on opened %s", why,
-                    min_window_count, folder.to_string());
-                yield load_by_id_async(null, min_window_count, Geary.Folder.ListFlags.NONE, 
cancellable_monitor);
-            }
-        } catch (Error e) {
-            debug("Reseed error: %s", e.message);
+        if (earliest_id != null) {
+            debug("ConversationMonitor (%s) reseeding starting from Email ID %s on opened %s", why,
+                earliest_id.to_string(), folder.to_string());
+            yield load_by_id_async(earliest_id, int.MAX,
+                Geary.Folder.ListFlags.OLDEST_TO_NEWEST | Geary.Folder.ListFlags.INCLUDING_ID,
+                cancellable_monitor);
+        } else {
+            debug("ConversationMonitor (%s) reseeding latest %d emails on opened %s", why,
+                min_window_count, folder.to_string());
+            yield load_by_id_async(null, min_window_count, Geary.Folder.ListFlags.NONE, cancellable_monitor);
         }
         
         if (!reseed_notified) {
@@ -793,19 +796,11 @@ public class Geary.App.ConversationMonitor : BaseObject {
             if (num_to_load < WINDOW_FILL_MESSAGE_COUNT)
                 num_to_load = WINDOW_FILL_MESSAGE_COUNT;
             
-            try {
-                yield load_by_id_async(low_id, num_to_load, flags, cancellable_monitor);
-            } catch(Error e) {
-                debug("Error filling conversation window: %s", e.message);
-            }
+            yield load_by_id_async(low_id, num_to_load, flags, cancellable_monitor);
         } else {
             // No existing messages or an insert invalidated our existing list,
             // need to start from scratch.
-            try {
-                yield load_by_id_async(null, min_window_count, flags, cancellable_monitor);
-            } catch(Error e) {
-                debug("Error filling conversation window: %s", e.message);
-            }
+            yield load_by_id_async(null, min_window_count, flags, cancellable_monitor);
         }
         
         // Run again to make sure we're full unless we ran out of messages.
diff --git a/src/engine/app/app-conversation.vala b/src/engine/app/app-conversation.vala
index e549a36..b3d5aee 100644
--- a/src/engine/app/app-conversation.vala
+++ b/src/engine/app/app-conversation.vala
@@ -260,30 +260,21 @@ public class Geary.App.Conversation : BaseObject {
         return true;
     }
     
-    // Returns the removed Message-IDs
-    internal Gee.Set<RFC822.MessageID>? remove(Email email) {
+    // Returns whether the email was completely removed from the conversation
+    internal bool remove(Email email, Geary.FolderPath path) {
+        path_map.remove(email.id, path);
+        if (path_map.get(email.id).size != 0)
+            return false;
+        
         emails.unset(email.id);
         sent_date_ascending.remove(email);
         sent_date_descending.remove(email);
         recv_date_ascending.remove(email);
         recv_date_descending.remove(email);
-        path_map.remove_all(email.id);
-        
-        Gee.Set<RFC822.MessageID> removed_message_ids = new Gee.HashSet<RFC822.MessageID>();
-        
-        Gee.Set<RFC822.MessageID>? ancestors = email.get_ancestors();
-        if (ancestors != null) {
-            foreach (RFC822.MessageID ancestor_id in ancestors) {
-                // if remove() changes set (i.e. it was present) but no longer present, that
-                // means the ancestor_id was the last one and is formally removed
-                if (message_ids.remove(ancestor_id) && !message_ids.contains(ancestor_id))
-                    removed_message_ids.add(ancestor_id);
-            }
-        }
         
         trimmed(email);
         
-        return (removed_message_ids.size > 0) ? removed_message_ids : null;
+        return true;
     }
     
     /**
diff --git a/src/engine/app/conversation-monitor/app-conversation-set.vala 
b/src/engine/app/conversation-monitor/app-conversation-set.vala
index 1466e75..87a8212 100644
--- a/src/engine/app/conversation-monitor/app-conversation-set.vala
+++ b/src/engine/app/conversation-monitor/app-conversation-set.vala
@@ -266,6 +266,7 @@ private class Geary.App.ConversationSet : BaseObject {
         if (!email_id_map.unset(email.id))
             error("Email %s already removed from conversation set", email.id.to_string());
         
+        /*
         Gee.Set<Geary.RFC822.MessageID>? removed_message_ids = conversation.remove(email);
         if (removed_message_ids != null) {
             foreach (Geary.RFC822.MessageID removed_message_id in removed_message_ids) {
@@ -275,6 +276,7 @@ private class Geary.App.ConversationSet : BaseObject {
                 }
             }
         }
+        */
     }
     
     private void remove_conversation(Conversation conversation) {