> On Feb. 12, 2013, 11:01 a.m., Andrea Diamantini wrote: > > I suggest following changes in your patch. I think it's... easier this way. > > I also removed "Search Engine" --> "Web Shortcuts" change. While I'm sure a > > kde user knows how to deal with "web shortcuts" and manage them, I think > > newbies feel better with the "Search Engine" term, widely used in browser. > > Moreover, rekonq choices its default search engine by that, so... :) > >
Thanks for reviewing my patch, particularly in SearchEngine::fromString(). I took the liberty of removing a few trailing whitespace characters and uploaded a commit patch. - Dimitrios ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108902/#review27268 ----------------------------------------------------------- On Feb. 12, 2013, 12:01 p.m., Dimitrios Christidis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108902/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2013, 12:01 p.m.) > > > Review request for rekonq and Dimitrios Christidis. > > > Description > ------- > > The issue is partly described in bug #309470. Rekonq does not honor two > configuration options for Web Shortcuts: "EnableWebShortcuts" and > "UsePreferredWebShortcutsOnly". > > The first part of the patch attempts to address this issue. I am not > particularly proud of the current solution; please advise me on proper > variable naming and overall code structure. Furthermore, I am not aware of > the correct way to compare two KService::Ptr objects. I ended up comparing > the filename of the desktop entry, but is there a better way? Is it even > correct? Lastly, should SearchEngine::favorites() be modified as well? If > not, preferred engine will still appear in the suggestions, but won't be > usable. > > The second part of the patch is about the change of "Search Engines" to "Web > Shortcuts". Should the conditional compilation directives be removed or is it > too early? > > > This review will be split into two commits. > > > This addresses bug 309470. > /show_bug.cgi?id=309470 > > > Diffs > ----- > > src/searchengine.cpp 742e9bc > > Diff: http://git.reviewboard.kde.org/r/108902/diff/ > > > Testing > ------- > > + With EnableWebShortcuts=false: > Typing a web shortcut followed by the delimiter does not suggest any engine. > Pressing Enter uses the default engine. > > + With EnableWebShortcuts=true and UsePreferredWebShortcutsOnly=true: > Engines that are not preferred are not used. > > + With EnableWebShortcuts=true and UsePreferredWebShortcutsOnly=false: > Works as before. > > > Thanks, > > Dimitrios Christidis > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
