Review Request: Modify the KUrilFilter API once more to accomodate unforseen use case...

[Dawit Alemayehu]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/5172/
-----------------------------------------------------------

(Updated 2010-09-01 19:23:26.522963)


Review request for kdelibs.


Changes
-------

Okay, upon feedback and some careful consideration I have changed the proposed modification to KUriFilter as follows:

#1. Instead of setAlwaysReturnPreferredSearchProvidersOn/ isAlwaysReturnPreferredSearchProvidersOn, I have added functions that allows you to set/get filtering options:

SearchFilterOptions searchFilteringOptions() const;
void setSearchFilteringOptions(SearchFilterOptions)

SearchFilterOption is the enum that provides the flags for use with these functions. See the attached patch. But basically, the idea behind is to allow future extensibility without having to add yet another new function ; so now you can do

KUriFilterData filterData("dummy"); // the "dummy" placeholder is not a requirement and can be left out...
filterData.setCheckExecutables(false);
filterData.setSearchFilteringOptions(KUriFilterData::RetreivePreferredSearchProvidersOnly);

if (KUriFilter::self()->filterSearchUri(filterData, KUriFilter::NormalTextFilter))
  ....

The other flag currently present in this new options enum, "RetreiveAllSearchProvidersOnly", allows you to retrieve all available search providers in the system. In the above example simply replacing "RetreivePreferredSearchProvidersOnly" with it will do the trick. Alternatively you can use an OR'ed combination of the two flag or "RetreivePreferredOrAllSearchProvidersOnly" to retrieve the preferred search providers with fallback to all whenever no preferred search providers are available. 

#2. A new function, allQueriesForSearchProvider, has been added to allow access to all the shortcut keywords available for a given search provider. This is the same as queryForPreferredSearchProvider (which should really be renamed queryForSearchProvider now) except it returns the entire list of webshortcut possible where as the latter returns only the first one. This use-case is also mostly specific to Plasma for now.

#3. A new function, setDefaultUrlScheme, was added to allow the programmer to specify which protocol should be appended when filtering short urls which are potentially valid, e.g. "kde.org". The url scheme (aka protocol) set will be validated to determine if there is any support for it in KDE, i.e. KProtocolInfo::isKnownProtocol.

#4.  Updated the documetation with as much information on how to use these new functionalities as possible.


Summary
-------

The attached patch does the following:

#1. Adds setAlwaysReturnPreferredSearchProvidersOn/isAlwaysReturnPreferredSearchProvidersOn to ensure that the kuriikwsfilter plugin returns the preferred provider search engine list when no default search engine is specified. Currently it fails to do so and that behavior will not change unless you set the flag using the aforementioned function. 
With this change instead of having to do the following:

KUriFilterData filterData(foo);
filterData.setAlternateDefaultSearchProvider("google"); // hmm... would google be there ?
if (KUriFilter::self()->filterUri(filterData, QStringList() << "kuriikwsfilter")
  // do something here...

With

KUriFilterData filterData(foo);
filterData.setAlwaysReturnPreferredSearchProvidersOn(true);
if (KUriFilter::self()->filterUri(filterData, QStringList() << "kuriikwsfilter")
  // do something here...

#2. Deprecates the newly added function filterSearchUri(KUriFilterData&) in favor of one that accepts an enum filterSearchUri(KUriFilterData&, SearchFilterTypes types). That way instead of the caller having to know what "kuriikwsfilter' or 'kurisearchfilter' plugins do, they can specify "NormalTextFilter" or "WebShortcutFilter" or an OR combination of both. This change makes it possible to do search filtering without having to worry about which filters to use. For example, the last if statement from the above example would become:

if (KUriFilter::self()->filterSearchUri(filterData, KUriFilter::NormalTextFilter))
   // do something here...


Diffs (updated)
-----

  /trunk/KDE/kdebase/runtime/kurifilter-plugins/ikws/kuriikwsfilter.h 1170486 
  /trunk/KDE/kdebase/runtime/kurifilter-plugins/ikws/kuriikwsfilter.cpp 1170486 
  /trunk/KDE/kdebase/runtime/kurifilter-plugins/shorturi/kshorturifilter.h 1170486 
  /trunk/KDE/kdebase/runtime/kurifilter-plugins/shorturi/kshorturifilter.cpp 1170486 
  /trunk/KDE/kdelibs/kio/kio/kurifilter.h 1170486 
  /trunk/KDE/kdelibs/kio/kio/kurifilter.cpp 1170486 

Diff: http://svn.reviewboard.kde.org/r/5172/diff


Testing (updated)
-------

kurifiltertest provides the same result as without this patch (8 passed, 1 failed). 
Some test cases will need to be added for these changes as well.


Thanks,

Dawit

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100901/135c52cc/attachment.htm>