Review Request 111315: Improved: Sort email addresses in main list by TLD first, then domain, and only last by account name

[Thomas Fischer]

> On June 29, 2013, 10:04 p.m., Rolf Eike Beer wrote:
> > model/keylistproxymodel.cpp, line 76
> > <http://git.reviewboard.kde.org/r/111315/diff/1/?file=166786#file166786line76>
> >
> >     Where does the www come from? That doesn't make sense. I think you should just remove the www from all following comments and everything is fine.

This is a left-over from older comments. A third-level domain is required to explain certain cases. Switched to "mail.kde.org" in all comments.


> On June 29, 2013, 10:04 p.m., Rolf Eike Beer wrote:
> > model/keylistproxymodel.cpp, lines 87-93
> > <http://git.reviewboard.kde.org/r/111315/diff/1/?file=166786#file166786line87>
> >
> >     I was just thinking about this:
> >     
> >     const int cnt = fqdncomponents.count();
> >     for (i = 0; i < cnt / 2; i++)
> >       fqdncomponents.swap(i, cnt - 1 - i);
> >     result = fqdncomponents.join(domainSplitDot).toLower();
> >     
> >     No hard preference in any directotion, take whatever algorithm you like better.

Using swap() violates the "const"ness of fqdnComponents. According to Qt's documentation, append() for QString has near-constant time complexity, thus keeping my solution.


> On June 29, 2013, 10:04 p.m., Rolf Eike Beer wrote:
> > model/keylistproxymodel.cpp, lines 116-119
> > <http://git.reviewboard.kde.org/r/111315/diff/1/?file=166786#file166786line116>
> >
> >     What about putting those "reverse some list and join with dots" into a file-static helper function instead of open-coding it three times?

I can use this function in two out of three cases.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111315/#review35293
-----------------------------------------------------------


On June 29, 2013, 7:26 p.m., Thomas Fischer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111315/
> -----------------------------------------------------------
> 
> (Updated June 29, 2013, 7:26 p.m.)
> 
> 
> Review request for KDE Utils.
> 
> 
> Description
> -------
> 
> Further improvements continuing the work started in review request 109847.
> 
> Addressing comments by Rolf Eike Beer (dakon):
> 
> > I wonder what happens to emails like foo.bar at example.com 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...
> 
> 
> Diffs
> -----
> 
>   model/keylistproxymodel.cpp cecb836 
> 
> Diff: http://git.reviewboard.kde.org/r/111315/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Thomas Fischer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20130701/d21285f0/attachment.html>