[KPhotoAlbum] [PATCH] Lazy keword matching mode in annotation window

[Franck Sicard]

Hi,

thanks for the feedback.
-  I will fix the patch to follow the "coding style".
-  you are right, the tools tip is inversed ;-).

a new version of the patch will be posted soon.

Franck

WEB: http://3demi.net/photos/


On Tue, Apr 20, 2010 at 1:00 AM, Jan Kundrát <jkt at gentoo.org> wrote:

> Franck Sicard wrote:
> > here it is a new version of ma patch related to the matching of the
> > keyword into the annotation dialog.
>
> Hi Franck, I haven't tried your patch yet, so I won't comment on the
> actual changes. What I will comment on is the coding style -- you should
> try to match the coding style to what we currently use in the codebase.
> Some specific points are inline.
>
> > +QString Utilities::removeAccent(const QString & str)
>
> We use another convention for the ampersand position, see almost any
> existing method.
>
> > +{
> > +  QString strWoAccent = str.normalized(QString::NormalizationForm_KD);
>
> We usually put spaces into parentheses if there's any passed argument.
>
> > +  while (i < strWoAccent.length())
> > +  {
>
> The curly brace should be on the previous line (basically that's the
> rule, with the only exception being the curly brace of the begin of the
> function body).
>
> Also, we indent with four spaces, please adjust.
>
> > +    if (strWoAccent[i].category() == QChar::Letter_Modifier)
>
> Spaces in the parentheses please, and again the { should not be on an
> extra line.
>
> > +    {
> > +      strWoAccent.remove(i, 1);
> > +    }
> > +    else
> > +    {
>
> "} else {"
>
> > +QString removeAccent(const QString & str);
>
> Already adressed.
>
> > +#include <Utilities/Util.h>
>
> This is not a system header, so it should be "this" and not <this>.
>
> > -        return item->text(0).toLower().contains( _text.toLower() );
> > +        return
> Utilities::removeAccent(item->text(0).toLower()).contains(
> Utilities::removeAccent(_text.toLower()) );
>
> removeAccent(__SPACE__...
>
> > -    ListViewTextMatchHider dummy( text, true, _listView );
> > +  ListViewTextMatchHider dummy( text,
> > +
>  !Settings::SettingsData::instance()->lazyKeywordMatching(),
> > +                                _listView );
>
> Indentation.
>
> > +    _lazyKeywordMatching->setToolTip( i18n("<p>In the annotation window,
> when tapping text to filters existing entries, the patern will be search at
> the begining of the existing keywords and first exact matching instance will
> be selected.\nIf unchecked any keyword having the patern will match (ie:
> 'hor' will match 'animal:horse')<p>") );
>
> This line is too long. Also, the meaning of "lazy" would imply exaclty
> the opposite of what your hint suggests. What is "tapping test"?
>
> Have fun,
> -jkt
>
> --
> cd /local/pub && more beer > /dev/mouth
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20100420/806487d9/attachment.htm>