From fff39e8a44e5ea3b0ef51728596dcbfc8a55e083 Mon Sep 17 00:00:00 2001 From: Jim Nelson Date: Fri, 1 Jul 2011 19:33:35 -0700 Subject: [PATCH] Move special folders to the top of the tree, put other folders in container, and start in Inbox: #3707 and #3692 This commit introduces the idea of a Personality, customizations for particular IMAP servers and services to allow the interface to configure itself for more natural use. Also, this commit has the app start in the Inbox, and an optimization was added that makes showing what's in the Inbox (at least, what's in the local cache) come up instantly. --- src/client/ui/folder-list-store.vala | 104 ++++++++++++++++-- src/client/ui/folder-list-view.vala | 11 ++ src/client/ui/main-window.vala | 38 ++++++- src/engine/api/geary-engine-account.vala | 8 +- src/engine/api/geary-engine-folder.vala | 9 +- src/engine/api/geary-engine.vala | 2 +- .../api/geary-generic-imap-account.vala | 27 ++++- src/engine/api/geary-gmail-account.vala | 56 ++++++++++ src/engine/api/geary-personality.vala | 14 +++ src/engine/api/geary-special-folder.vala | 60 ++++++++++ src/engine/imap/api/imap-account.vala | 62 +++++++++-- src/engine/imap/imap-error.vala | 3 +- src/wscript | 3 + 13 files changed, 363 insertions(+), 34 deletions(-) create mode 100644 src/engine/api/geary-gmail-account.vala create mode 100644 src/engine/api/geary-personality.vala create mode 100644 src/engine/api/geary-special-folder.vala diff --git a/src/client/ui/folder-list-store.vala b/src/client/ui/folder-list-store.vala index 960119d4..1212d0d7 100644 --- a/src/client/ui/folder-list-store.vala +++ b/src/client/ui/folder-list-store.vala @@ -12,9 +12,17 @@ public enum TreeSortable { } public class FolderListStore : Gtk.TreeStore { + private enum Internal { + SPECIAL_FOLDER, + USER_FOLDER_ROOT, + USER_FOLDER + } + public enum Column { NAME, FOLDER_OBJECT, + INTERNAL, + ORDERING, N_COLUMNS; public static Column[] all() { @@ -27,7 +35,9 @@ public class FolderListStore : Gtk.TreeStore { public static Type[] get_types() { return { typeof (string), - typeof (Geary.Folder) + typeof (Geary.Folder), + typeof (Internal), + typeof (int) }; } @@ -36,11 +46,8 @@ public class FolderListStore : Gtk.TreeStore { case NAME: return _("Name"); - case FOLDER_OBJECT: - return "(hidden)"; - default: - assert_not_reached(); + return "(hidden)"; } } } @@ -49,19 +56,55 @@ public class FolderListStore : Gtk.TreeStore { set_column_types(Column.get_types()); set_default_sort_func(sort_by_name); set_sort_column_id(TreeSortable.DEFAULT_SORT_COLUMN_ID, Gtk.SortType.ASCENDING); + + // add user folder root + Gtk.TreeIter iter; + append(out iter, null); + + set(iter, + Column.NAME, _("Folders"), + Column.INTERNAL, Internal.USER_FOLDER_ROOT + ); } - public void add_folder(Geary.Folder folder) { + public void set_user_folders_root_name(string name) { + Gtk.TreeIter? iter = get_user_folders_iter(); + if (iter == null) + return; + + set(iter, + Column.NAME, name + ); + } + + public void add_special_folder(Geary.SpecialFolder special, Geary.Folder folder) { + Gtk.TreeIter iter; + append(out iter, null); + + set(iter, + Column.NAME, special.name, + Column.FOLDER_OBJECT, folder, + Column.INTERNAL, Internal.SPECIAL_FOLDER, + Column.ORDERING, special.ordering + ); + } + + public void add_user_folder(Geary.Folder folder) { + Gtk.TreeIter? user_folders_root_iter = get_user_folders_iter(); + if (user_folders_root_iter == null) + return; + Gtk.TreeIter? parent_iter = !folder.get_path().is_root() - ? find_path(folder.get_path().get_parent()) - : null; + ? find_path(folder.get_path().get_parent(), user_folders_root_iter) + : user_folders_root_iter; Gtk.TreeIter iter; append(out iter, parent_iter); set(iter, Column.NAME, folder.get_path().basename, - Column.FOLDER_OBJECT, folder + Column.FOLDER_OBJECT, folder, + Column.INTERNAL, Internal.USER_FOLDER ); } @@ -76,8 +119,25 @@ public class FolderListStore : Gtk.TreeStore { return folder; } + private Gtk.TreeIter? get_user_folders_iter() { + Gtk.TreeIter iter; + get_iter_first(out iter); + + do { + Internal internl; + get(iter, Column.INTERNAL, out internl); + + if (internl == Internal.USER_FOLDER_ROOT) + return iter; + } while (iter_next(ref iter)); + + debug("Unable to locate user folders root"); + + return null; + } + // TODO: This could be replaced with a binary search - private Gtk.TreeIter? find_path(Geary.FolderPath path, Gtk.TreeIter? parent = null) { + public Gtk.TreeIter? find_path(Geary.FolderPath path, Gtk.TreeIter? parent = null) { Gtk.TreeIter iter; // no parent, start at the root, otherwise start at the parent's children if (parent == null) { @@ -107,6 +167,30 @@ public class FolderListStore : Gtk.TreeStore { } private int sort_by_name(Gtk.TreeModel model, Gtk.TreeIter aiter, Gtk.TreeIter biter) { + Internal ainternal; + model.get(aiter, Column.INTERNAL, out ainternal); + + Internal binternal; + model.get(biter, Column.INTERNAL, out binternal); + + // sort special folders in their own magical way + if (ainternal == Internal.SPECIAL_FOLDER && binternal == Internal.SPECIAL_FOLDER) { + int apos; + model.get(aiter, Column.ORDERING, out apos); + + int bpos; + model.get(biter, Column.ORDERING, out bpos); + + return apos - bpos; + } + + // sort the USER_FOLDER_ROOT dead last + if (ainternal == Internal.USER_FOLDER_ROOT) + return 1; + else if (binternal == Internal.USER_FOLDER_ROOT) + return -1; + + // sort everything else by name string aname; model.get(aiter, Column.NAME, out aname); diff --git a/src/client/ui/folder-list-view.vala b/src/client/ui/folder-list-view.vala index 17ca605d..2b36be0f 100644 --- a/src/client/ui/folder-list-view.vala +++ b/src/client/ui/folder-list-view.vala @@ -22,6 +22,17 @@ public class FolderListView : Gtk.TreeView { return (FolderListStore) get_model(); } + public void select_path(Geary.FolderPath path) { + Gtk.TreeIter? iter = get_store().find_path(path); + if (iter == null) + return; + + Gtk.TreePath tree_path = get_store().get_path(iter); + + get_selection().select_path(tree_path); + set_cursor(tree_path, null, false); + } + private void on_selection_changed() { Gtk.TreeModel model; Gtk.TreePath? path = get_selection().get_selected_rows(out model).nth_data(0); diff --git a/src/client/ui/main-window.vala b/src/client/ui/main-window.vala index 62856066..29183107 100644 --- a/src/client/ui/main-window.vala +++ b/src/client/ui/main-window.vala @@ -65,12 +65,28 @@ public class MainWindow : Gtk.Window { this.account = account; account.folders_added_removed.connect(on_folders_added_removed); + folder_list_store.set_user_folders_root_name(account.get_user_folders_label()); + do_start.begin(); } private async void do_start() { try { - // pull down the root-level folders + // add all the special folders, which are assumed to always exist + Geary.SpecialFolderMap? special_folders = account.get_special_folder_map(); + if (special_folders != null) { + foreach (Geary.SpecialFolder special_folder in special_folders.get_all()) { + Geary.Folder folder = yield account.fetch_folder_async(special_folder.path); + folder_list_store.add_special_folder(special_folder, folder); + } + + // If inbox is specified, select that + Geary.SpecialFolder? inbox = special_folders.get_folder(Geary.SpecialFolderType.INBOX); + if (inbox != null) + folder_list_view.select_path(inbox.path); + } + + // pull down the root-level user folders Gee.Collection folders = yield account.list_folders_async(null); if (folders != null) on_folders_added_removed(folders, null); @@ -253,10 +269,24 @@ public class MainWindow : Gtk.Window { private void on_folders_added_removed(Gee.Collection? added, Gee.Collection? removed) { if (added != null && added.size > 0) { - foreach (Geary.Folder folder in added) - folder_list_store.add_folder(folder); + Gee.Set? ignored_paths = account.get_ignored_paths(); - search_folders_for_children.begin(added); + Gee.ArrayList skipped = new Gee.ArrayList(); + foreach (Geary.Folder folder in added) { + if (ignored_paths != null && ignored_paths.contains(folder.get_path())) + skipped.add(folder); + else + folder_list_store.add_user_folder(folder); + } + + Gee.Collection remaining = added; + if (skipped.size > 0) { + remaining = new Gee.ArrayList(); + remaining.add_all(added); + remaining.remove_all(skipped); + } + + search_folders_for_children.begin(remaining); } } diff --git a/src/engine/api/geary-engine-account.vala b/src/engine/api/geary-engine-account.vala index b60916d8..ddb85105 100644 --- a/src/engine/api/geary-engine-account.vala +++ b/src/engine/api/geary-engine-account.vala @@ -4,9 +4,15 @@ * (version 2.1 or later). See the COPYING file in this distribution. */ -public abstract class Geary.EngineAccount : Geary.AbstractAccount { +public abstract class Geary.EngineAccount : Geary.AbstractAccount, Geary.Personality { public EngineAccount(string name) { base (name); } + + public abstract string get_user_folders_label(); + + public abstract Geary.SpecialFolderMap? get_special_folder_map(); + + public abstract Gee.Set? get_ignored_paths(); } diff --git a/src/engine/api/geary-engine-folder.vala b/src/engine/api/geary-engine-folder.vala index e34f36c7..827469ba 100644 --- a/src/engine/api/geary-engine-folder.vala +++ b/src/engine/api/geary-engine-folder.vala @@ -56,17 +56,16 @@ private class Geary.EngineFolder : Geary.AbstractFolder { // wait_for_remote_to_open(), which uses a NonblockingSemaphore to indicate that the remote // is open (or has failed to open). This allows for early calls to list and fetch emails // can work out of the local cache until the remote is ready. - RemoteFolder folder = (RemoteFolder) yield remote.fetch_folder_async(local_folder.get_path(), - cancellable); - open_remote_async.begin(folder, readonly, cancellable, on_open_remote_completed); + open_remote_async.begin(readonly, cancellable, on_open_remote_completed); opened = true; notify_opened(); } - private async void open_remote_async(RemoteFolder folder, bool readonly, Cancellable? cancellable) - throws Error { + private async void open_remote_async(bool readonly, Cancellable? cancellable) throws Error { + RemoteFolder folder = (RemoteFolder) yield remote.fetch_folder_async(local_folder.get_path(), + cancellable); yield folder.open_async(readonly, cancellable); remote_folder = folder; diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index 06797b35..a17e0502 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -15,7 +15,7 @@ public class Geary.Engine { } // Only Gmail today - return new GenericImapAccount( + return new GmailAccount( "Gmail account %s".printf(cred.to_string()), new Geary.Imap.Account(cred, Imap.ClientConnection.DEFAULT_PORT_TLS), new Geary.Sqlite.Account(cred)); diff --git a/src/engine/api/geary-generic-imap-account.vala b/src/engine/api/geary-generic-imap-account.vala index 796b7322..f977f8d7 100644 --- a/src/engine/api/geary-generic-imap-account.vala +++ b/src/engine/api/geary-generic-imap-account.vala @@ -5,7 +5,7 @@ */ private class Geary.GenericImapAccount : Geary.EngineAccount { - public const string INBOX = "Inbox"; + private SpecialFolderMap? special_folders = null; private RemoteAccount remote; private LocalAccount local; @@ -15,6 +15,19 @@ private class Geary.GenericImapAccount : Geary.EngineAccount { this.remote = remote; this.local = local; + + if (special_folders == null) { + special_folders = new SpecialFolderMap(); + + special_folders.set_folder( + new SpecialFolder( + SpecialFolderType.INBOX, + _("Inbox"), + new Geary.FolderRoot(Imap.Account.INBOX_NAME, Imap.Account.ASSUMED_SEPARATOR, false), + 0 + ) + ); + } } public override Geary.Email.Field get_required_fields_for_writing() { @@ -117,5 +130,17 @@ private class Geary.GenericImapAccount : Geary.EngineAccount { if (engine_added != null) notify_folders_added_removed(engine_added, null); } + + public override string get_user_folders_label() { + return _("Folders"); + } + + public override Geary.SpecialFolderMap? get_special_folder_map() { + return special_folders; + } + + public override Gee.Set? get_ignored_paths() { + return null; + } } diff --git a/src/engine/api/geary-gmail-account.vala b/src/engine/api/geary-gmail-account.vala new file mode 100644 index 00000000..492a90d2 --- /dev/null +++ b/src/engine/api/geary-gmail-account.vala @@ -0,0 +1,56 @@ +/* Copyright 2011 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +private class Geary.GmailAccount : Geary.GenericImapAccount { + private const string GMAIL_FOLDER = "[Gmail]"; + + private static SpecialFolderMap? special_folder_map = null; + private static Gee.Set? ignored_paths = null; + + public GmailAccount(string name, RemoteAccount remote, LocalAccount local) { + base (name, remote, local); + + if (special_folder_map == null || ignored_paths == null) + initialize_personality(); + } + + private static void initialize_personality() { + Geary.FolderPath gmail_root = new Geary.FolderRoot(GMAIL_FOLDER, Imap.Account.ASSUMED_SEPARATOR, + true); + + special_folder_map = new SpecialFolderMap(); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.INBOX, _("Inbox"), + new Geary.FolderRoot(Imap.Account.INBOX_NAME, Imap.Account.ASSUMED_SEPARATOR, false), 0)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.DRAFTS, _("Drafts"), + gmail_root.get_child("Drafts"), 1)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.SENT, _("Sent Mail"), + gmail_root.get_child("Sent Mail"), 2)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.FLAGGED, _("Starred"), + gmail_root.get_child("Starred"), 3)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.ALL_MAIL, _("All Mail"), + gmail_root.get_child("All Mail"), 4)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.SPAM, _("Spam"), + gmail_root.get_child("Spam"), 5)); + special_folder_map.set_folder(new SpecialFolder(Geary.SpecialFolderType.TRASH, _("Trash"), + gmail_root.get_child("Trash"), 6)); + + ignored_paths = new Gee.HashSet(Hashable.hash_func, Equalable.equal_func); + ignored_paths.add(gmail_root); + } + + public override string get_user_folders_label() { + return _("Labels"); + } + + public override Geary.SpecialFolderMap? get_special_folder_map() { + return special_folder_map; + } + + public override Gee.Set? get_ignored_paths() { + return ignored_paths.read_only_view; + } +} + diff --git a/src/engine/api/geary-personality.vala b/src/engine/api/geary-personality.vala new file mode 100644 index 00000000..c387b1d9 --- /dev/null +++ b/src/engine/api/geary-personality.vala @@ -0,0 +1,14 @@ +/* Copyright 2011 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +public interface Geary.Personality : Object { + public abstract string get_user_folders_label(); + + public abstract Geary.SpecialFolderMap? get_special_folder_map(); + + public abstract Gee.Set? get_ignored_paths(); +} + diff --git a/src/engine/api/geary-special-folder.vala b/src/engine/api/geary-special-folder.vala new file mode 100644 index 00000000..aeb30706 --- /dev/null +++ b/src/engine/api/geary-special-folder.vala @@ -0,0 +1,60 @@ +/* Copyright 2011 Yorba Foundation + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +public enum Geary.SpecialFolderType { + INBOX, + DRAFTS, + SENT, + FLAGGED, + ALL_MAIL, + SPAM, + TRASH +} + +public class Geary.SpecialFolder : Object { + public SpecialFolderType folder_type { get; private set; } + public string name { get; private set; } + public Geary.FolderPath path { get; private set; } + public int ordering { get; private set; } + + public SpecialFolder(SpecialFolderType folder_type, string name, FolderPath path, int ordering) { + this.folder_type = folder_type; + this.name = name; + this.path = path; + this.ordering = ordering; + } +} + +public class Geary.SpecialFolderMap : Object { + private Gee.HashMap map = new Gee.HashMap(); + private Gee.HashSet paths = new Gee.HashSet( + Hashable.hash_func, Equalable.equal_func); + + public SpecialFolderMap() { + } + + public void set_folder(SpecialFolder special_folder) { + map.set(special_folder.folder_type, special_folder); + } + + public SpecialFolder? get_folder(SpecialFolderType folder_type) { + return map.get(folder_type); + } + + public Gee.Set get_supported_types() { + return map.keys.read_only_view; + } + + public Gee.Collection get_all() { + return map.values.read_only_view; + } + + public bool has_path(Geary.FolderPath path) { + return paths.contains(path); + } +} + diff --git a/src/engine/imap/api/imap-account.vala b/src/engine/imap/api/imap-account.vala index 63d568e5..36d835af 100644 --- a/src/engine/imap/api/imap-account.vala +++ b/src/engine/imap/api/imap-account.vala @@ -5,6 +5,11 @@ */ public class Geary.Imap.Account : Geary.AbstractAccount, Geary.RemoteAccount { + // all references to Inbox are converted to this string, purely for sanity sake when dealing + // with Inbox's case issues + public const string INBOX_NAME = "INBOX"; + public const string ASSUMED_SEPARATOR = "/"; + private ClientSessionManager session_mgr; private Gee.HashMap delims = new Gee.HashMap(); @@ -36,11 +41,14 @@ public class Geary.Imap.Account : Geary.AbstractAccount, Geary.RemoteAccount { public override async Gee.Collection list_folders_async(Geary.FolderPath? parent, Cancellable? cancellable = null) throws Error { + Geary.FolderPath? processed = process_path(parent, null, + (parent != null) ? parent.get_root().default_separator : ASSUMED_SEPARATOR); + Gee.Collection mboxes; try { - mboxes = (parent == null) + mboxes = (processed == null) ? yield session_mgr.list_roots(cancellable) - : yield session_mgr.list(parent.get_fullpath(), parent.get_root().default_separator, + : yield session_mgr.list(processed.get_fullpath(), processed.get_root().default_separator, cancellable); } catch (Error err) { if (err is ImapError.SERVER_ERROR) @@ -51,14 +59,12 @@ public class Geary.Imap.Account : Geary.AbstractAccount, Geary.RemoteAccount { Gee.Collection folders = new Gee.ArrayList(); foreach (MailboxInformation mbox in mboxes) { - if (parent == null) - delims.set(mbox.name, mbox.delim); + Geary.FolderPath path = process_path(processed, mbox.name, mbox.delim); - string basename = mbox.get_path().last(); - - Geary.FolderPath path = (parent != null) - ? parent.get_child(basename) - : new Geary.FolderRoot(basename, mbox.delim, Folder.CASE_SENSITIVE); + // only add to delims map if root-level folder (all sub-folders respect its delimiter) + // also use the processed name, not the one reported off the wire + if (processed == null) + delims.set(path.get_root().basename, mbox.delim); folders.add(new Geary.Imap.Folder(session_mgr, path, mbox)); } @@ -68,12 +74,17 @@ public class Geary.Imap.Account : Geary.AbstractAccount, Geary.RemoteAccount { public override async Geary.Folder fetch_folder_async(Geary.FolderPath path, Cancellable? cancellable = null) throws Error { + Geary.FolderPath? processed = process_path(path, null, path.get_root().default_separator); + if (processed == null) + throw new ImapError.INVALID_PATH("Invalid path %s", path.to_string()); + try { - MailboxInformation? mbox = yield session_mgr.fetch_async(path.get_fullpath(), cancellable); + MailboxInformation? mbox = yield session_mgr.fetch_async(processed.get_fullpath(), + cancellable); if (mbox == null) throw_not_found(path); - return new Geary.Imap.Folder(session_mgr, path, mbox); + return new Geary.Imap.Folder(session_mgr, processed, mbox); } catch (ImapError err) { if (err is ImapError.SERVER_ERROR) throw_not_found(path); @@ -87,4 +98,33 @@ public class Geary.Imap.Account : Geary.AbstractAccount, Geary.RemoteAccount { throw new EngineError.NOT_FOUND("Folder %s not found on %s", (path != null) ? path.to_string() : "root", session_mgr.to_string()); } + + // This method ensures that Inbox is dealt with in a consistent fashion throughout the + // application. + private static Geary.FolderPath? process_path(Geary.FolderPath? parent, string? basename, + string? delim) throws ImapError { + // 1. Both null, done + if (parent == null && basename == null) + return null; + + // 2. Parent null but basename not, create FolderRoot for Inbox + if (parent == null && basename != null && basename.up() == INBOX_NAME) + return new Geary.FolderRoot(INBOX_NAME, delim, false); + + // 3. Parent and basename supplied, verify parent is not Inbox, as IMAP does not allow it + // to have children + if (parent != null && basename != null && parent.get_root().basename.up() == INBOX_NAME) + throw new ImapError.INVALID_PATH("Inbox may not have children"); + + // 4. Default behavior: create child of basename or basename as root, otherwise return parent + // unmodified + if (parent != null && basename != null) + return parent.get_child(basename); + + if (basename != null) + return new Geary.FolderRoot(basename, delim, Folder.CASE_SENSITIVE); + + return parent; + } } + diff --git a/src/engine/imap/imap-error.vala b/src/engine/imap/imap-error.vala index 7b60d626..4c984ed0 100644 --- a/src/engine/imap/imap-error.vala +++ b/src/engine/imap/imap-error.vala @@ -12,6 +12,7 @@ public errordomain Geary.ImapError { COMMAND_FAILED, UNAUTHENTICATED, NOT_SUPPORTED, - NOT_SELECTED + NOT_SELECTED, + INVALID_PATH } diff --git a/src/wscript b/src/wscript index ba607477..47e76652 100644 --- a/src/wscript +++ b/src/wscript @@ -29,8 +29,11 @@ def build(bld): '../engine/api/geary-folder-properties.vala', '../engine/api/geary-folder.vala', '../engine/api/geary-generic-imap-account.vala', + '../engine/api/geary-gmail-account.vala', '../engine/api/geary-local-interfaces.vala', + '../engine/api/geary-personality.vala', '../engine/api/geary-remote-interfaces.vala', + '../engine/api/geary-special-folder.vala', '../engine/common/common-interfaces.vala', '../engine/common/common-message-data.vala',