> On Feb. 5, 2016, 6:59 p.m., Bhushan Shah wrote: > > drkonqi/CMakeLists.txt, line 86 > > <https://git.reviewboard.kde.org/r/126993/diff/1/?file=442761#file442761line86> > > > > Erm wait, drkonqi is gui program, no? > > René J.V. Bertin wrote: > Yes, but: > - on Linux, that statement does nothing AFAIK (drkonqi continues to work > just fine for me) > - on OS X, its only effect is that the executable is built as a > traditional POSIX executable instead of as an "app bundle". There is no > difference in functionality (IOW, app bundles make it easier to provide > features that are irrelevant to drkonqi). > > I can either drop this issue, or I can make the statement conditional > (`if(APPLE) ...`). But note that in most if not all previous RRs I was told > that there was no need to make this statement conditional.
Ah okay, I wasn't aware of that. Dropped issue and sorry for noise. - Bhushan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126993/#review92087 ----------------------------------------------------------- On Feb. 5, 2016, 7:43 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126993/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2016, 7:43 p.m.) > > > Review request for KDE Software on Mac OS X and Plasma. > > > Repository: plasma-workspace > > > Description > ------- > > While looking to build DrKonqi for OS X/MacPorts I came across a few issues > that appear to be syntax/coding errors that, surprisingly, were never caught > before given that they also caused compile failures when I used the same > configure/build/packaging script on Linux. > > The code uses methods that no longer exist in Qt 5.5+ : > `QString::fromAscii()`, `QHeaderView::setResizeMod()` (replaced with > `QHeaderView::setSectionResizeMode()` and `QInputDialog::getInteger()` > (instead of `QInputDialog::getInt()`). Those are the easy and straightforward > ones fixed by this patch. > > The handling of file dialogs (in 2 locations) was more problematic. Both > Apple's clang "6.0.0" and gcc 5.3 (on Linux) refused statements like > `QWeakPointer<QFileDialog> dlg = new QFileDialog(parent,defname);` . My > initial attempt was to follow the documentation for QWeakPointer, and do > `QWeakPointer<QFileDialog> dlg = QSharedPointer<QFileDialog>(new > QFileDialog(parent,defname));` but that caused a crash as soon as I clicked > on the "save backtrace as" button - on both platforms. > It turned out that `QSharedPointer<QFileDialog>()` returned a valid > (non-null) shared pointer, but after creating a QWeakPointer from it, > `dlg.data()` returned NULL -- immediately. > > I had a look around in the 5.17.0 frameworks codebase, and could find no > relevant examples of using QWeakPointer; some expressions appear to be > equivalent to the original DrKonqi code, in other locations QWeakPointer > instances are indeed created "going through" a QSharedPointer. In the end I > tested using QFileDialog (and `QFileDialog::selectedUrls()` as kate5 does, > and that works (on OS X and Linux). > > That's what the attached patch does: no QFileDialog pointer at all, just an > instance. > This RR is intended to fuel a discussion on the subject, as suggested by > Aleix; therefore I left the comment about the possibility of > invalidating/deleting the file dialog e.g. through a DBus call. Is that > actually a possibility, and why wouldn't it be in Kate? Are there no other > ways to prevent it, or check for its occurrence? > > This patch also marks drkonqi as a nongui executable, a requirement on OS X > and which shouldn't cause any regressions elsewhere AFAIK. > > > Diffs > ----- > > drkonqi/CMakeLists.txt deb8c40 > drkonqi/bugzillaintegration/bugzillalib.cpp 802c5fb > drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp e60fb14 > drkonqi/bugzillaintegration/reportassistantpages_bugzilla_duplicates.cpp > 4f8f4ea > drkonqi/drkonqi.cpp b12c118 > > Diff: https://git.reviewboard.kde.org/r/126993/diff/ > > > Testing > ------- > > On OS X 10.9.1 and Kubuntu 14.04, both with Qt 5.5.1 and KF5 5.17.0 installed > into /opt/local > > > Thanks, > > René J.V. Bertin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
