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

Jesper K. Pedersen blackie at kde.org
Tue Apr 20 09:46:26 BST 2010


Hi Frank, good to see you back.

On the concept of coding style, the by most important one is the indentation - 
my emacs will reindent automatically, and then give a lot of lines changed.
Personally, I think the other things are minor things, though of course if it 
doesn't matter much to you where you put say spaces, then following the rest 
of the code makes it much easier to keep tidy.

I have exams from on to till June 19th, so I'm afraid there wont be more 
feedback from me till then, so please work with the other contributers to get 
this in shape. If all else fails, please ping me after June 19th to get you 
over the finishing line.

Cheers

On Tuesday 20 April 2010 10:26:05 Franck Sicard wrote:
| 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
| 

-- 
Having trouble finding a given image in your collection containing
thousands of images?

http://www.kphotoalbum.org might be the answer.



More information about the Kphotoalbum mailing list