From 7f034107b0a53fa60e4c0a5ea377f0dc33e38a66 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 10:24:23 +1100 Subject: [PATCH 1/6] Fix race when Application.Contoller's constructor opens a window Don't try to open windows from the constructor since MainWindow needs access to the controller via the applictaion, but the application can't set the controller instance until is is constructed. --- .../application/application-client.vala | 30 ++++++++++++++++- .../application/application-controller.vala | 33 ------------------- 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/client/application/application-client.vala b/src/client/application/application-client.vala index aa853fba..4241eed4 100644 --- a/src/client/application/application-client.vala +++ b/src/client/application/application-client.vala @@ -808,7 +808,23 @@ public class Application.Client : Gtk.Application { this.controller.register_window(window); window.focus_in_event.connect(on_main_window_focus_in); if (select_first_inbox) { - window.select_first_inbox(true); + if (!window.select_first_inbox(true)) { + // The first inbox wasn't selected, so the account is + // likely still loading folders after being + // opened. Add a listener to try again later. + try { + Geary.Account first = Geary.Collection.get_first( + this.engine.get_accounts() + ); + if (first != null) { + first.folders_available_unavailable.connect_after( + on_folders_first_available + ); + } + } catch (GLib.Error error) { + debug("Error getting Inbox for first account"); + } + } } return window; } @@ -1093,6 +1109,18 @@ public class Application.Client : Gtk.Application { } } + private void on_folders_first_available(Geary.Account account, + Gee.BidirSortedSet? available, + Gee.BidirSortedSet? unavailable + ) { + if (get_active_main_window().select_first_inbox(true)) { + // The handler has done its job, so disconnect it + account.folders_available_unavailable.disconnect( + on_folders_first_available + ); + } + } + private bool on_main_window_focus_in(Gtk.Widget widget, Gdk.EventFocus event) { MainWindow? main = widget as MainWindow; diff --git a/src/client/application/application-controller.vala b/src/client/application/application-controller.vala index 909cade5..ec46ed5d 100644 --- a/src/client/application/application-controller.vala +++ b/src/client/application/application-controller.vala @@ -227,27 +227,6 @@ internal class Application.Controller : Geary.BaseObject { warning("Error loading accounts: %s", e.message); } - // Since the accounts may still be loading folders, when the - // main window first opens no folder might be available to be - // selected. Add look for the inbox and if not found, add a - // listener here as a once off for when it is loaded. - if (!application.get_active_main_window().select_first_inbox(true)) { - // Connect after so the folder is added to any - // open main windows first. - try { - Geary.Account first = Geary.Collection.get_first( - application.engine.get_accounts() - ); - if (first != null) { - first.folders_available_unavailable.connect_after( - on_folders_first_available - ); - } - } catch (GLib.Error error) { - debug("Error getting Inbox for first account"); - } - } - // Expunge any deleted accounts in the background, so we're // not blocking the app continuing to open. this.expunge_accounts.begin(); @@ -1387,18 +1366,6 @@ internal class Application.Controller : Geary.BaseObject { } } - private void on_folders_first_available(Geary.Account account, - Gee.BidirSortedSet? available, - Gee.BidirSortedSet? unavailable - ) { - if (application.get_active_main_window().select_first_inbox(true)) { - // The handler has done its job, so disconnect it - account.folders_available_unavailable.disconnect( - on_folders_first_available - ); - } - } - private bool should_notify_new_messages(Geary.Folder folder) { // A monitored folder must be selected to squelch notifications; // if conversation list is at top of display, don't display From c37a5412a79c645dfef8c3a0f90a84499bb44104 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 10:48:47 +1100 Subject: [PATCH 2/6] Take copies of ConversationListView's selection for undo ops Undoable operations like trashing or marking messages need to take a copy of the conversation lists's selection so that as the selection changes over time, the op's underlying collection doesn't also change. --- .../application/application-client.vala | 6 +--- .../application/application-main-window.vala | 30 ++++++++----------- .../conversation-list-view.vala | 23 ++++++++------ .../conversation-viewer.vala | 2 +- 4 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/client/application/application-client.vala b/src/client/application/application-client.vala index 4241eed4..579158c9 100644 --- a/src/client/application/application-client.vala +++ b/src/client/application/application-client.vala @@ -1021,13 +1021,9 @@ public class Application.Client : Gtk.Application { // If there was an existing active main, select the same // account/folder/conversation. MainWindow? current = this.last_active_main_window; - // Make a copy of the selection so the underlying collection - // doesn't change as the selection does. this.new_window.begin( current.selected_folder, - Geary.traverse( - current.conversation_list_view.get_selected_conversations() - ).to_linked_list() + current.conversation_list_view.copy_selected() ); } diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index d9be7a5a..6f13a07e 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -1576,7 +1576,7 @@ public class Application.MainWindow : Gee.Collection ids = new Gee.LinkedList(); foreach (Geary.App.Conversation convo in - this.conversation_list_view.get_selected_conversations()) { + this.conversation_list_view.get_selected()) { ids.add_all(convo.get_email_ids()); } try { @@ -1874,13 +1874,9 @@ public class Application.MainWindow : private void on_conversation_activated(Geary.App.Conversation activated) { if (this.selected_folder != null) { if (this.selected_folder.special_folder_type != DRAFTS) { - // Make a copy of the selection so the underlying - // collection doesn't change as the selection does. this.application.new_window.begin( this.selected_folder, - Geary.traverse( - this.conversation_list_view.get_selected_conversations() - ).to_linked_list() + this.conversation_list_view.copy_selected() ); } else { // TODO: Determine how to map between conversations @@ -1974,7 +1970,7 @@ public class Application.MainWindow : bool starred_selected = false; bool unstarred_selected = false; foreach (Geary.App.Conversation conversation in - this.conversation_list_view.get_selected_conversations()) { + this.conversation_list_view.get_selected()) { if (conversation.is_unread()) unread_selected = true; @@ -2031,7 +2027,7 @@ public class Application.MainWindow : if (location != null) { this.application.controller.mark_conversations.begin( location, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), Geary.EmailFlags.UNREAD, false, (obj, res) => { @@ -2050,7 +2046,7 @@ public class Application.MainWindow : if (location != null) { this.application.controller.mark_conversations.begin( location, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), Geary.EmailFlags.UNREAD, true, (obj, res) => { @@ -2069,7 +2065,7 @@ public class Application.MainWindow : if (location != null) { this.application.controller.mark_conversations.begin( location, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), Geary.EmailFlags.FLAGGED, true, (obj, res) => { @@ -2088,7 +2084,7 @@ public class Application.MainWindow : if (location != null) { this.application.controller.mark_conversations.begin( location, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), Geary.EmailFlags.FLAGGED, false, (obj, res) => { @@ -2112,7 +2108,7 @@ public class Application.MainWindow : this.application.controller.move_conversations_special.begin( source, destination, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), (obj, res) => { try { this.application.controller.move_conversations_special.end(res); @@ -2131,7 +2127,7 @@ public class Application.MainWindow : this.application.controller.move_conversations.begin( source, destination, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), (obj, res) => { try { this.application.controller.move_conversations.end(res); @@ -2151,7 +2147,7 @@ public class Application.MainWindow : this.application.controller.copy_conversations.begin( source, destination, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), (obj, res) => { try { this.application.controller.copy_conversations.end(res); @@ -2170,7 +2166,7 @@ public class Application.MainWindow : this.application.controller.move_conversations_special.begin( source, ARCHIVE, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), (obj, res) => { try { this.application.controller.move_conversations_special.end(res); @@ -2188,7 +2184,7 @@ public class Application.MainWindow : this.application.controller.move_conversations_special.begin( source, Geary.SpecialFolderType.TRASH, - this.conversation_list_view.get_selected_conversations(), + this.conversation_list_view.copy_selected(), (obj, res) => { try { this.application.controller.move_conversations_special.end(res); @@ -2204,7 +2200,7 @@ public class Application.MainWindow : Geary.FolderSupport.Remove target = this.selected_folder as Geary.FolderSupport.Remove; Gee.Collection conversations = - this.conversation_list_view.get_selected_conversations(); + this.conversation_list_view.copy_selected(); if (target != null && this.prompt_delete_conversations(conversations.size)) { this.application.controller.delete_conversations.begin( target, diff --git a/src/client/conversation-list/conversation-list-view.vala b/src/client/conversation-list/conversation-list-view.vala index ab1c5e57..6440222f 100644 --- a/src/client/conversation-list/conversation-list-view.vala +++ b/src/client/conversation-list/conversation-list-view.vala @@ -131,11 +131,18 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface { selection.changed.connect(on_selection_changed); } - /** Returns a read-only collection of the current selection. */ - public Gee.Set get_selected_conversations() { + /** Returns a read-only iteration of the current selection. */ + public Gee.Set get_selected() { return this.selected.read_only_view; } + /** Returns a copy of the current selection. */ + public Gee.Set copy_selected() { + var copy = new Gee.HashSet(); + copy.add_all(this.selected); + return copy; + } + public void inhibit_next_autoselect() { this.should_inhibit_autoselect = true; } @@ -288,13 +295,11 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface { // Get the current conversation. If it's selected, we'll apply the mark operation to // all selected conversations; otherwise, it just applies to this one. Geary.App.Conversation conversation = get_model().get_conversation_at_path(path); - Gee.Collection to_mark; - if (this.selected.contains(conversation)) - // take a copy of currently selected for handling to - // the signal - to_mark = get_selected_conversations(); - else - to_mark = Geary.Collection.single(conversation); + Gee.Collection to_mark = ( + this.selected.contains(conversation) + ? copy_selected() + : Geary.Collection.single(conversation) + ); if (read_clicked) { mark_conversations(to_mark, Geary.EmailFlags.UNREAD); diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index 2c3ec220..3d2a0226 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -156,7 +156,7 @@ public class ConversationViewer : Gtk.Stack, Geary.BaseInterface { // XXX move the ConversationListView management code into // MainWindow or somewhere more appropriate ConversationListView conversation_list = main_window.conversation_list_view; - this.selection_while_composing = conversation_list.get_selected_conversations(); + this.selection_while_composing = conversation_list.copy_selected(); conversation_list.get_selection().unselect_all(); box.vanished.connect(on_composer_closed); From 1deae845c30753c317da45426f0d5bcc4b3668d3 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 13:23:04 +1100 Subject: [PATCH 3/6] Fix indentation of util-collection.vala --- src/engine/util/util-collection.vala | 376 ++++++++++++++------------- 1 file changed, 193 insertions(+), 183 deletions(-) diff --git a/src/engine/util/util-collection.vala b/src/engine/util/util-collection.vala index 0ba4e09e..46ba01d1 100644 --- a/src/engine/util/util-collection.vala +++ b/src/engine/util/util-collection.vala @@ -6,203 +6,213 @@ namespace Geary.Collection { -public delegate uint8 ByteTransformer(uint8 b); + public delegate uint8 ByteTransformer(uint8 b); -public inline bool is_empty(Gee.Collection? c) { - return c == null || c.size == 0; -} - -/** Returns a modifiable collection containing a single element. */ -public Gee.Collection single(T element) { - Gee.Collection single = new Gee.LinkedList(); - single.add(element); - return single; -} - -/** Returns a modifiable map containing a single entry. */ -public Gee.Map single_map(K key, V value) { - Gee.Map single = new Gee.HashMap(); - single.set(key, value); - return single; -} - -// A substitute for ArrayList.wrap() for compatibility with older versions of Gee. -public Gee.ArrayList array_list_wrap(G[] a, owned Gee.EqualDataFunc? equal_func = null) { - Gee.ArrayList list = new Gee.ArrayList((owned) equal_func); - add_all_array(list, a); - return list; -} - -public Gee.ArrayList to_array_list(Gee.Collection c) { - Gee.ArrayList list = new Gee.ArrayList(); - list.add_all(c); - - return list; -} - -public Gee.HashMap to_hash_map( - Gee.Collection c, Gee.MapFunc key_selector) { - Gee.HashMap map = new Gee.HashMap(); - foreach (Value v in c) - map.set(key_selector(v), v); - return map; -} - -public void add_all_array(Gee.Collection c, G[] ar) { - foreach (G g in ar) - c.add(g); -} - -public G? get_first(Gee.Collection c) { - Gee.Iterator iter = c.iterator(); - - return iter.next() ? iter.get() : null; -} - -/** - * Returns the first element in the Collection that passes the Predicte function. - * - * The Collection is walked in Iterator order. - */ -public G? find_first(Gee.Collection c, owned Gee.Predicate pred) { - Gee.Iterator iter = c.iterator(); - while (iter.next()) { - if (pred(iter.get())) - return iter.get(); + public inline bool is_empty(Gee.Collection? c) { + return c == null || c.size == 0; } - return null; -} + /** Returns a modifiable collection containing a single element. */ + public Gee.Collection single(T element) { + Gee.Collection single = new Gee.LinkedList(); + single.add(element); + return single; + } -public bool are_sets_equal(Gee.Set a, Gee.Set b) { - if (a.size != b.size) - return false; + /** Returns a modifiable map containing a single entry. */ + public Gee.Map single_map(K key, V value) { + Gee.Map single = new Gee.HashMap(); + single.set(key, value); + return single; + } - foreach (G element in a) { - if (!b.contains(element)) + // A substitute for ArrayList.wrap() for compatibility with older versions of Gee. + public Gee.ArrayList array_list_wrap(G[] a, owned Gee.EqualDataFunc? equal_func = null) { + Gee.ArrayList list = new Gee.ArrayList((owned) equal_func); + add_all_array(list, a); + return list; + } + + public Gee.ArrayList to_array_list(Gee.Collection c) { + Gee.ArrayList list = new Gee.ArrayList(); + list.add_all(c); + return list; + } + + public Gee.HashMap to_hash_map( + Gee.Collection c, Gee.MapFunc key_selector) { + Gee.HashMap map = new Gee.HashMap(); + foreach (Value v in c) { + map.set(key_selector(v), v); + } + return map; + } + + public void add_all_array(Gee.Collection c, G[] ar) { + foreach (G g in ar) { + c.add(g); + } + } + + public G? get_first(Gee.Collection c) { + Gee.Iterator iter = c.iterator(); + return iter.next() ? iter.get() : null; + } + + /** + * Returns the first element in the Collection that passes the Predicte function. + * + * The Collection is walked in Iterator order. + */ + public G? find_first(Gee.Collection c, owned Gee.Predicate pred) { + Gee.Iterator iter = c.iterator(); + while (iter.next()) { + if (pred(iter.get())) + return iter.get(); + } + + return null; + } + + public bool are_sets_equal(Gee.Set a, Gee.Set b) { + if (a.size != b.size) { return false; + } + + foreach (G element in a) { + if (!b.contains(element)) { + return false; + } + } + + return true; } - return true; -} + /** + * Removes all elements from the Collection that do pass the Predicate function. + * + * Note that this modifies the supplied Collection. + */ + public Gee.Collection remove_if(Gee.Collection c, owned Gee.Predicate pred) { + Gee.Iterator iter = c.iterator(); + while (iter.next()) { + if (pred(iter.get())) + iter.remove(); + } -/** - * Removes all elements from the Collection that do pass the Predicate function. - * - * Note that this modifies the supplied Collection. - */ -public Gee.Collection remove_if(Gee.Collection c, owned Gee.Predicate pred) { - Gee.Iterator iter = c.iterator(); - while (iter.next()) { - if (pred(iter.get())) - iter.remove(); + return c; } - return c; -} - -/** - * Sets the dest Map with all keys and values in src. - */ -public void map_set_all(Gee.Map dest, Gee.Map src) { - foreach (K key in src.keys) - dest.set(key, src.get(key)); -} - -/** - * Sets multiple elements with the same key in a MultiMap. - */ -public void multi_map_set_all(Gee.MultiMap dest, K key, Gee.Collection values) { - foreach (V value in values) - dest.set(key, value); -} - -/** - * Removes all keys from the Map. - */ -public void map_unset_all_keys(Gee.Map map, Gee.Collection keys) { - foreach (K key in keys) - map.unset(key); -} - -/** - * Return a MultiMap of value => key of the input map's key => values. - */ -public Gee.MultiMap reverse_multi_map(Gee.MultiMap map) { - Gee.HashMultiMap reverse = new Gee.HashMultiMap(); - foreach (K key in map.get_keys()) { - foreach (V value in map.get(key)) - reverse.set(value, key); + /** + * Sets the dest Map with all keys and values in src. + */ + public void map_set_all(Gee.Map dest, Gee.Map src) { + foreach (K key in src.keys) { + dest.set(key, src.get(key)); + } } - return reverse; -} - -/** - * To be used by a Hashable's to_hash() method. - */ -public inline uint int64_hash(int64 value) { - return hash_memory(&value, sizeof(int64)); -} - -/** - * To be used as hash_func for Gee collections. - */ -public inline uint int64_hash_func(int64? n) { - return hash_memory((uint8 *) n, sizeof(int64)); -} - -/** - * To be used as equal_func for Gee collections. - */ -public bool int64_equal_func(int64? a, int64? b) { - int64 *bia = (int64 *) a; - int64 *bib = (int64 *) b; - - return (*bia) == (*bib); -} - -/** - * A rotating-XOR hash that can be used to hash memory buffers of any size. - */ -public uint hash_memory(void *ptr, size_t bytes) { - if (ptr == null || bytes == 0) - return 0; - - uint8 *u8 = (uint8 *) ptr; - - // initialize hash to first byte value and then rotate-XOR from there - uint hash = *u8; - for (int ctr = 1; ctr < bytes; ctr++) - hash = (hash << 4) ^ (hash >> 28) ^ (*u8++); - - return hash; -} - -/** - * A rotating-XOR hash that can be used to hash memory buffers of any size until a terminator byte - * is found. - * - * A {@link ByteTransformer} may be supplied to convert bytes before they are hashed. - * - * Returns zero if the initial byte is the terminator. - */ -public uint hash_memory_stream(void *ptr, uint8 terminator, ByteTransformer? cb) { - uint8 *u8 = (uint8 *) ptr; - - uint hash = 0; - for (;;) { - uint8 b = *u8++; - if (b == terminator) - break; - - if (cb != null) - b = cb(b); - - hash = (hash << 4) ^ (hash >> 28) ^ b; + /** + * Sets multiple elements with the same key in a MultiMap. + */ + public void multi_map_set_all(Gee.MultiMap dest, K key, Gee.Collection values) { + foreach (V value in values) { + dest.set(key, value); + } } - return hash; -} + /** + * Removes all keys from the Map. + */ + public void map_unset_all_keys(Gee.Map map, Gee.Collection keys) { + foreach (K key in keys) { + map.unset(key); + } + } + + /** + * Return a MultiMap of value => key of the input map's key => values. + */ + public Gee.MultiMap reverse_multi_map(Gee.MultiMap map) { + Gee.HashMultiMap reverse = new Gee.HashMultiMap(); + foreach (K key in map.get_keys()) { + foreach (V value in map.get(key)) { + reverse.set(value, key); + } + } + + return reverse; + } + + /** + * To be used by a Hashable's to_hash() method. + */ + public inline uint int64_hash(int64 value) { + return hash_memory(&value, sizeof(int64)); + } + + /** + * To be used as hash_func for Gee collections. + */ + public inline uint int64_hash_func(int64? n) { + return hash_memory((uint8 *) n, sizeof(int64)); + } + + /** + * To be used as equal_func for Gee collections. + */ + public bool int64_equal_func(int64? a, int64? b) { + int64 *bia = (int64 *) a; + int64 *bib = (int64 *) b; + + return (*bia) == (*bib); + } + + /** + * A rotating-XOR hash that can be used to hash memory buffers of any size. + */ + public uint hash_memory(void *ptr, size_t bytes) { + if (ptr == null || bytes == 0) { + return 0; + } + + uint8 *u8 = (uint8 *) ptr; + + // initialize hash to first byte value and then rotate-XOR from there + uint hash = *u8; + for (int ctr = 1; ctr < bytes; ctr++) { + hash = (hash << 4) ^ (hash >> 28) ^ (*u8++); + } + + return hash; + } + + /** + * A rotating-XOR hash that can be used to hash memory buffers of any size until a terminator byte + * is found. + * + * A {@link ByteTransformer} may be supplied to convert bytes before they are hashed. + * + * Returns zero if the initial byte is the terminator. + */ + public uint hash_memory_stream(void *ptr, uint8 terminator, ByteTransformer? cb) { + uint8 *u8 = (uint8 *) ptr; + + uint hash = 0; + for (;;) { + uint8 b = *u8++; + if (b == terminator) { + break; + } + + if (cb != null) { + b = cb(b); + } + + hash = (hash << 4) ^ (hash >> 28) ^ b; + } + + return hash; + } } From e4cbff8bfa9e7d50d6c6319a1acc86bea15bd0fd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 14:20:42 +1100 Subject: [PATCH 4/6] De-cruftify Geary.Collection utility functions Remove functions that have as-convenient methods in up-tp-date Gee, rename ::get_first to ::first so it reads better and add API docs, replace ::to_array_list with ::copy, since that's what's actually needed. --- .../application-attachment-manager.vala | 2 +- .../application/application-client.vala | 4 +- .../application/application-main-window.vala | 6 +- src/client/composer/composer-widget.vala | 14 ++-- .../conversation-list-view.vala | 24 +++--- .../conversation-list-box.vala | 2 +- .../conversation-web-view.vala | 2 +- .../app-conversation-set.vala | 2 +- .../app/email-store/app-copy-operation.vala | 6 +- .../app/email-store/app-fetch-operation.vala | 2 +- .../app/email-store/app-mark-operation.vala | 6 +- .../imap-db/search/imap-db-search-folder.vala | 3 +- .../replay-ops/imap-engine-create-email.vala | 2 +- src/engine/imap/api/imap-account-session.vala | 10 ++- src/engine/imap/api/imap-folder-session.vala | 3 +- .../imap/transport/imap-client-session.vala | 2 +- .../rfc822/rfc822-mailbox-addresses.vala | 18 ++--- src/engine/smtp/smtp-client-service.vala | 2 +- src/engine/util/util-collection.vala | 77 ++++--------------- src/engine/util/util-config-file.vala | 12 +-- src/engine/util/util-iterable.vala | 12 ++- src/engine/util/util-object.vala | 10 +-- .../app/app-conversation-monitor-test.vala | 6 +- .../engine/app/app-conversation-set-test.vala | 8 +- .../common/common-contact-harvester-test.vala | 8 +- .../common-contact-store-impl-test.vala | 8 +- .../engine/rfc822-mailbox-addresses-test.vala | 33 ++++++++ 27 files changed, 135 insertions(+), 149 deletions(-) diff --git a/src/client/application/application-attachment-manager.vala b/src/client/application/application-attachment-manager.vala index 805ecb6f..f4b5eb06 100644 --- a/src/client/application/application-attachment-manager.vala +++ b/src/client/application/application-attachment-manager.vala @@ -40,7 +40,7 @@ public class Application.AttachmentManager : GLib.Object { GLib.Cancellable? cancellable) { if (attachments.size == 1) { return yield save_attachment( - Geary.Collection.get_first(attachments), null, cancellable + Geary.Collection.first(attachments), null, cancellable ); } else { return yield save_all(attachments, cancellable); diff --git a/src/client/application/application-client.vala b/src/client/application/application-client.vala index 579158c9..88d32887 100644 --- a/src/client/application/application-client.vala +++ b/src/client/application/application-client.vala @@ -813,7 +813,7 @@ public class Application.Client : Gtk.Application { // likely still loading folders after being // opened. Add a listener to try again later. try { - Geary.Account first = Geary.Collection.get_first( + Geary.Account? first = Geary.Collection.first( this.engine.get_accounts() ); if (first != null) { @@ -1131,7 +1131,7 @@ public class Application.Client : Gtk.Application { if (main != null) { this.controller.unregister_window(main); if (this.last_active_main_window == main) { - this.last_active_main_window = Geary.Collection.get_first( + this.last_active_main_window = Geary.Collection.first( get_main_windows() ); } diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index 6f13a07e..e163d1bc 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -586,7 +586,7 @@ public class Application.MainWindow : if (loaded.size == 1) { // A single conversation was loaded, so ensure we // scroll to the email in the conversation. - Geary.App.Conversation target = Geary.Collection.get_first(loaded); + Geary.App.Conversation? target = Geary.Collection.first(loaded); ConversationListBox? current_list = this.conversation_viewer.current_list; if (current_list != null && @@ -860,7 +860,7 @@ public class Application.MainWindow : private Geary.Folder? get_first_inbox() { Geary.Folder? inbox = null; try { - Geary.Account first = Geary.Collection.get_first( + Geary.Account? first = Geary.Collection.first( this.application.engine.get_accounts() ); if (first != null) { @@ -1285,7 +1285,7 @@ public class Application.MainWindow : case 1: update_conversation_actions(SINGLE); - Geary.App.Conversation convo = Geary.Collection.get_first(to_select); + Geary.App.Conversation? convo = Geary.Collection.first(to_select); // It's possible for a conversation with zero email to // be selected, when it has just evaporated after its diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index d47af36c..9bbd6660 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -624,24 +624,26 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { // Assemble the headers. if (email.length > 0 && headers.contains("to")) - this.to = "%s,%s".printf(email, Geary.Collection.get_first(headers.get("to"))); + this.to = "%s,%s".printf( + email, Geary.Collection.first(headers.get("to")) + ); else if (email.length > 0) this.to = email; else if (headers.contains("to")) - this.to = Geary.Collection.get_first(headers.get("to")); + this.to = Geary.Collection.first(headers.get("to")); if (headers.contains("cc")) - this.cc = Geary.Collection.get_first(headers.get("cc")); + this.cc = Geary.Collection.first(headers.get("cc")); if (headers.contains("bcc")) - this.bcc = Geary.Collection.get_first(headers.get("bcc")); + this.bcc = Geary.Collection.first(headers.get("bcc")); if (headers.contains("subject")) - this.subject = Geary.Collection.get_first(headers.get("subject")); + this.subject = Geary.Collection.first(headers.get("subject")); if (headers.contains("body")) this.body_html = Geary.HTML.preserve_whitespace(Geary.HTML.escape_markup( - Geary.Collection.get_first(headers.get("body")))); + Geary.Collection.first(headers.get("body")))); Gee.List attachments = new Gee.LinkedList(); attachments.add_all(headers.get("attach")); diff --git a/src/client/conversation-list/conversation-list-view.vala b/src/client/conversation-list/conversation-list-view.vala index 6440222f..96acc4b8 100644 --- a/src/client/conversation-list/conversation-list-view.vala +++ b/src/client/conversation-list/conversation-list-view.vala @@ -492,8 +492,8 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface { } // only notify if different than what was previously reported - if (!Geary.Collection.are_sets_equal( - this.selected, new_selection)) { + if (this.selected.size != new_selection.size || + !this.selected.contains_all(new_selection)) { this.selected = new_selection; conversations_selected(this.selected.read_only_view); } @@ -520,18 +520,18 @@ public class ConversationListView : Gtk.TreeView, Geary.BaseInterface { // Always returns false, so it can be used as a one-time SourceFunc private bool update_visible_conversations() { + bool changed = false; Gee.Set visible_conversations = get_visible_conversations(); - if (current_visible_conversations != null - && Geary.Collection.are_sets_equal( - current_visible_conversations, visible_conversations)) { - return false; + if (this.current_visible_conversations == null || + this.current_visible_conversations.size != visible_conversations.size || + !this.current_visible_conversations.contains_all(visible_conversations)) { + this.current_visible_conversations = visible_conversations; + visible_conversations_changed( + this.current_visible_conversations.read_only_view + ); + changed = true; } - - current_visible_conversations = visible_conversations; - - visible_conversations_changed(current_visible_conversations.read_only_view); - - return false; + return changed; } private void schedule_visible_conversations_changed() { diff --git a/src/client/conversation-viewer/conversation-list-box.vala b/src/client/conversation-viewer/conversation-list-box.vala index 377f66e6..4e501aa4 100644 --- a/src/client/conversation-viewer/conversation-list-box.vala +++ b/src/client/conversation-viewer/conversation-list-box.vala @@ -678,7 +678,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface { id => this.conversation.contains_email_by_id(id) ).to_array_list(); valid_scroll_to.sort((a, b) => a.natural_sort_comparator(b)); - var first_scroll = Geary.Collection.get_first(valid_scroll_to); + var first_scroll = Geary.Collection.first(valid_scroll_to); if (first_scroll != null) { foreach (Geary.Email email in all_email) { diff --git a/src/client/conversation-viewer/conversation-web-view.vala b/src/client/conversation-viewer/conversation-web-view.vala index 217528ab..d22a836e 100644 --- a/src/client/conversation-viewer/conversation-web-view.vala +++ b/src/client/conversation-viewer/conversation-web-view.vala @@ -132,7 +132,7 @@ public class ConversationWebView : ClientWebView { }); controller.search( - Geary.Collection.get_first(terms), + Geary.Collection.first(terms), WebKit.FindOptions.CASE_INSENSITIVE | WebKit.FindOptions.WRAP_AROUND, 128 diff --git a/src/engine/app/conversation-monitor/app-conversation-set.vala b/src/engine/app/conversation-monitor/app-conversation-set.vala index ca626f6f..e9eb016a 100644 --- a/src/engine/app/conversation-monitor/app-conversation-set.vala +++ b/src/engine/app/conversation-monitor/app-conversation-set.vala @@ -302,7 +302,7 @@ private class Geary.App.ConversationSet : BaseObject { } else { Gee.Set associated = get_associated_conversations(email); - conversation = Collection.get_first(associated); + conversation = Collection.first(associated); if (conversation == null) { // Not in or related to any existing conversations, so // create one diff --git a/src/engine/app/email-store/app-copy-operation.vala b/src/engine/app/email-store/app-copy-operation.vala index 44c76fd7..f6d37079 100644 --- a/src/engine/app/email-store/app-copy-operation.vala +++ b/src/engine/app/email-store/app-copy-operation.vala @@ -19,9 +19,9 @@ private class Geary.App.CopyOperation : Geary.App.AsyncFolderOperation { Geary.FolderSupport.Copy? copy = folder as Geary.FolderSupport.Copy; assert(copy != null); - Gee.List list - = Geary.Collection.to_array_list(ids); - yield copy.copy_email_async(list, destination, cancellable); + yield copy.copy_email_async( + Collection.copy(ids), destination, cancellable + ); return ids; } } diff --git a/src/engine/app/email-store/app-fetch-operation.vala b/src/engine/app/email-store/app-fetch-operation.vala index eef93d10..4db6c457 100644 --- a/src/engine/app/email-store/app-fetch-operation.vala +++ b/src/engine/app/email-store/app-fetch-operation.vala @@ -20,7 +20,7 @@ private class Geary.App.FetchOperation : Geary.App.AsyncFolderOperation { Geary.Folder folder, Gee.Collection ids, Cancellable? cancellable) throws Error { assert(result == null); - Geary.EmailIdentifier? id = Geary.Collection.get_first(ids); + Geary.EmailIdentifier? id = Collection.first(ids); assert(id != null); result = yield folder.fetch_email_async( diff --git a/src/engine/app/email-store/app-mark-operation.vala b/src/engine/app/email-store/app-mark-operation.vala index f69b9049..00a0858c 100644 --- a/src/engine/app/email-store/app-mark-operation.vala +++ b/src/engine/app/email-store/app-mark-operation.vala @@ -21,9 +21,9 @@ private class Geary.App.MarkOperation : Geary.App.AsyncFolderOperation { Geary.FolderSupport.Mark? mark = folder as Geary.FolderSupport.Mark; assert(mark != null); - Gee.List list - = Geary.Collection.to_array_list(ids); - yield mark.mark_email_async(list, flags_to_add, flags_to_remove, cancellable); + yield mark.mark_email_async( + Collection.copy(ids), flags_to_add, flags_to_remove, cancellable + ); return ids; } } diff --git a/src/engine/imap-db/search/imap-db-search-folder.vala b/src/engine/imap-db/search/imap-db-search-folder.vala index 5664b361..34e3ee71 100644 --- a/src/engine/imap-db/search/imap-db-search-folder.vala +++ b/src/engine/imap-db/search/imap-db-search-folder.vala @@ -319,8 +319,7 @@ private class Geary.ImapDB.SearchFolder : Geary.SearchFolder, Geary.FolderSuppor yield folder.open_async(Geary.Folder.OpenFlags.NONE, cancellable); open = true; yield remove.remove_email_async( - Geary.Collection.to_array_list(ids), - cancellable + Collection.copy(ids), cancellable ); } finally { if (open) { diff --git a/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala b/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala index 9d519c47..51866395 100644 --- a/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala +++ b/src/engine/imap-engine/replay-ops/imap-engine-create-email.vala @@ -73,7 +73,7 @@ private class Geary.ImapEngine.CreateEmail : SendReplayOperation { ); if (results.size > 0) { - this.created_id = Collection.get_first(results.keys).id; + this.created_id = Collection.first(results.keys).id; } else { // Something went wrong creating/merging the message, // so pretend we don't know what its UID is so the diff --git a/src/engine/imap/api/imap-account-session.vala b/src/engine/imap/api/imap-account-session.vala index ff335262..63838b2f 100644 --- a/src/engine/imap/api/imap-account-session.vala +++ b/src/engine/imap/api/imap-account-session.vala @@ -397,9 +397,13 @@ internal class Geary.Imap.AccountSession : Geary.Imap.SessionObject { cancellable ); - assert(responses.size == 1); - - return Geary.Collection.get_first(responses.values); + var response = Collection.first(responses.values); + if (response == null) { + throw new ImapError.SERVER_ERROR( + "No status response received from server" + ); + } + return response; } private async Gee.Map diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala index 00f48ca8..d07199b2 100644 --- a/src/engine/imap/api/imap-folder-session.vala +++ b/src/engine/imap/api/imap-folder-session.vala @@ -688,7 +688,8 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { debug("Unable to retrieve COPYUID UIDs: %s", ierr.message); } - if (!Collection.is_empty(src_uids) && !Collection.is_empty(dst_uids)) { + if (src_uids != null && !src_uids.is_empty && + dst_uids != null && !dst_uids.is_empty) { Gee.Map copyuids = new Gee.HashMap(); int ctr = 0; for (;;) { diff --git a/src/engine/imap/transport/imap-client-session.vala b/src/engine/imap/transport/imap-client-session.vala index d3ff645a..d26aef7a 100644 --- a/src/engine/imap/transport/imap-client-session.vala +++ b/src/engine/imap/transport/imap-client-session.vala @@ -1281,7 +1281,7 @@ public class Geary.Imap.ClientSession : BaseObject { check_unsupported_send_command(cmd); // only issue one event to the state machine for all commands; either all succeed or all fail - MachineParams params = new MachineParams(Geary.Collection.get_first(cmds)); + MachineParams params = new MachineParams(Collection.first(cmds)); fsm.issue(Event.SEND_CMD, null, params); if (params.err != null) diff --git a/src/engine/rfc822/rfc822-mailbox-addresses.vala b/src/engine/rfc822/rfc822-mailbox-addresses.vala index f218944f..616c28ab 100644 --- a/src/engine/rfc822/rfc822-mailbox-addresses.vala +++ b/src/engine/rfc822/rfc822-mailbox-addresses.vala @@ -184,19 +184,11 @@ public class Geary.RFC822.MailboxAddresses : } public bool equal_to(MailboxAddresses other) { - if (this == other) - return true; - - if (addrs.size != other.addrs.size) - return false; - - Gee.HashSet first = new Gee.HashSet(); - first.add_all(addrs); - - Gee.HashSet second = new Gee.HashSet(); - second.add_all(other.addrs); - - return Collection.are_sets_equal(first, second); + return ( + this == other || + (this.addrs.size == other.addrs.size && + this.addrs.contains_all(other.addrs)) + ); } /** diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala index 69071d00..70893b38 100644 --- a/src/engine/smtp/smtp-client-service.vala +++ b/src/engine/smtp/smtp-client-service.vala @@ -428,7 +428,7 @@ public class Geary.Smtp.ClientService : Geary.ClientService { null, 1, REFERENCES, NONE, cancellable ); if (list != null && !list.is_empty) { - Email listed = Collection.get_first(list); + Email listed = Collection.first(list); if (listed.message_id != null && listed.message_id.equal_to(id)) { break; diff --git a/src/engine/util/util-collection.vala b/src/engine/util/util-collection.vala index 46ba01d1..4cf8316e 100644 --- a/src/engine/util/util-collection.vala +++ b/src/engine/util/util-collection.vala @@ -8,9 +8,6 @@ namespace Geary.Collection { public delegate uint8 ByteTransformer(uint8 b); - public inline bool is_empty(Gee.Collection? c) { - return c == null || c.size == 0; - } /** Returns a modifiable collection containing a single element. */ public Gee.Collection single(T element) { @@ -26,80 +23,34 @@ namespace Geary.Collection { return single; } - // A substitute for ArrayList.wrap() for compatibility with older versions of Gee. - public Gee.ArrayList array_list_wrap(G[] a, owned Gee.EqualDataFunc? equal_func = null) { - Gee.ArrayList list = new Gee.ArrayList((owned) equal_func); - add_all_array(list, a); - return list; + /** Returns a copy of the given collection in a new collection. */ + public Gee.Collection copy(Gee.Collection original) { + // Use a linked list, the returned value can't be accessed by + // index anyway + var copy = new Gee.LinkedList(); + copy.add_all(original); + return copy; } - public Gee.ArrayList to_array_list(Gee.Collection c) { - Gee.ArrayList list = new Gee.ArrayList(); - list.add_all(c); - return list; - } - - public Gee.HashMap to_hash_map( - Gee.Collection c, Gee.MapFunc key_selector) { - Gee.HashMap map = new Gee.HashMap(); - foreach (Value v in c) { - map.set(key_selector(v), v); - } - return map; - } - - public void add_all_array(Gee.Collection c, G[] ar) { - foreach (G g in ar) { - c.add(g); - } - } - - public G? get_first(Gee.Collection c) { + /** Returns the first element from a collection. */ + public G? first(Gee.Collection c) { Gee.Iterator iter = c.iterator(); return iter.next() ? iter.get() : null; } /** - * Returns the first element in the Collection that passes the Predicte function. - * - * The Collection is walked in Iterator order. - */ - public G? find_first(Gee.Collection c, owned Gee.Predicate pred) { - Gee.Iterator iter = c.iterator(); - while (iter.next()) { - if (pred(iter.get())) - return iter.get(); - } - - return null; - } - - public bool are_sets_equal(Gee.Set a, Gee.Set b) { - if (a.size != b.size) { - return false; - } - - foreach (G element in a) { - if (!b.contains(element)) { - return false; - } - } - - return true; - } - - /** - * Removes all elements from the Collection that do pass the Predicate function. + * Removes all elements that pass the given predicate. * * Note that this modifies the supplied Collection. */ - public Gee.Collection remove_if(Gee.Collection c, owned Gee.Predicate pred) { + public Gee.Collection remove_if(Gee.Collection c, + owned Gee.Predicate pred) { Gee.Iterator iter = c.iterator(); while (iter.next()) { - if (pred(iter.get())) + if (pred(iter.get())) { iter.remove(); + } } - return c; } diff --git a/src/engine/util/util-config-file.vala b/src/engine/util/util-config-file.vala index 61ced4c4..6b938285 100644 --- a/src/engine/util/util-config-file.vala +++ b/src/engine/util/util-config-file.vala @@ -126,20 +126,20 @@ public class Geary.ConfigFile { } public Gee.List get_string_list(string key) { + var strs = new Gee.ArrayList(); try { - string[] list = this.backing.get_string_list(this.name, key); - if (list.length > 0) - return Geary.Collection.array_list_wrap(list); + strs.add_all_array(this.backing.get_string_list(this.name, key)); } catch (GLib.KeyFileError err) { // Oh well } - return new Gee.ArrayList(); + return strs; } public Gee.List get_required_string_list(string key) throws GLib.KeyFileError { - string[] list = this.backing.get_string_list(this.name, key); - return Geary.Collection.array_list_wrap(list); + var strs = new Gee.ArrayList(); + strs.add_all_array(this.backing.get_string_list(this.name, key)); + return strs; } public void set_string_list(string key, Gee.List value) { diff --git a/src/engine/util/util-iterable.vala b/src/engine/util/util-iterable.vala index 2cff4b16..29ff8e27 100644 --- a/src/engine/util/util-iterable.vala +++ b/src/engine/util/util-iterable.vala @@ -20,7 +20,9 @@ namespace Geary { va_list args = va_list(); G arg = g; - Gee.ArrayList list = new Gee.ArrayList(); + // Use a linked list since we will only ever be iterating over + // it + var list = new Gee.LinkedList(); do { list.add(arg); } while((arg = args.arg()) != null); @@ -31,8 +33,12 @@ namespace Geary { /** * Take an array of items and return a Geary.Iterable for convenience. */ - public Geary.Iterable iterate_array(G[] a) { - return Geary.traverse(Geary.Collection.array_list_wrap(a)); + public Geary.Iterable iterate_array(G[] a, owned Gee.EqualDataFunc? equal_func = null) { + // Use a linked list since we will only ever be iterating over + // it + var list = new Gee.LinkedList((owned) equal_func); + list.add_all_array(a); + return Geary.traverse(list); } } diff --git a/src/engine/util/util-object.vala b/src/engine/util/util-object.vala index 2bbc0dd8..071da6f1 100644 --- a/src/engine/util/util-object.vala +++ b/src/engine/util/util-object.vala @@ -12,12 +12,10 @@ namespace Geary.ObjectUtils { public Gee.List? mirror_properties(Object source, Object dest, BindingFlags flags = GLib.BindingFlags.DEFAULT | GLib.BindingFlags.SYNC_CREATE) { // Make sets of both object's properties. - Gee.HashSet source_properties = new Gee.HashSet(); - Collection.add_all_array(source_properties, - source.get_class().list_properties()); - Gee.HashSet dest_properties = new Gee.HashSet(); - Collection.add_all_array(dest_properties, - dest.get_class().list_properties()); + Gee.HashSet source_properties = + iterate_array(source.get_class().list_properties()).to_hash_set(); + Gee.HashSet dest_properties = + iterate_array(dest.get_class().list_properties()).to_hash_set(); // Remove properties from source_properties that are not in both sets. source_properties.retain_all(dest_properties); diff --git a/test/engine/app/app-conversation-monitor-test.vala b/test/engine/app/app-conversation-monitor-test.vala index 3d10684e..b38ad0a3 100644 --- a/test/engine/app/app-conversation-monitor-test.vala +++ b/test/engine/app/app-conversation-monitor-test.vala @@ -133,7 +133,7 @@ class Geary.App.ConversationMonitorTest : TestCase { assert_non_null(monitor.window_lowest, "Lowest window id"); assert_equal(e1.id, monitor.window_lowest, "Lowest window id"); - Conversation c1 = Geary.Collection.get_first(monitor.read_only_view); + Conversation c1 = Collection.first(monitor.read_only_view); assert_equal(e1, c1.get_email_by_id(e1.id), "Email not present in conversation"); } @@ -175,7 +175,7 @@ class Geary.App.ConversationMonitorTest : TestCase { assert_non_null(monitor.window_lowest, "Lowest window id"); assert_equal(e2.id, monitor.window_lowest, "Lowest window id"); - Conversation c1 = Geary.Collection.get_first(monitor.read_only_view); + Conversation c1 = Collection.first(monitor.read_only_view); assert_equal(e1, c1.get_email_by_id(e1.id), "Related email not present in conversation"); assert_equal(e2, c1.get_email_by_id(e2.id), "In folder not present in conversation"); } @@ -329,7 +329,7 @@ class Geary.App.ConversationMonitorTest : TestCase { assert_int(1, monitor.size, "Conversation count"); - Conversation c1 = Geary.Collection.get_first(monitor.read_only_view); + Conversation c1 = Collection.first(monitor.read_only_view); assert_int(2, c1.get_count(), "Conversation message count"); assert_equal(e3, c1.get_email_by_id(e3.id), "Appended email not present in conversation"); diff --git a/test/engine/app/app-conversation-set-test.vala b/test/engine/app/app-conversation-set-test.vala index 29ac6512..1b3dc97b 100644 --- a/test/engine/app/app-conversation-set-test.vala +++ b/test/engine/app/app-conversation-set-test.vala @@ -116,7 +116,7 @@ class Geary.App.ConversationSetTest : TestCase { assert(this.test.get_email_count() == 1); assert(added.size == 1); - assert(Geary.Collection.get_first(added).get_email_by_id(e1.id) == e1); + assert(Collection.first(added).get_email_by_id(e1.id) == e1); assert(appended.size == 0); assert(removed.is_empty); @@ -165,7 +165,7 @@ class Geary.App.ConversationSetTest : TestCase { assert(added.size == 1); - Conversation convo1 = Geary.Collection.get_first(added); + Conversation convo1 = Collection.first(added); assert(convo1.get_email_by_id(e1.id) == e1); assert(convo1.get_email_by_id(e2.id) == e2); @@ -195,7 +195,7 @@ class Geary.App.ConversationSetTest : TestCase { assert(added.is_empty); assert(appended.size == 1); - Conversation convo2 = Geary.Collection.get_first(appended.get_keys()); + Conversation convo2 = Collection.first(appended.get_keys()); assert(convo2.get_email_by_id(e1.id) == e1); assert(convo2.get_email_by_id(e2.id) == e2); assert(convo2.get_email_by_id(e3.id) == e3); @@ -234,7 +234,7 @@ class Geary.App.ConversationSetTest : TestCase { assert(added.is_empty); assert(appended.size == 1); - Conversation convo = Geary.Collection.get_first(appended.get_keys()); + Conversation convo = Collection.first(appended.get_keys()); assert(convo.get_email_by_id(e1.id) == e1); assert(convo.get_email_by_id(e2.id) == e2); diff --git a/test/engine/common/common-contact-harvester-test.vala b/test/engine/common/common-contact-harvester-test.vala index 02bc73d0..e73ccedf 100644 --- a/test/engine/common/common-contact-harvester-test.vala +++ b/test/engine/common/common-contact-harvester-test.vala @@ -73,7 +73,7 @@ class Geary.ContactHarvesterImplTest : TestCase { Gee.Collection contacts = update_call.called_arg>(0); assert_int(1, contacts.size, "contacts length"); - Contact? created = Collection.get_first(contacts) as Contact; + Contact? created = Collection.first(contacts) as Contact; assert_non_null(created, "contacts contents"); assert_string("Test", created.real_name); @@ -121,7 +121,7 @@ class Geary.ContactHarvesterImplTest : TestCase { this.store.assert_expectations(); Gee.Collection contacts = update_call.called_arg>(0); - Contact? created = Collection.get_first(contacts) as Contact; + Contact? created = Collection.first(contacts) as Contact; assert_int( Contact.Importance.SEEN, created.highest_importance, @@ -150,7 +150,7 @@ class Geary.ContactHarvesterImplTest : TestCase { this.store.assert_expectations(); Gee.Collection contacts = update_call.called_arg>(0); - Contact? created = Collection.get_first(contacts) as Contact; + Contact? created = Collection.first(contacts) as Contact; assert_int( Contact.Importance.SENT_TO, created.highest_importance, @@ -179,7 +179,7 @@ class Geary.ContactHarvesterImplTest : TestCase { this.store.assert_expectations(); Gee.Collection contacts = update_call.called_arg>(0); - Contact? created = Collection.get_first(contacts) as Contact; + Contact? created = Collection.first(contacts) as Contact; assert_int( Contact.Importance.RECEIVED_FROM, created.highest_importance, diff --git a/test/engine/common/common-contact-store-impl-test.vala b/test/engine/common/common-contact-store-impl-test.vala index 3974c00f..2bc50dea 100644 --- a/test/engine/common/common-contact-store-impl-test.vala +++ b/test/engine/common/common-contact-store-impl-test.vala @@ -125,7 +125,7 @@ class Geary.ContactStoreImplTest : TestCase { ); assert_int(1, results.size, "results.size"); - Contact search_hit = Collection.get_first(results); + Contact search_hit = Collection.first(results); assert_string("Test@example.com", search_hit.email, "Existing email"); assert_string("test@example.com", search_hit.normalized_email, "Existing normalized_email"); assert_string("Test Name", search_hit.real_name, "Existing real_name"); @@ -146,7 +146,7 @@ class Geary.ContactStoreImplTest : TestCase { ); assert_int(1, results.size, "results.size"); - Contact search_hit = Collection.get_first(results); + Contact search_hit = Collection.first(results); assert_string("Test@example.com", search_hit.email, "Existing email"); assert_string("test@example.com", search_hit.normalized_email, "Existing normalized_email"); assert_string("Test Name", search_hit.real_name, "Existing real_name"); @@ -180,7 +180,7 @@ class Geary.ContactStoreImplTest : TestCase { ); assert_int(1, results.size, "results.size"); - Contact search_hit = Collection.get_first(results); + Contact search_hit = Collection.first(results); assert_string("Germán", search_hit.real_name, "Existing real_name"); } @@ -211,7 +211,7 @@ class Geary.ContactStoreImplTest : TestCase { ); assert_int(1, results.size, "results.size"); - Contact search_hit = Collection.get_first(results); + Contact search_hit = Collection.first(results); assert_string("年収1億円目指せ", search_hit.real_name, "Existing real_name"); } diff --git a/test/engine/rfc822-mailbox-addresses-test.vala b/test/engine/rfc822-mailbox-addresses-test.vala index 6c3a59b1..a9b2415e 100644 --- a/test/engine/rfc822-mailbox-addresses-test.vala +++ b/test/engine/rfc822-mailbox-addresses-test.vala @@ -12,6 +12,7 @@ class Geary.RFC822.MailboxAddressesTest : TestCase { add_test("from_rfc822_string_encoded", from_rfc822_string_encoded); add_test("from_rfc822_string_quoted", from_rfc822_string_quoted); add_test("to_rfc822_string", to_rfc822_string); + add_test("equal_to", equal_to); } public void from_rfc822_string_encoded() throws Error { @@ -49,6 +50,38 @@ class Geary.RFC822.MailboxAddressesTest : TestCase { .to_rfc822_string() == "test1@example.com, test2@example.com"); } + public void equal_to() throws Error { + var mailboxes_a = new_addreses({ "test1@example.com" }); + var mailboxes_b = new_addreses({ "test1@example.com" }); + var mailboxes_c = new_addreses({ "test2@example.com" }); + + assert_true(mailboxes_a.equal_to(mailboxes_a)); + assert_true(mailboxes_a.equal_to(mailboxes_b)); + assert_false(mailboxes_a.equal_to(mailboxes_c)); + + assert_true( + new_addreses({ "test1@example.com", "test2@example.com" }).equal_to( + new_addreses({ "test1@example.com", "test2@example.com" }) + ) + ); + assert_true( + new_addreses({ "test1@example.com", "test2@example.com" }).equal_to( + new_addreses({ "test2@example.com", "test1@example.com" }) + ) + ); + + assert_false( + new_addreses({ "test1@example.com", "test2@example.com" }).equal_to( + new_addreses({ "test1@example.com" }) + ) + ); + assert_false( + new_addreses({ "test1@example.com", "test2@example.com" }).equal_to( + new_addreses({ "test1@example.com", "test3@example.com" }) + ) + ); + } + private MailboxAddresses new_addreses(string[] address_strings) { Gee.List addresses = new Gee.LinkedList(); foreach (string address in address_strings) { From 2b15b49e4fdcb80e3fc8cb4f93fa7b9e11da1c89 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 14:34:37 +1100 Subject: [PATCH 5/6] Fix valac criticals at build time See GNOME/vala#835 --- src/client/application/application-client.vala | 2 +- src/client/application/application-main-window.vala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/application/application-client.vala b/src/client/application/application-client.vala index 88d32887..0e75cde9 100644 --- a/src/client/application/application-client.vala +++ b/src/client/application/application-client.vala @@ -813,7 +813,7 @@ public class Application.Client : Gtk.Application { // likely still loading folders after being // opened. Add a listener to try again later. try { - Geary.Account? first = Geary.Collection.first( + Geary.Account? first = Geary.Collection.first( this.engine.get_accounts() ); if (first != null) { diff --git a/src/client/application/application-main-window.vala b/src/client/application/application-main-window.vala index e163d1bc..0012727c 100644 --- a/src/client/application/application-main-window.vala +++ b/src/client/application/application-main-window.vala @@ -860,7 +860,7 @@ public class Application.MainWindow : private Geary.Folder? get_first_inbox() { Geary.Folder? inbox = null; try { - Geary.Account? first = Geary.Collection.first( + Geary.Account? first = Geary.Collection.first( this.application.engine.get_accounts() ); if (first != null) { From 32ee1b2b06ade1d60bfafea4c199639db408f2bc Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Tue, 19 Nov 2019 14:35:04 +1100 Subject: [PATCH 6/6] Remove unused method --- src/client/sidebar/sidebar-tree.vala | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/client/sidebar/sidebar-tree.vala b/src/client/sidebar/sidebar-tree.vala index fdcd570b..e01103ab 100644 --- a/src/client/sidebar/sidebar-tree.vala +++ b/src/client/sidebar/sidebar-tree.vala @@ -766,17 +766,6 @@ public class Sidebar.Tree : Gtk.TreeView { store.set(iter, Columns.ICON, icon); } - private void load_branch_icons(Gtk.TreeIter iter) { - load_entry_icons(iter); - - Gtk.TreeIter child_iter; - if (store.iter_children(out child_iter, iter)) { - do { - load_branch_icons(child_iter); - } while (store.iter_next(ref child_iter)); - } - } - private bool on_selection(Gtk.TreeSelection selection, Gtk.TreeModel model, Gtk.TreePath path, bool path_currently_selected) { // only allow selection if a page is selectable