[Marble-devel] Review Request: Split class RunnerPlugin

Tue May 8 18:03:31 UTC 2012

> On May 5, 2012, 7:45 a.m., Torsten Rahn wrote:
> > src/lib/RoutingRunnerPlugin.h, line 50
> > <http://git.reviewboard.kde.org/r/104840/diff/1/?file=62293#file62293line50>
> >
> >     I think usually in Qt's API the factory methods are called "create..." instead of "new..."

That's true and I also prefer "create..." to "new...". In order to keep the breakage of 3rd-party plugins minimal, I left the method name as is. I suggest to change that when we also introduce specialized abstract classes for the runners (which is also on my todo list).

> On May 5, 2012, 7:45 a.m., Torsten Rahn wrote:
> > src/lib/RoutingRunnerPlugin.h, line 41
> > <http://git.reviewboard.kde.org/r/104840/diff/1/?file=62293#file62293line41>
> >
> >     
> >     This comment is missing in the other classes :-)
> >     
> >     BTW: Originally we distinguished between the normal "name" string and the "guiString" because the gui string added the Keyboard Accelerator underlining for certain letters and its associated keystrokes. Nowadays this accelerator assignment seems to be done automatically. So there's maybe no need for the distinction.

Yeah, there is a new distinction between name() and guiString(): name() is part of the generic metadata of a plugin and shall return a full name that indicates what the plugin is about, e.g. "OpenRouteService routing plugin". I'm planning to extend our about dialog to list *all* installed plugins (with their respective authors and licenses), where this meta information is used. guiString(), on the other hand, shall return a context-sensitive name, e.g. "MoNav" in the routing widget (if the current route was found by MoNav), or "GPS" in the current position widget if the current position is tracked by a GPS device.

- Bernhard

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

On May 3, 2012, 11:01 a.m., Bernhard Beschow wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104840/
> -----------------------------------------------------------
> 
> (Updated May 3, 2012, 11:01 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> By splitting the class RunnerPlugin into separate classes for searching, routing, reverse geocoding and parsing, implementing a runner plugin will become more straight-forward. Before the patch, an implementer of e.g. a parsing plugin had to filter out unneccessary things such as which routing profile is supported.
> 
> Before the split could happen, the nominatim and gosmore runner plugins had to be split such that each new plugin had only one capability.
> 
> Please also review the user-visible strings of the nominatim and gosmore plugins.
> 
> Have a look at [1] to see the individual commits.
> 
> I'm looking forward to your reviews!
> 
> [1] https://github.com/shentok/marble/commits/RunnerPlugins-single-capability
> 
> 
> Diffs
> -----
> 
>   src/plugins/runner/gosmore/GosmoreRunner.cpp a5062be2a6ef55cae72b8fedfde675d89f73e53f 
>   src/plugins/runner/gosmore/GosmoreRunner.h 74b0400f8ee120328c7b94a4752436107d5c27b6 
>   src/plugins/runner/gosmore/CMakeLists.txt 984550686b4c184182d17027c44b966eda544e7c 
>   src/plugins/runner/gosmore/GosmorePlugin.h 6d9af2afea9ea21341b147e66320b298c6dd6c32 
>   src/plugins/runner/gosmore/GosmorePlugin.cpp 14a2d78b001bc203716c3402424657a19d9aace8 
>   src/plugins/runner/gosmore-routing/CMakeLists.txt PRE-CREATION 
>   src/plugins/runner/gosmore-routing/GosmoreRoutingPlugin.h PRE-CREATION 
>   src/plugins/runner/gosmore-routing/GosmoreRoutingPlugin.cpp PRE-CREATION 
>   src/plugins/runner/gosmore-routing/GosmoreRoutingRunner.h PRE-CREATION 
>   src/plugins/runner/gosmore-routing/GosmoreRoutingRunner.cpp PRE-CREATION 
>   src/lib/RunnerPlugin.cpp 31b89f6c2a88dba46cb6c7f6e2a8cc4ab796c548 
>   src/lib/RunnerTask.h a547f3caca23eefd88375718c85dcd9946b70cba 
>   src/lib/RunnerTask.cpp 5e2860e52dc2ff024cb49237d6e2a1073154c776 
>   src/lib/SearchRunnerPlugin.h PRE-CREATION 
>   src/lib/SearchRunnerPlugin.cpp PRE-CREATION 
>   src/lib/routing/RoutingManager.cpp 0e0d0ef49aa9ebffd720150907265068d6693d89 
>   src/lib/routing/RoutingProfileSettingsDialog.h 1402cc91904d9ac8cc825f64cc3a58bfd9bf1b7f 
>   src/lib/routing/RoutingProfileSettingsDialog.cpp 48f0f31ac070cbb16dc0fad7930f10b05370ceec 
>   src/lib/routing/RoutingProfilesModel.cpp 152b4ecd088e01cbb50536b4bec176a15884e7b8 
>   src/lib/routing/RoutingProfilesWidget.cpp 90bafb486f0cca8e34c2e4f2f56117f0e93b0a50 
>   src/plugins/runner/CMakeLists.txt edf0422e48a9393f10c6200b8ddfd5614beaafb2 
>   src/plugins/runner/cache/CachePlugin.h 8ef176a48d7a7439c96846150123cf4e947aa07d 
>   src/plugins/runner/cache/CachePlugin.cpp ce32c97e74baa6eabf10fa46c643bfab27e1f674 
>   src/plugins/runner/gosmore-reversegeocoding/CMakeLists.txt PRE-CREATION 
>   src/plugins/runner/gosmore-reversegeocoding/GosmoreReverseGeocodingPlugin.h PRE-CREATION 
>   src/plugins/runner/gosmore-reversegeocoding/GosmoreReverseGeocodingPlugin.cpp PRE-CREATION 
>   src/plugins/runner/gosmore-reversegeocoding/GosmoreReverseGeocodingRunner.h PRE-CREATION 
>   src/plugins/runner/gosmore-reversegeocoding/GosmoreReverseGeocodingRunner.cpp PRE-CREATION 
>   src/lib/RoutingRunnerPlugin.h PRE-CREATION 
>   src/lib/RoutingRunnerPlugin.cpp PRE-CREATION 
>   src/lib/RunnerPlugin.h f3f972132a26b85474d4c0cc8a0118cbdb864336 
>   src/lib/ReverseGeocodingRunnerPlugin.h PRE-CREATION 
>   src/lib/ReverseGeocodingRunnerPlugin.cpp PRE-CREATION 
>   src/lib/ParseRunnerPlugin.cpp PRE-CREATION 
>   src/lib/PluginManager.h 3b1ab1e3c7859317f18532abf541becea2a61964 
>   src/lib/PluginManager.cpp a2510ed4feeafc246dd175c7524ee558e62f80c1 
>   src/lib/CMakeLists.txt 5210329fb771254a8dc5ab19f7020d7145057401 
>   src/lib/MarbleRunnerManager.cpp c546bd4a140d7e875c984b4049fde69d74a30ea3 
>   src/lib/ParseRunnerPlugin.h PRE-CREATION 
>   src/plugins/runner/gpx/GpxPlugin.h 5ebc5c1130bc07bdb0c84c1f91879ef110e30c91 
>   src/plugins/runner/gpx/GpxPlugin.cpp 25d8eb1969ba6bd7dc19f3c2a58b03be7be01ddd 
>   src/plugins/runner/hostip/HostipPlugin.h 6b13bd2408a70394e2fb9d3e5443642c35fd12f8 
>   src/plugins/runner/hostip/HostipPlugin.cpp 002417da327b7b68795c56d0d53671be60236b6a 
>   src/plugins/runner/kml/KmlPlugin.h 29136bae8732ac6cfdd75e56b27331ba0180d9da 
>   src/plugins/runner/kml/KmlPlugin.cpp 00023f46c746fef1314a05aa363091aeb98c3d0c 
>   src/plugins/runner/latlon/LatLonPlugin.h 542a5d1148a4cdc67a1b463ce3d186fcadb9b099 
>   src/plugins/runner/latlon/LatLonPlugin.cpp 5900fc54c97fae39677c4703c99f3d1b5c9dff65 
>   src/plugins/runner/local-osm-search/LocalOsmSearchPlugin.h 0225813628509203c1da044bf4e5e9c07451c07d 
>   src/plugins/runner/local-osm-search/LocalOsmSearchPlugin.cpp 871f4ef1219c13422130a1afb6f8cf7069ec1209 
>   src/plugins/runner/localdatabase/LocalDatabasePlugin.h 1fec957ef4eb0d90dfcc6c344dc6eb735a4da6ff 
>   src/plugins/runner/localdatabase/LocalDatabasePlugin.cpp 0977bf5a16bfec74a42cf5cce71e85b2e15843ba 
>   src/plugins/runner/log/LogPlugin.h e08d67f5faf15e39dc52461aab698de00d4e2fe0 
>   src/plugins/runner/log/LogPlugin.cpp 0f9361b3ae0580e4996ed4626295874bace5d125 
>   src/plugins/runner/mapquest/MapQuestPlugin.h 49b6046cc7adfb4d5dc950148906df7c1e687236 
>   src/plugins/runner/mapquest/MapQuestPlugin.cpp 3d4d49f88011befdd474ecf7d3276a8c6c90c785 
>   src/plugins/runner/monav/MonavConfigWidget.h c0b9c7f3f798c70424f57a15df8da1f7bbcba4c2 
>   src/plugins/runner/monav/MonavConfigWidget.cpp 70b016bca206f227b172216ff88f1541f9787a67 
>   src/plugins/runner/monav/MonavPlugin.h 56661936da2bd624002a46c2f1cadb4bdd59b6bd 
>   src/plugins/runner/monav/MonavPlugin.cpp bcab1dd9c30dc2b2e9a2956d14a49f4b2fba3f1b 
>   src/plugins/runner/nominatim-reversegeocoding/CMakeLists.txt PRE-CREATION 
>   src/plugins/runner/nominatim-reversegeocoding/NominatimReverseGeocodingPlugin.h PRE-CREATION 
>   src/plugins/runner/nominatim-reversegeocoding/NominatimReverseGeocodingPlugin.cpp PRE-CREATION 
>   src/plugins/runner/nominatim-reversegeocoding/OsmNominatimReverseGeocodingRunner.h PRE-CREATION 
>   src/plugins/runner/nominatim-reversegeocoding/OsmNominatimReverseGeocodingRunner.cpp PRE-CREATION 
>   src/plugins/runner/nominatim-search/CMakeLists.txt PRE-CREATION 
>   src/plugins/runner/nominatim-search/NominatimSearchPlugin.h PRE-CREATION 
>   src/plugins/runner/nominatim-search/NominatimSearchPlugin.cpp PRE-CREATION 
>   src/plugins/runner/nominatim-search/OsmNominatimSearchRunner.h PRE-CREATION 
>   src/plugins/runner/nominatim-search/OsmNominatimSearchRunner.cpp PRE-CREATION 
>   src/plugins/runner/nominatim/CMakeLists.txt 461e3a4eb662c33c5ef0b29a34db1c08512661eb 
>   src/plugins/runner/nominatim/NominatimPlugin.h 22d8d10e01f4cf61df7d628a0be6fc19dc5d60cc 
>   src/plugins/runner/nominatim/NominatimPlugin.cpp 2a4ffed4a568c6a38578f88a0cd630d62acfcee3 
>   src/plugins/runner/nominatim/OsmNominatimRunner.h 43b498aa5b06278b141cdd6946da9d3aaab3bafc 
>   src/plugins/runner/nominatim/OsmNominatimRunner.cpp 8d1d0461e6ff67c830af4f2cdcf84b993698c523 
>   src/plugins/runner/open-source-routing-machine/OSRMPlugin.h d906bfdf6e104aa70fcc2dfa05849b94abe7d20d 
>   src/plugins/runner/open-source-routing-machine/OSRMPlugin.cpp c415699266056a78698228f22c8f882600a95080 
>   src/plugins/runner/openrouteservice/OpenRouteServicePlugin.h bdcbcb18a28e3951c1eb739635635ad27b89120e 
>   src/plugins/runner/openrouteservice/OpenRouteServicePlugin.cpp 65bb12323cfd2bca28612d77ea596c2184c55589 
>   src/plugins/runner/osm/OsmPlugin.h 472d1894a36d96c537f6554a66263f3b17a04aa7 
>   src/plugins/runner/osm/OsmPlugin.cpp bd15538f0cba74b79188d1d7f57a7a18671e37ad 
>   src/plugins/runner/pnt/PntPlugin.h e705fde173c28acaedcafbbe8a8c9de61202c935 
>   src/plugins/runner/pnt/PntPlugin.cpp aaa47a6be600c124a78ddae4ba9067efd969c675 
>   src/plugins/runner/routino/RoutinoPlugin.h f65c2e66f5d6db08677d78afefbd75435807a4a9 
>   src/plugins/runner/routino/RoutinoPlugin.cpp 05395790a2747ff5cb4d07bd834e41caa508b952 
>   src/plugins/runner/shp/ShpPlugin.h 7bf2d60fca21e066430919a2285770a7075940dc 
>   src/plugins/runner/shp/ShpPlugin.cpp 8896e4964fb23ae0764f7e88a61e80f5e3792e16 
>   src/plugins/runner/yours/YoursPlugin.h 9c2ce0f4d7db9e3e9b3dc3ac85c57aaeb7fbf29d 
>   src/plugins/runner/yours/YoursPlugin.cpp e3758e2656bf998e294d04072624c990ed23ff10 
>   tests/PluginManagerTest.cpp 7adaf2e0fb6095f6369924ef9781cec13380ff62 
> 
> Diff: http://git.reviewboard.kde.org/r/104840/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bernhard Beschow
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20120508/d1d51b62/attachment-0001.html>