> On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/urlbar/webshortcutwidget.h, line 2 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4850#file4850line2> > > > > Copy paste? Isn't your name supposed to be here?
This class come from konqueror's searchbar plug-in. Fredy Yanardi is the author. I modified the class, so I should add my name too. > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/urlbar/listitem.cpp, lines 636-639 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4846#file4846line636> > > > > Something does not seems right here. > > > > if (item.type & UrlSearchItem::Suggestion) { > > [...] > > if (item.type & UrlSearchItem::Suggestion) { > > > > The second if is always true. You added something in the else, I don't > > think it is ever reached. true. it is a bad merge resolution. > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/opensearch/opensearchengine.cpp, line 61 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4839#file4839line61> > > > > This line is hard to read to me. > > I would do that in 2 step: > > 1) remove the first dash: > > const int firstDashPosition = country.indexOf(QL1C('-')); > > if (firstDash >= 0) > > country.remove(firstDashPosition, 1) > > > > 2) lower the string > > country = country.lower() > > > > It is a bit longuer, but clearer IMHO. ok > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/opensearch/opensearchmanager.cpp, lines 241-243 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4841#file4841line241> > > > > QFile is a bit overkill just to find if a file exists. QFile is a > > QIODevice->QObject. > > QFileInfo::exists() should do the job without so much overhead. > > > > There are also QL1S() missing I think. ok I must study that. > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/opensearch/opensearchmanager.cpp, line 217 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4841#file4841line217> > > > > Please put some explanation in the TODO, like > > > > // TODO: handle charset of the provider. > > > > > > This limits the chance of having the TODO out of sync with the code. > > ok > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/urlbar/webshortcutwidget.h, line 33 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4850#file4850line33> > > > > This constructor should be explicit. ok > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/urlbar/webshortcutwidget.h, line 35 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4850#file4850line35> > > > > Space between & and pos for the third argument. ok > On 2010-12-08 18:17:58, Benjamin Poulain wrote: > > src/webtab.h, line 86 > > <http://git.reviewboard.kde.org/r/100064/diff/4/?file=4852#file4852line86> > > > > "const QPoint &" for the argument. ok - Lionel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100064/#review468 ----------------------------------------------------------- On 2010-12-08 08:20:34, Lionel Chauvin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100064/ > ----------------------------------------------------------- > > (Updated 2010-12-08 08:20:34) > > > Review request for rekonq. > > > Summary > ------- > > Icon at the right of the urlbar that allows to add an opensearch engine > > > Diffs > ----- > > "a/src/data/recherche_de_vid/303/251os_youtube.xml" PRE-CREATION > "a/src/data/wikip/303/251dia_(fr).xml" PRE-CREATION > src/CMakeLists.txt f0310b4 > src/application.cpp 95aa9cf > src/data/7digitalcom.xml PRE-CREATION > src/data/CMakeLists.txt 5f49994 > src/data/beolingus.xml PRE-CREATION > src/data/bliptv.xml PRE-CREATION > src/data/bugzilla.xml PRE-CREATION > src/data/citeseerx.xml PRE-CREATION > src/data/cpan_search.xml PRE-CREATION > src/data/de-en_beolingus.xml PRE-CREATION > src/data/dictfr.xml PRE-CREATION > src/data/duck_duck_go.xml PRE-CREATION > src/data/ecosia.xml PRE-CREATION > src/data/facebook.xml PRE-CREATION > src/data/flickr.xml PRE-CREATION > src/data/github.xml PRE-CREATION > src/data/identica_people_search.xml PRE-CREATION > src/data/kde_techbase.xml PRE-CREATION > src/data/kde_techbase_(en).xml PRE-CREATION > src/data/kde_userbase.xml PRE-CREATION > src/data/kde_userbase_(en).xml PRE-CREATION > src/data/leo_deu-eng.xml PRE-CREATION > src/data/leo_deu-fra.xml PRE-CREATION > src/data/portail_lexical_-_cnrtl.xml PRE-CREATION > src/data/urban_dictionary.xml PRE-CREATION > src/data/voila.xml PRE-CREATION > src/data/wikia.xml PRE-CREATION > src/data/wikia_(en).xml PRE-CREATION > src/data/wikipedia.xml 4e15208 > src/data/wiktionary.xml PRE-CREATION > src/data/wiktionary_(en).xml PRE-CREATION > src/data/wolfram_alpha.xml PRE-CREATION > src/data/wr_english-french.xml PRE-CREATION > src/data/wr_english-italian.xml PRE-CREATION > src/data/wr_english-spanish.xml PRE-CREATION > src/data/wr_english.xml PRE-CREATION > src/data/wr_french-english.xml PRE-CREATION > src/data/wr_italian-english.xml PRE-CREATION > src/data/wr_spanish-english.xml PRE-CREATION > src/data/yippy.xml PRE-CREATION > src/mainwindow.cpp c9863f0 > src/opensearch/opensearchengine.cpp 9d5e866 > src/opensearch/opensearchmanager.h 4f42b4a > src/opensearch/opensearchmanager.cpp e5afc14 > src/opensearch/suggestionparser.cpp 4ed13e1 > src/settings/generalwidget.cpp 642813a > src/settings/settingsdialog.cpp a3b4f97 > src/urlbar/listitem.h f5f1153 > src/urlbar/listitem.cpp f29a0e9 > src/urlbar/urlbar.h 67a693c > src/urlbar/urlbar.cpp 1f4a036 > src/urlbar/urlresolver.cpp 57d6ca6 > src/urlbar/webshortcutwidget.h PRE-CREATION > src/urlbar/webshortcutwidget.cpp PRE-CREATION > src/webtab.h 12f528d > src/webtab.cpp bac73fe > > Diff: http://git.reviewboard.kde.org/r/100064/diff > > > Testing > ------- > > > Thanks, > > Lionel > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
