<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/108902/">http://git.reviewboard.kde.org/r/108902/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 12th, 2013, 11:01 a.m. EET, <b>Andrea Diamantini</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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... :)
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- Dimitrios</p>
<br />
<p>On February 12th, 2013, 12:01 p.m. EET, Dimitrios Christidis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for rekonq and Dimitrios Christidis.</div>
<div>By Dimitrios Christidis.</div>
<p style="color: grey;"><i>Updated Feb. 12, 2013, 12:01 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">+ 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.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="/show_bug.cgi?id=309470">309470</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/searchengine.cpp <span style="color: grey">(742e9bc)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108902/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>