D6516: Fix selecting the color from a set of filters

[Andre Heinecke]

aheinecke requested changes to this revision.
aheinecke added a comment.
This revision now requires changes to proceed.


  Looks good in general to me apart from my inline comments. The filter stuff I will test now because I can't see from the code if it works as intended.
  
  Regarding style: Please replace all tabs with 4 spaces. And we don't have a space between "!" and the statement.
  
  The Style is documented under:
  https://techbase.kde.org/Policies/Kdelibs_Coding_Style
  
  It even has an Emacs Mode for KDE ;-P

INLINE COMMENTS

> justusw wrote in formatting.cpp:825
> So obviously, a predicate is not about formatting, so it shouldn't reside here.  I thought about adding a module, e.g. 'compliance' or something, but I did not feel confident enough to do so and decide how to name it and where to put it.
> 
> Guidance appreciated.

I think it's ok to have it in Formatting: isKeyValid -> Should we display this as a valid key? Is kind of formatting for me.

A better place might be GpgMEpp directly, we might add it there but still add it here to avoid a new round of release / dependency bump.

I'd like you to rename the function though to something like "uidsHaveFullValidity" to avoid confusion because "valid" in other contexts is used as some kind of "is key usable" (e.g. not expired, not revoked, etc.)" and the use of just "valid" tends to be confusing.

> formatting.cpp:829
> +    for (const auto &uid: key.userIDs()) {
> +	if (uid.validity() >= UserID::Validity::Full) {
> +	    fullyTrusted++;

I think it's ok for a key not to have any user id. In that case this function would return true.
Also I think we need to skip revoked or expired UserID's here similar to isDeVs

As a minor nitpick I would just return false once a uid.validity() < UserID::Validity::Full is found.

> formatting.cpp:885
> +{
> +    unsigned int fullyTrusted = 0;
> +    for (const auto &uid: key.userIDs()) {

I think we can use the now called isKeyValid function for this.

REPOSITORY
  R90 PIM: Kleo Library

REVISION DETAIL
  https://phabricator.kde.org/D6516

To: justusw, aheinecke
Cc: #kde_pim, dvasin, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20170706/00c59f06/attachment.html>