<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://reviewboard.kde.org/r/4587/">http://reviewboard.kde.org/r/4587/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for marble.</div>
<div>By Dennis Nienhüser.</div>
<p style="color: grey;"><i>Updated 2010-07-18 16:01:13.200564</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">Extensions to the former patch:
- Extend MarbleAbstractRunner interface to allow runners do reverse geocoding and routing
- MarbleAbstractRunner does not derive from QThread anymore, threads are handled transparently by the MarbleRunnerManager
- Use QThreadPool and QRunnable (new class RunnerTask) to execute runners in threads
- Improve method names in MarbleAbstractRunner and MarbleRunnerManager
- Move route parsing from RoutingModel to plugins, RoutingModel only accepts GeoDataDocument* from now on
- Transform OpenRouteService and Yours RoutingProvider classes into runner plugins
- Remove AbstractRoutingProvider, not needed anymore
- Add new gosmore runner plugin, it does offline routing by calling the gosmore application and parsing its output. Does nothing if gosmore is not installed or ~/.local/share/marble/maps/earth/gosmore/gosmore.pak does not exist (no default map provided, to be improved)
- Routing UI can handle several alternative routes coming in (displayed fifo in a combobox for now, to be improved, see comments in code)
>From my tests, things are working fine. Some todos are noted in the code, but I'd rather continue to work on them in trunk before the patch gets unmaintainable locally.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description (updated)</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">This is the first part of some refactoring of MarbleAbstractRunner and related classes. It moves all individual runners into their own plugins. Later I plan to add reverse geocoding and routing support to the interface and make more such plugins from the existing code (OSR, Yours for routing, nominatim for reverse geocoding, more to come).
The patch looks longish, but mostly just moves things around without adding/changing too much functionality. UI and behavior should be just the same.
To get an overview, here are the high level changes:
- Move MarbleAbstractRunner* from src/lib/runners/ to src/lib.
- Create a RunnerPlugin class in src/lib, derived from PluginInterface, which adds a new MarbleAbstractRunner factory method.
- Export RunnerPlugin and MarbleAbstractRunner headers
- Add support for RunnerPlugins in PluginManager
- Move all individual src/lib/runners/*Runner* classes to their own plugins in src/plugins/runner/$name
- Get rid of src/lib/runners/ directory
- Extend MarbleAbstractRunner interface, runners must tell whether they can work offline and on other planets
- Change MarbleRunnerManager to use the plugins
I made two changes compared to previous approaches wrt plugins:
- RunnerPlugin provides default implementations for all the name(), description() etc methods and lets plugins change it (protected). That removes quite a few lines of IMHO useless code when writing new plugins (instead of overwriting all those methods again, call setName("foo"), setDescription("bla bla") etc in the new plugin).
- A RunnerPlugin is not both a MarbleAbstractRunner and a factory for itself, instead it is just a factory for MarbleAbstractRunner instances. I think this is generally mixed up currently in the other plugins without any benefit, but confusing code (look at PluginManager).
I think it's better this way, speak up if there are reasons against it.</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;">Various search and routing queries on earth and moon.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PluginManager.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RunnerTask.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RunnerTask.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/AbstractRoutingProvider.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/AbstractRoutingProvider.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/OrsRoutingProvider.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/OrsRoutingProvider.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingManager.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingModel.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.ui <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/YoursRoutingProvider.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/routing/YoursRoutingProvider.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.h <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.cpp <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/CMakeLists.txt <span style="color: grey">(1151118)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmoreRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmoreRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServiceRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServiceRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursRunner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursRunner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4587/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>