[PATCH] Make Marble optional

[Lubomir I. Ivanov]

On 1 March 2014 08:15, Alberto <albcoron at gmail.com> wrote:
> From: Alberto Corona <albcoron at gmail.com>
>
> Implemented some changes suggested by glance
> Building could now be done without having to have Marble installed
> Fixes #394
>

hello,
this is a fix in the correct direction. i've added some notes bellow.

> Signed-off-by: Alberto Corona <albcoron at gmail.com>
> ---
>  qt-ui/globe.cpp          | 11 +++++++++++
>  qt-ui/globe.h            | 21 +++++++++++++++++++++
>  qt-ui/mainwindow.h       |  1 +
>  subsurface-configure.pri | 10 +++++++---
>  subsurface.pro           |  2 +-
>  5 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/qt-ui/globe.cpp b/qt-ui/globe.cpp
> index 496afb4..7837ae7 100644
> --- a/qt-ui/globe.cpp
> +++ b/qt-ui/globe.cpp
> @@ -1,4 +1,5 @@
>  #include "globe.h"
> +#ifndef NO_MARBLE
>  #include "kmessagewidget.h"
>  #include "mainwindow.h"
>  #include "ui_mainwindow.h"
> @@ -297,3 +298,13 @@ void GlobeGPS::resizeEvent(QResizeEvent *event)
>                 messageWidget->setGeometry(5, 5, size - 10, 0);
>         messageWidget->setMaximumHeight(500);
>  }
> +
> +#else
> +
> +GlobeGPS::GlobeGPS(QWidget *parent) {}
> +void GlobeGPS::repopulateLabels() {}
> +void GlobeGPS::centerOn(dive *dive) {}
> +void GlobeGPS::prepareForGetDiveCoordinates() {}
> +void GlobeGPS::reload() {}
> +bool GlobeGPS::eventFilter(QObject *obj, QEvent *ev) {}

eventFilter() would need to return a bool here, to prevent a warning.

> +#endif
> diff --git a/qt-ui/globe.h b/qt-ui/globe.h
> index 80d9613..e25f244 100644
> --- a/qt-ui/globe.h
> +++ b/qt-ui/globe.h
> @@ -1,5 +1,6 @@
>  #ifndef GLOBE_H
>  #define GLOBE_H
> +#ifndef NO_MARBLE
>
>  #include <marble/MarbleWidget.h>
>  #include <marble/GeoDataCoordinates.h>
> @@ -41,4 +42,24 @@ slots:
>         void prepareForGetDiveCoordinates();
>  };
>
> +#else
> +
> +#include <QWidget>
> +
> +struct dive;
> +
> +class GlobeGPS : public QWidget {
> +        Q_OBJECT
> +public:
> +        GlobeGPS(QWidget *parent);
> +        void reload();
> +        void repopulateLabels();
> +        void centerOn(struct dive *dive);
> +        bool eventFilter(QObject *, QEvent *);
> +public
> +slots:
> +        void prepareForGetDiveCoordinates();
> +};
> +
> +#endif

the indentation here needs to be with one real-tab instead of 8
spaces. even if your indentation is shown as 8 spaces the actual tab
character needs to be inserted.
you could adjust your text editor as described in CodingStyle.

>  #endif // GLOBE_H
> diff --git a/qt-ui/mainwindow.h b/qt-ui/mainwindow.h
> index 1db3d5a..65a532c 100644
> --- a/qt-ui/mainwindow.h
> +++ b/qt-ui/mainwindow.h
> @@ -9,6 +9,7 @@
>
>  #include <QMainWindow>
>  #include <QAction>
> +#include <QSettings>

this seems like a separate change.
any reason for that?

>  #include <QUrl>
>
>  #include "ui_mainwindow.h"
> diff --git a/subsurface-configure.pri b/subsurface-configure.pri
> index 8e6aead..fb48185 100644
> --- a/subsurface-configure.pri
> +++ b/subsurface-configure.pri
> @@ -116,7 +116,6 @@ isEmpty(XML2_CFLAGS)|isEmpty(XML2_LIBS): \
>  isEmpty(XSLT_CFLAGS)|isEmpty(XSLT_LIBS): \
>         error("Could not find libxslt. Did you forget to install it?")
>
> -

whitespace changes with not particular meaning in general should be
avoided as they are like noise near the *actual* changes, even if
intentional.
we tend to leave them roaming for a while until a patch is *specific*
for whitespace cleanup.

>  QMAKE_CFLAGS *= $$XML2_CFLAGS $$XSLT_CFLAGS
>  QMAKE_CXXFLAGS *= $$XML2_CFLAGS $$XSLT_CFLAGS
>  LIBS *= $$XSLT_LIBS $$XML2_LIBS
> @@ -131,14 +130,19 @@ link_pkgconfig: PKGCONFIG += libzip sqlite3
>  # Add libiconv if needed
>  link_pkgconfig: packagesExist(libiconv): PKGCONFIG += libiconv
>
> +# Add Marble if present
> +link_pkgconfig: packagesExist(libmarlbe): DEFINES += NO_MARBLE
> +

there seems to be a typo here libmarlbe -> libmarble.
but this part should be removed...
the marble plugin does not provide pkg-config entries, which means
that packagesExist() will always return false unless you have manually
created a .pc file for it.
i think that we should always look for marble, unless the user does manually a:
qmake <someoptions> "DEFINES += NO_MARBLE"

>  #
>  # Find libmarble
>  #
>  # Before Marble 4.9, the GeoDataTreeModel.h header wasn't installed
>  # Check if it's present by trying to compile
>  # ### FIXME: implement that
> -win32: CONFIG(debug, debug|release): LIBS += -lmarblewidgetd
> -else: LIBS += -lmarblewidget
> +!contains(DEFINES, NO_MARBLE) {
> +        win32: CONFIG(debug, debug|release): LIBS += -lmarblewidgetd
> +        else: LIBS += -lmarblewidget
> +}
>

looks good to me.

>  #
>  # Platform-specific changes
> diff --git a/subsurface.pro b/subsurface.pro
> index 81cb906..49865d7 100644
> --- a/subsurface.pro
> +++ b/subsurface.pro
> @@ -4,7 +4,7 @@ QT = core gui network svg
>  lessThan(QT_MAJOR_VERSION, 5) {
>         QT += webkit
>  } else {
> -       QT += webkitwidgets
> +       !android: QT += webkitwidgets
>  }

any reason for this particular change?
it will probably be best to separate this patch into 2-3 patches and
send them in series with separate commit messages.

>  INCLUDEPATH += qt-ui $$PWD
>  DEPENDPATH += qt-ui
> --

lubomir
--