[rekonq] Review Request 108902: Honor configuration for Web Shortcuts

[Andrea Diamantini]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108902/#review27269
-----------------------------------------------------------


well... probably it's me or it is rb... I cannot update your patch with mine. Here it is for now. If this is not enough, I'll push a remote branch about.

diff --git a/src/searchengine.cpp b/src/searchengine.cpp
index 742e9bc..28ab605 100644
--- a/src/searchengine.cpp
+++ b/src/searchengine.cpp
@@ -37,6 +37,8 @@ struct SearchEnginePrivate
 {
     SearchEnginePrivate() : isLoaded(false) {}
     bool isLoaded;
+    bool isEnabled;
+    bool usePreferredOnly;
     QString delimiter;
     KService::List favorites;
     KService::Ptr defaultEngine;
@@ -51,16 +53,15 @@ void SearchEngine::reload()
     KConfig config("kuriikwsfilterrc"); //Shared with konqueror
     KConfigGroup cg = config.group("General");
 
+    d->isEnabled = cg.readEntry("EnableWebShortcuts", true);
+    d->usePreferredOnly = cg.readEntry("UsePreferredWebShortcutsOnly", false);
+    
     //load delimiter
     d->delimiter = cg.readEntry("KeywordDelimiter", ":");
 
     // load favorite engines
     QStringList favoriteEngines;
-#if KDE_IS_VERSION(4,9,0)
     favoriteEngines = cg.readEntry("PreferredWebShortcuts", favoriteEngines);
-#else
-    favoriteEngines = cg.readEntry("FavoriteSearchEngines", favoriteEngines);
-#endif
 
     KService::List favorites;
     KService::Ptr service;
@@ -76,11 +77,7 @@ void SearchEngine::reload()
 
     // load default engine
     QString dse;
-#if KDE_IS_VERSION(4,9,0)
     dse = cg.readEntry("DefaultWebShortcut");
-#else
-    dse = cg.readEntry("DefaultSearchEngine");
-#endif
 
     d->defaultEngine = KService::serviceByDesktopPath(QString("searchproviders/%1.desktop").arg(dse));
 
@@ -117,10 +114,18 @@ KService::Ptr SearchEngine::defaultEngine()
 
 KService::Ptr SearchEngine::fromString(const QString &text)
 {
-    KService::List providers = KServiceTypeTrader::self()->query("SearchProvider");
+    KService::Ptr service;
+
+    // first, the easy part...
+    if (!d->isEnabled)
+        return service;
+
+    KService::List providers = (d->usePreferredOnly) 
+        ? SearchEngine::favorites()
+        : KServiceTypeTrader::self()->query("SearchProvider");
+    
     int i = 0;
     bool found = false;
-    KService::Ptr service;
     while (!found && i < providers.size())
     {
         QStringList list = providers.at(i)->property("Keys").toStringList();


- Andrea Diamantini


On Feb. 11, 2013, 8:59 a.m., Dimitrios Christidis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108902/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2013, 8:59 a.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/settings/settingsdialog.cpp 11575f7 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rekonq/attachments/20130212/85869c2a/attachment-0001.html>