[PATCH] Make Marble optional
Anton Lundin
glance at acc.umu.se
Thu Mar 6 11:06:15 PST 2014
On 05 March, 2014 - Alberto Corona wrote:
> 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.
>
I would say that so long we have a blank panel there instead of a marble
widget we should say something about why its blank.
The patch you suggested doesn't say anything about only working on Linux
and I think it would be a bad thing if it doesn't work across all our
different platforms. I would much prefer the original solution.
//Anton
> 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
> >>
> >
> >
--
Anton Lundin +46702-161604
More information about the subsurface
mailing list