[PATCH] Make Marble optional

Alberto Corona albcoron at gmail.com
Wed Mar 5 09:25:48 PST 2014


I hate gmail, still need to configure my mutt. For the list:

Sure, the reason I didn't send the patch initially was to see if you were
alright with the changes. Would a warning or verbose message that
subsurface was build without marble really be necessary if the user has to
run qmake with "CONFIG += no-marble"? Also, is there different naming for
the marble library on Windows as in order to link up marble you have to
pass "-lmarblewidget" and NOT "-lmarblewidgetd"? So that is, that config
works for compiling without marble on Linux. Also, I'm sorry if I was
unclear again and didn't ask if I could continue using  some of your work,
the reason for me continuing this thread was to communicate with you and
then see if you'd like me to ammend the commit message and or send you the
patch for your SOB.


On Wed, Mar 5, 2014 at 11:24 AM, Alberto Corona <albcoron at gmail.com> wrote:

> Sure, the reason I didn't send the patch initially was to see if you were
> alright with the changes. Would a warning or verbose message that
> subsurface was build without marble really be necessary if the user has to
> run qmake with "CONFIG += no-marble"? Also, is there different naming for
> the marble library on Windows as in order to link up marble you have to
> pass "-lmarblewidget" and NOT "-lmarblewidgetd"? So that is, that config
> works for compiling without marble on Linux. Also, I'm sorry if I was
> unclear again and didn't ask if I could continue using  some of your work,
> the reason for me continuing this thread was to communicate with you and
> then see if you'd like me to ammend the commit message and or send you the
> patch for your SOB.
>
>
> On Tue, Mar 4, 2014 at 5:34 PM, Anton Lundin <glance at acc.umu.se> wrote:
>
>> On 04 March, 2014 - Alberto Corona wrote:
>>
>> > Right, first off I fixed the warnings by the compiler by returning the
>> > object and event. I also fixed the qmake config (it was previously
>> > completely broken). Also added the instructions to compile without in
>> > the INSTALL.
>> >
>>
>> First, There are nothing about attribution about where most of the code
>> comes from. If you base your code on my code, write that in the commit
>> message. Add also a Signed-off-by: from me as the first of the lines in
>> the "audit-trail".
>> When you continue on someone else work its a really good ask that person
>> for a ACK that it looks sane.
>>
>> For example when Dirk sees that you have bin continuing on something i
>> wrote, he usually would like my feedback before merging that code.
>>
>> https://www.kernel.org/doc/Documentation/SubmittingPatches have a quite
>> good explanation how to think surrounding how and when stuff should be
>> signed.
>>
>>
>> If we don't completely remove that frame from the main window it should
>> be replaced with something saying that marble is disabled at least.
>>
>>
>> Then the "LIBS -= -lmarblewidget", if you look up a couple of rows, it
>> might be named -lmarblewidgetd .
>>
>>
>> Could you elaborate on the: "QSettings
>> had to be included into maintab.h in order for QSettings to be defined."
>>
>> I can't find any reason for that change to interact with the rest of the
>> code.
>>
>>
>> //Anton
>>
>>
>> > On Tue, Mar 04, 2014 at 09:33:13AM +0100, Anton Lundin wrote:
>> > > On 03 March, 2014 - Alberto Corona wrote:
>> > >
>> > > > Here's the link again as I forgot to include the list
>> > > >
>> https://github.com/0x1A/subsurface/commit/eb215101097120f8c62339fd767b71574cdd0bbd
>> > > >
>> > > > Like Anton said, this is a bit hacky, though I don't think there's
>> a very
>> > > > easy way to do so without reworking quite a bit of the mainwindow
>> (for
>> > > > which I'm too inexperienced with Qt for to change).
>> > > >
>> > >
>> > > When resubmitting a patch, its a god practice to resubmit it in the
>> same
>> > > way as you submitted your first patch.
>> > >
>> > > Its also a really god idea to include a changelog addressing the
>> review
>> > > comments from the last submission.
>> > >
>> > >
>> > > So, pleas go back and look at the comments from the last submission
>> and
>> > > tell us how you have addressed those.
>> > >
>> > >
>> > > //Anton
>> > >
>> > >
>> > > > On Mon, Mar 3, 2014 at 8:00 PM, Alberto Corona <albcoron at gmail.com>
>> wrote:
>> > > >
>> > > > >
>> > > > > The changes are here
>> > > > >
>> https://github.com/0x1A/subsurface/commit/eb215101097120f8c62339fd767b71574cdd0bbd
>> > > > >
>> > > > >
>> > > > > On Mon, Mar 3, 2014 at 7:29 PM, Dirk Hohndel <dirk at hohndel.org>
>> wrote:
>> > > > >
>> > > > >> On Mon, 2014-03-03 at 18:17 -0600, Alberto Corona wrote:
>> > > > >> > Ok so I've got things working now, though I'd like to see if
>> Anton is
>> > > > >> > ok with the changes. Unfortunately, I'm not familiar enough
>> with Qt to
>> > > > >> > change the way the splitters work in order to get rid of the
>> blank
>> > > > >> > dummy widget.
>> > > > >> >
>> > > > >>
>> > > > >> And where would we find those changes?
>> > > > >>
>> > > > >> /D
>> > > > >>
>> > > > >>
>> > > > >>
>> > > > >
>> > >
>> > > > _______________________________________________
>> > > > subsurface mailing list
>> > > > subsurface at hohndel.org
>> > > > http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
>> > >
>> > >
>> > > --
>> > > Anton Lundin        +46702-161604
>>
>> > >From eb215101097120f8c62339fd767b71574cdd0bbd Mon Sep 17 00:00:00 2001
>> > From: Alberto Corona <albcoron at gmail.com>
>> > Date: Mon, 3 Mar 2014 13:49:21 -0600
>> > Subject: [PATCH] Make Marble optional at compile
>> >
>> > Make Marble optional at compile by creating a dummy widget. QSettings
>> > had to be included into maintab.h in order for QSettings to be defined.
>> > Add instructions in INSTALL for compiling without Marble.
>> >
>> > Signed-off-by: Alberto Corona <albcoron at gmail.com>
>> > ---
>> >  INSTALL                  |  1 +
>> >  qt-ui/globe.cpp          | 12 ++++++++++++
>> >  qt-ui/globe.h            | 21 +++++++++++++++++++++
>> >  qt-ui/maintab.h          |  1 +
>> >  subsurface-configure.pri |  6 ++++++
>> >  5 files changed, 41 insertions(+)
>> >
>> > diff --git a/INSTALL b/INSTALL
>> > index be3c5ed..0c192e6 100644
>> > --- a/INSTALL
>> > +++ b/INSTALL
>> > @@ -74,6 +74,7 @@ $ sudo make install
>> >  To compile Subsurface:
>> >  $ git clone git://subsurface.hohndel.org/subsurface.git
>> >  $ cd subsurface
>> > + - You can use "CONFIG += no-marble" to compile without the Marble
>> widget -
>> >  $ qmake # qmake-qt4 on some flavors of Linux
>> >  $ make
>> >  $ sudo make install     [optionally, add: prefix=/usr/local]
>> > diff --git a/qt-ui/globe.cpp b/qt-ui/globe.cpp
>> > index 496afb4..69a12c5 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,14 @@ void GlobeGPS::resizeEvent(QResizeEvent *event)
>> >               messageWidget->setGeometry(5, 5, size - 10, 0);
>> >       messageWidget->setMaximumHeight(500);
>> >  }
>> > +
>> > +#else
>> > +
>> > +GlobeGPS::GlobeGPS(QWidget *parent) {}
>> > +void GlobeGPS::reload() {}
>> > +void GlobeGPS::repopulateLabels() {}
>> > +void GlobeGPS::centerOn(struct dive *dive) {}
>> > +void GlobeGPS::prepareForGetDiveCoordinates() {}
>> > +bool GlobeGPS::eventFilter(QObject *obj, QEvent *ev) {return
>> QObject::eventFilter(obj, ev);}
>> > +
>> > +#endif
>> > diff --git a/qt-ui/globe.h b/qt-ui/globe.h
>> > index 80d9613..fb57d2e 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
>> > +
>> > +// Dummy widget in place of Marble widget
>> > +
>> > +#include <QWidget>
>> > +
>> > +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
>> >  #endif // GLOBE_H
>> > diff --git a/qt-ui/maintab.h b/qt-ui/maintab.h
>> > index 946b673..fda74dc 100644
>> > --- a/qt-ui/maintab.h
>> > +++ b/qt-ui/maintab.h
>> > @@ -9,6 +9,7 @@
>> >
>> >  #include <QTabWidget>
>> >  #include <QDialog>
>> > +#include <QSettings>
>> >  #include <QMap>
>> >
>> >  #include "models.h"
>> > diff --git a/subsurface-configure.pri b/subsurface-configure.pri
>> > index 8e6aead..8d38934 100644
>> > --- a/subsurface-configure.pri
>> > +++ b/subsurface-configure.pri
>> > @@ -148,6 +148,12 @@ win32 {
>> >       DEFINES -= UNICODE
>> >  }
>> >
>> > +# Optional Marble
>> > +no-marble {
>> > +     DEFINES += NO_MARBLE
>> > +     LIBS -= -lmarblewidget
>> > +}
>> > +
>> >  #
>> >  # misc
>> >  #
>> > --
>> > 1.9.0
>> >
>>
>>
>> --
>> Anton Lundin    +46702-161604
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140305/3aecb776/attachment-0001.html>


More information about the subsurface mailing list