Review Request 128724: [WIP] [KContacts] Allways save the upper case version of a custom key of a KContacts::Addressee

Bernhard Scheirle bernhard+kde at scheirle.de
Mon Aug 22 10:34:33 BST 2016



> On Aug. 22, 2016, 11:19 a.m., Daniel Vrátil wrote:
> > As you mentioned, the RFC dictates that keys should case insensitive, which means you should fix all the code that performs case-sensitive comparision instead of converting the keys to uppercase....

It is not possible to write case-insensitive code without modifying the Addressee functions. Most of the time others rely on Addressee::custom to get the right value.

Of cause instead of forcing uppercase we could do a more complex comparison in Addressee::custom via std::find_if (as you mentioned in https://git.reviewboard.kde.org/r/128723/)


- Bernhard


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128724/#review98544
-----------------------------------------------------------


On Aug. 20, 2016, 8:59 p.m., Bernhard Scheirle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128724/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2016, 8:59 p.m.)
> 
> 
> Review request for KDEPIM and Laurent Montel.
> 
> 
> Bugs: 317966
>     https://bugs.kde.org/show_bug.cgi?id=317966
> 
> 
> Repository: kcontacts
> 
> 
> Description
> -------
> 
> Allways save the upper case version of a custom key. This forces developers to not use case-sensitive keys.
> Case-sensitive keys are bad, because:
> 1. RFC 6350 explicitly says that keys are case-insensitive: https://tools.ietf.org/html/rfc6350#section-3.3
> 2. Ex- and importing contacts may change the case (e.g. if you use owncloud).
> 
> Of cause this change affects a few other applications:
> 
> * https://git.reviewboard.kde.org/r/128723
>     * akonadi-contacts/src/editor/generalinfoeditor/messaging/messagingwidgetlister.cpp
>     * akonadi-contacts/src/standardcontactformatter.cpp
>     * akonadi-contacts/src/editor/customfieldeditor/customfieldslistwidget.cpp
> 
> 
> * https://git.reviewboard.kde.org/r/128722
>     * kdepim-apps-libs/kaddressbookgrantlee/src/printing/contactgrantleeprintobject.cpp
>     * kdepim-apps-libs/kaddressbookgrantlee/src/formatter/grantleecontactformatter.cpp
> 
> 
> * (No review request yet)
>     * kdepim/kmail/src/kmreaderwin.cpp
> 
> 
> * (No review request yet)
>     * kopete/libkopete/kabcpersistence.cpp
> 
> 
> * (No review request yet)
>     * akonadi-google-applets/contacts/src/contactwidgetitem.cpp
> 
> 
> * (No review request yet)
>     * libkgapi/src/contacts/contactsservice.cpp
> 
> 
> * (No review request yet)
>     * messagelib/messageviewer/src/header/contactdisplaymessagememento.cpp
> 
> 
> Since I'm fairly new to KDE development I appreciate any feedback.
> 
> 
> Diffs
> -----
> 
>   src/addressee.cpp 86072c0da026e85fe5cd2a2115a2c47e0f31be74 
> 
> Diff: https://git.reviewboard.kde.org/r/128724/diff/
> 
> 
> Testing
> -------
> 
> ctest fails currently (4 test cases fail).
> I want to wait for your feedback before updating the test cases.
> 
> 
> Thanks,
> 
> Bernhard Scheirle
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20160822/d006c9fc/attachment.html>


More information about the kde-pim mailing list