Replaced old connect() with QT5 style. Part 2
ClosedPublic

Authored by mchabrecek on Oct 24 2018, 9:04 AM.

Details

Summary
Summary:
Replacing connect() lines with the new signal/slot syntax in Qt5. Details: https://wiki.qt.io/New_Signal_Slot_Syntax.
There many of them. This is just the second part, more will follow.

Diff Detail

Repository
R167 Krusader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mchabrecek requested review of this revision.Oct 24 2018, 9:04 AM
mchabrecek created this revision.
yurchor added a subscriber: Krusader.

Hello,

I have realized now that used lambdas in two ways, with and without this, examples:
connect(new QShortcut(QKeySequence("Ctrl++"), this), &QShortcut::activated, this, [=]() { zoomIn(); });
connect(diskUsage, &DiskUsage::enteringDirectory, [=]() { DiskUsageViewer::slotUpdateStatus(); });

Even compiler didn't complain, I suppose it should be without this.
What is your opinion?

nmel requested changes to this revision.Oct 27 2018, 7:49 AM
nmel added a subscriber: nmel.

I have realized now that used lambdas in two ways, with and without this, examples:
connect(new QShortcut(QKeySequence("Ctrl++"), this), &QShortcut::activated, this, [=]() { zoomIn(); });
connect(diskUsage, &DiskUsage::enteringDirectory, [=]() { DiskUsageViewer::slotUpdateStatus(); });

Even compiler didn't complain, I suppose it should be without this.
What is your opinion?

Please use the one with this. In case a slot object is destroyed before a signal object, there will be an automatic disconnect.

If you have time, could you please check this pattern in the project and adjust accordingly using a separate CR? Thanks!

krusader/KViewer/diskusageviewer.cpp
55–57

connect(diskUsage, &DiskUsage::enteringDirectory, this, [=]() { slotUpdateStatus(); });

This revision now requires changes to proceed.Oct 27 2018, 7:49 AM
nmel added a comment.Oct 27 2018, 7:51 AM

Also, summary is incorrect:

This is just the fist part, more will follow.

mchabrecek updated this revision to Diff 44407.Oct 29 2018, 8:59 AM
mchabrecek marked an inline comment as done.

Fixed missing this pointer in lamba call.

This comment was removed by mchabrecek.
mchabrecek edited the summary of this revision. (Show Details)Oct 29 2018, 9:07 AM
In D16397#349121, @nmel wrote:

Also, summary is incorrect:

This is just the fist part, more will follow.

I fixed the summary here, not in the commit, is it fine?

Thank you,
Miro

nmel accepted this revision.Nov 1 2018, 4:35 AM

Fixed missing this pointer in lamba call.

You haven't removed DiskUsageViewer:: which is excessive.

In D16397#349121, @nmel wrote:

Also, summary is incorrect:

This is just the fist part, more will follow.

I fixed the summary here, not in the commit, is it fine?

The correct description in the commit is more important, of course. Please fix the commit message content before pushing. Thanks!

This revision is now accepted and ready to land.Nov 1 2018, 4:35 AM
This revision was automatically updated to reflect the committed changes.