Sort email addresses in main list by TLD first, then domain, and only last by account name
Review Request #109847 - Created April 3, 2013 and updated
Sort email addresses in main list by TLD first, then domain, and only last by account name. (1) email address is split along @ and . (2a) list is reversed (2b) list elements are joined, two elements separated by .
Looks good, but I have 2 things. First, this should be configurable, so please also add an option somewhere in the Appearance section to make that configurable. I'm not sure if this should be only 2 options, i.e. old behavior and your one, or if we may need more options here. The other 2 valid options I could think of would be "sort by second level domain first" and "sort by whole domain". I'm not sure about those 2.
Looks like you have forgotten to add at least one file to the diff: kgpg/model/keylistproxymodel.cpp: In member function ‘QString KeyListProxyModelPrivate::reorderEmailComponents(const QString&) const’: kgpg/model/keylistproxymodel.cpp:68:10: error: ‘emailSorting’ is not a member of ‘KGpgSettings’ Also from a first look I would say you should simplify the email regexes. We don't really care about the validity of the addresses in the keys here at all. So simply split them at @ and . as said in the description, and ignore whatever other characters are in between that. So if the email address is foo@example or email@example.com that's fine for sorting here.
Review request changed
> I wonder what happens to emails like firstname.lastname@example.org with TLDfirst, the > regex will split them into foo, bar, example, com AFAICT, which would screw > up sorting, no? I rewrote this case, should work better now, too. The patch contains some minor rewrites for the other cases as well plus some unifications in the white spaces. > For an unknown reason the items added in the ui file are ignored, they only > appear when added again in C++. No idea either. > When the setting is changed and Accept or OK is clicked keysmanager will not > refresh it's display. You should probably call something like invalidateFilter > on the filter model used for the main view. Observed as well. How do I trigger such a refresh/invalidation? The settings/options class do no see the model (or the view) directly as far as I can see...
Revision 5 (+39 -25)