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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > >> wrote: > >> > > > > >> > > > > > >> > > > > The changes are here > >> > > > > > >> https://github.com/0x1A/subsurface/commit/eb215101097120f8c62339fd767b71574cdd0bbd > >> > > > > > >> > > > > > >> > > > > On Mon, Mar 3, 2014 at 7:29 PM, Dirk Hohndel <[email protected]> > >> 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 > >> > > > [email protected] > >> > > > 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 <[email protected]> > >> > 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 <[email protected]> > >> > --- > >> > 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 _______________________________________________ subsurface mailing list [email protected] http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
