Review Request 117478: Convert dolphin (frameworks) to Qt5 signal/slot syntax

Mon Apr 14 21:30:49 BST 2014

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117478/#review55777
-----------------------------------------------------------

Thanks for your work on this, Alexander! I'm glad that we are getting closer to a working KF5-based Dolphin.

I'm a bit concerned by Thiago's comment though (thanks Thiago for the hint btw - are there details about the behavior differences between the two ways to establishe connections in Qt's docs? I couldn't find anything with a quick search). If there are slight behavior differences, then there might be a risk that we will see regressions. Since it is basically impossible to test the effect of all connections exhaustively, and I don't know exactly what the behavior difference is and if it could cause trouble, I'm not sure if we should really do an automated conversion of all connect statements. Maybe one should discuss this issue (which will be of interest not only to Dolphin, but also to many other apps and to KF5 itself) on the kde-frameworks mailing list?

The safer solution would be to leave all existing connections as they are, unless they don't work, and use the new syntax only for new code. In principle, it should be easy to find out which connections don't work and cause Dolphin to not show any files because there is a runtime warning for each failed connect call, right?

- Frank Reininghaus

On April 10, 2014, 2:57 p.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117478/
> -----------------------------------------------------------
> 
> (Updated April 10, 2014, 2:57 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Requires https://git.reviewboard.kde.org/r/117395/
> 
> Most of this conversion was done automatically, only minor manual changes were needed.
> There is a problem with KAction -> QAction, since there is no longer the signal triggered() that reports the middle mouse button.
> I attempted to fix this by using QApplication::mouseButtons(), however the triggered signal is emitted after the mouse is released, so we don't get that reported anymore. Will have to investigate more how this can be fixed. This issue affects middle mouse clicking on any toolbar button. Other than that dolphin seems to work fine.
> 
> The terminal view signals could not be converted since there is no header file available that defines that signal (-> cannot get the function pointer).
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphincontextmenu.cpp f295de7 
>   dolphin/src/dolphinmainwindow.h cb97612 
>   dolphin/src/dolphinmainwindow.cpp 8473014 
>   dolphin/src/dolphinpart.cpp 9081731 
>   dolphin/src/dolphinremoveaction.cpp 7d7c2f0 
>   dolphin/src/dolphinviewcontainer.cpp 768fd5e 
>   dolphin/src/filterbar/filterbar.cpp 6de6fbe 
>   dolphin/src/kitemviews/kfileitemlistview.cpp fd01f2c 
>   dolphin/src/kitemviews/kfileitemmodel.cpp fd773e1 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.h a9e979a 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp 0865d40 
>   dolphin/src/kitemviews/kitemlistcontainer.cpp 8498286 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 7344b99 
>   dolphin/src/kitemviews/kitemlistheader.cpp e89ece0 
>   dolphin/src/kitemviews/kitemlistview.cpp 82f8a20 
>   dolphin/src/kitemviews/kitemlistwidget.cpp 85cd70c 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 9a9a734 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.cpp 65afb7c 
>   dolphin/src/kitemviews/private/kfileitemclipboard.cpp faace2a 
>   dolphin/src/kitemviews/private/kitemlistheaderwidget.cpp 1f210ab 
>   dolphin/src/kitemviews/private/kitemlistroleeditor.cpp 0a48f1b 
>   dolphin/src/kitemviews/private/kitemlistsmoothscroller.cpp 491461b 
>   dolphin/src/kitemviews/private/kitemlistviewanimation.cpp e347c5b 
>   dolphin/src/panels/folders/folderspanel.cpp 46c1b34 
>   dolphin/src/panels/folders/treeviewcontextmenu.cpp fa8844d 
>   dolphin/src/panels/information/informationpanel.cpp eda74f3 
>   dolphin/src/panels/information/informationpanelcontent.cpp b2dd158 
>   dolphin/src/panels/information/phononwidget.cpp a36ada1 
>   dolphin/src/panels/information/pixmapviewer.cpp 8a752c5 
>   dolphin/src/panels/places/placesitem.cpp 41f22cc 
>   dolphin/src/panels/places/placesitemeditdialog.cpp 08c910d 
>   dolphin/src/panels/places/placesitemmodel.cpp baa770d 
>   dolphin/src/panels/places/placespanel.cpp d5308ea 
>   dolphin/src/panels/terminal/terminalpanel.cpp bfd3002 
>   dolphin/src/search/dolphinfacetswidget.cpp b7315a0 
>   dolphin/src/search/dolphinsearchbox.cpp c178c43 
>   dolphin/src/search/filenamesearchprotocol.cpp 4d6f59f 
>   dolphin/src/settings/additionalinfodialog.cpp e9d5f60 
>   dolphin/src/settings/applyviewpropsjob.cpp 4bc77ca 
>   dolphin/src/settings/dolphinsettingsdialog.cpp 609e2ab 
>   dolphin/src/settings/general/behaviorsettingspage.cpp cbbde1d 
>   dolphin/src/settings/general/configurepreviewplugindialog.cpp 3ca08df 
>   dolphin/src/settings/general/confirmationssettingspage.cpp ab23a19 
>   dolphin/src/settings/general/generalsettingspage.cpp 18e1528 
>   dolphin/src/settings/general/previewssettingspage.cpp 38b61b9 
>   dolphin/src/settings/general/statusbarsettingspage.cpp 48622ac 
>   dolphin/src/settings/kcm/kcmdolphingeneral.cpp 26cb580 
>   dolphin/src/settings/kcm/kcmdolphinnavigation.cpp 36345a5 
>   dolphin/src/settings/kcm/kcmdolphinservices.cpp 6d8c761 
>   dolphin/src/settings/kcm/kcmdolphinviewmodes.cpp a7a9db3 
>   dolphin/src/settings/navigation/navigationsettingspage.cpp 8076d70 
>   dolphin/src/settings/serviceitemdelegate.cpp 7538e03 
>   dolphin/src/settings/services/servicessettingspage.cpp 48e816b 
>   dolphin/src/settings/startup/startupsettingspage.cpp 6938263 
>   dolphin/src/settings/trash/trashsettingspage.cpp cd69985 
>   dolphin/src/settings/viewmodes/dolphinfontrequester.cpp 6cb7b99 
>   dolphin/src/settings/viewmodes/viewsettingspage.cpp 4f8a3f0 
>   dolphin/src/settings/viewmodes/viewsettingstab.cpp bc12451 
>   dolphin/src/settings/viewpropertiesdialog.cpp 574f8e1 
>   dolphin/src/settings/viewpropsprogressinfo.cpp 9b7797d 
>   dolphin/src/statusbar/dolphinstatusbar.cpp 671ef4f 
>   dolphin/src/statusbar/statusbarspaceinfo.cpp 61b2833 
>   dolphin/src/views/dolphinnewfilemenuobserver.h 239476e 
>   dolphin/src/views/dolphinnewfilemenuobserver.cpp 7669f15 
>   dolphin/src/views/dolphinremoteencoding.cpp 04b350e 
>   dolphin/src/views/dolphinview.cpp 9f5f48a 
>   dolphin/src/views/dolphinviewactionhandler.h e80ffc0 
>   dolphin/src/views/dolphinviewactionhandler.cpp 48ec95c 
>   dolphin/src/views/renamedialog.cpp d8dbd77 
>   dolphin/src/views/tooltips/filemetadatatooltip.cpp b726996 
>   dolphin/src/views/tooltips/tooltipmanager.cpp bd69483 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 36345d9 
> 
> Diff: https://git.reviewboard.kde.org/r/117478/diff/
> 
> 
> Testing
> -------
> 
> After converting all the signal/slot connections, dolphin seems to work normally.
> 
> I got a crash when moving something to trash, but I think this is because I currently don't have kio-trash on my system.
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140414/91cfc8f7/attachment.htm>