GNOME Bugzilla – Bug 708353
Enhance folder size dialog
Last modified: 2013-11-11 20:37:49 UTC
Fidencio, once you have some time, would you mind to enhance the folder-size dialog? The one from Account Properties->EWS Settings->Folder Sizes. I would like to see there a folder structure, the same as is in the folder tree (including folder icons), and beside it the current folder size, like it is now (aka in the second column). That way it'll be easier to navigate through the folders. (The tree might be expanded by default.) It would be nice to also sort by the size column, though once the view will be in a folder structure, it'll be partly useless (unless turning on sort by folder size would change the view to flat, with full paths on the left (still with folder icons) and actual size on the right).
Created attachment 258963 [details] [review] EEwsFolder: As getters don't modify the internal data, use const EEwsFolder * in e_ews_folder_get.* functions
Created attachment 258964 [details] [review] Bug #708353 - Enhance folder size dialog
Created attachment 258965 [details] [review] Bug #708353 - Enhance folder size dialog
Review of attachment 258963 [details] [review]: Looks fine, feel free to commit to master. Thanks.
Review of attachment 258965 [details] [review]: I also noticed that: a) the Drafts folder is missing the special icon b) folder order differs due to different ordering/compare function (I have folder which begins with 'b' and a folder which begins with 'D' and the mail view's folder tree shows 'b' before 'D', while the folder size's tree shows 'D' before 'b'. ::: src/configuration/e-ews-config-utils.c @@ +518,3 @@ + * Populate model with data + * + * Yeah, it's clearly sub-optimal. It's awfully awfully awfully awfully awfully awfully suboptimal... awfully... :) @@ +548,3 @@ + fiter = fsd->folder_list; + + while (g_slist_length (fsd->folder_list) > 0) { let's see... you may have 50 folders. In each iteration of the cycle you count the items, which means to walk through all the items, thus 50 jumps, next cycle 49 jumps, ...and so on down to zero, while the only thing you want is fsd->folder_list != NULL. (Nonetheless, see below) @@ +680,3 @@ fsd->cancellable, &fsd->error); + fsd->folder_list = g_slist_sort (fsd->folder_list, compare_folder_names); you have always available CamelEwsStore, thus ask it for the folder tree and needed folders, by camel_store_get_folder_info_sync(), where you get whole folder hierarchy already as a tree.
Created attachment 259398 [details] [review] Bug #708353 - Enhance folder size dialog
Review of attachment 259398 [details] [review]: the ordering of folders is still wrong, use g_utf8_collate() as a sort function for folders on one level (the exact function from EMFolderTree might not be available, I'm afraid. It would be also interesting to see sizes for calendars/books/... basically any non-email folder there, but I know it's not returned by the Mail part, neither is there in current code. Thus just fix the sorting and the draft folder and that's it. ::: src/configuration/e-ews-config-utils.c @@ +454,3 @@ + const gchar *folder_size; + + if (g_strcmp0 (folder_info->full_name, "Drafts") == 0) this doesn't work, if you've setup a different localization on the server (as localized by the server, not by Gnome localization team). Maybe consider traversing the EMFolderTree and getting icon for the folders from there, to stay consistent.
(In reply to comment #7) > ::: src/configuration/e-ews-config-utils.c > @@ +454,3 @@ > + const gchar *folder_size; > + > + if (g_strcmp0 (folder_info->full_name, "Drafts") == 0) > > this doesn't work, if you've setup a different localization on the server (as > localized by the server, not by Gnome localization team). Maybe consider > traversing the EMFolderTree and getting icon for the folders from there, to > stay consistent. It's exactly what is being done by EMFolderTree, no? https://git.gnome.org/browse/evolution/tree/mail/em-folder-tree-model.c#n829
Created attachment 259570 [details] [review] Bug #708353 - Enhance folder size dialog
(In reply to comment #8) > It's exactly what is being done by EMFolderTree, no? > https://git.gnome.org/browse/evolution/tree/mail/em-folder-tree-model.c#n829 Nope, because the compare to "Drafts" is done only for local folders: https://git.gnome.org/browse/evolution/tree/mail/em-folder-tree-model.c#n776 which is basically those in On This Computer, over which evolution has control. See how the 'folder_is_drafts' variable is populated elsewhere in this file.
Review of attachment 259570 [details] [review]: A, I see, you've found it, good. Pity to do it for each folder in the list, while it can be on the level 0 only, but this is surely the best we can do and keep generality of the code, thus I'm fine with this. Please commit to master. Thanks.
Attachment 258963 [details] pushed as a1cb5ae (evolution-ews 3.11.2+) - EEwsFolder: As getters don't modify the internal data, use const EEwsFolder * in e_ews_folder_get.* functions Attachment 259570 [details] pushed as 5f6ee76 (evolution-ews 3.11.2+) - Bug #708353 - Enhance folder size dialog