Hi,<br><br>thanks for the feedback.<br>-  I will fix the patch to follow the "coding style".<br>-  you are right, the tools tip is inversed ;-).<br><br>a new version of the patch will be posted soon.<br><br>Franck<br>

<br clear="all">WEB: <a href="http://3demi.net/photos/">http://3demi.net/photos/</a><br>
<br><br><div class="gmail_quote">On Tue, Apr 20, 2010 at 1:00 AM, Jan Kundrát <span dir="ltr"><<a href="mailto:jkt@gentoo.org">jkt@gentoo.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

<div class="im">Franck Sicard wrote:<br>
> here it is a new version of ma patch related to the matching of the<br>
> keyword into the annotation dialog.<br>
<br>
</div>Hi Franck, I haven't tried your patch yet, so I won't comment on the<br>
actual changes. What I will comment on is the coding style -- you should<br>
try to match the coding style to what we currently use in the codebase.<br>
Some specific points are inline.<br>
<br>
> +QString Utilities::removeAccent(const QString & str)<br>
<br>
We use another convention for the ampersand position, see almost any<br>
existing method.<br>
<br>
> +{<br>
> +  QString strWoAccent = str.normalized(QString::NormalizationForm_KD);<br>
<br>
We usually put spaces into parentheses if there's any passed argument.<br>
<br>
> +  while (i < strWoAccent.length())<br>
> +  {<br>
<br>
The curly brace should be on the previous line (basically that's the<br>
rule, with the only exception being the curly brace of the begin of the<br>
function body).<br>
<br>
Also, we indent with four spaces, please adjust.<br>
<br>
> +    if (strWoAccent[i].category() == QChar::Letter_Modifier)<br>
<br>
Spaces in the parentheses please, and again the { should not be on an<br>
extra line.<br>
<br>
> +    {<br>
> +      strWoAccent.remove(i, 1);<br>
> +    }<br>
> +    else<br>
> +    {<br>
<br>
"} else {"<br>
<br>
> +QString removeAccent(const QString & str);<br>
<br>
Already adressed.<br>
<br>
> +#include <Utilities/Util.h><br>
<br>
This is not a system header, so it should be "this" and not <this>.<br>
<br>
> -        return item->text(0).toLower().contains( _text.toLower() );<br>
> +        return Utilities::removeAccent(item->text(0).toLower()).contains( Utilities::removeAccent(_text.toLower()) );<br>
<br>
removeAccent(__SPACE__...<br>
<br>
> -    ListViewTextMatchHider dummy( text, true, _listView );<br>
> +  ListViewTextMatchHider dummy( text,<br>
> +                                !Settings::SettingsData::instance()->lazyKeywordMatching(),<br>
> +                                _listView );<br>
<br>
Indentation.<br>
<br>
> +    _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>") );<br>


<br>
This line is too long. Also, the meaning of "lazy" would imply exaclty<br>
the opposite of what your hint suggests. What is "tapping test"?<br>
<br>
Have fun,<br>
-jkt<br>
<font color="#888888"><br>
--<br>
cd /local/pub && more beer > /dev/mouth<br>
<br>
</font></blockquote></div><br>