[KDE/Mac] Review Request 126993: drkonqi : fix build errors and build nongui

[Bhushan Shah]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20160205/8b670219/attachment-0001.html>