[Marble-devel] Review Request: Transform all runners into plugins

Dennis Nienhüser earthwings at gentoo.org
Sun Jul 11 01:29:51 CEST 2010


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

Review request for marble.


Summary
-------

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.


Diffs
-----

  /trunk/KDE/kdeedu/marble/src/lib/CMakeLists.txt 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleAbstractRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleControlBox.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/MarbleRunnerManager.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/PluginManager.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingInputWidget.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/routing/RoutingWidget.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/HostipRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LatLonRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/LocalDatabaseRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/MarbleAbstractRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OnfRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.h 1148459 
  /trunk/KDE/kdeedu/marble/src/lib/runners/OsmNominatimRunner.cpp 1148459 
  /trunk/KDE/kdeedu/marble/src/plugins/CMakeLists.txt 1148459 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabaseRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/OnfRunner.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/CMakeLists.txt PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.cpp PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.h PRE-CREATION 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/OsmNominatimRunner.cpp PRE-CREATION 

Diff: http://reviewboard.kde.org/r/4587/diff


Testing
-------

Various search and routing queries on earth and moon.


Thanks,

Dennis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100710/ab6f32ff/attachment.html 


More information about the Marble-devel mailing list