Differential D9881

Replace deprecated QSignalMapper by lambda expressions
ClosedPublic
Actions

Authored by croick on Jan 14 2018, 10:03 PM.

Details

Reviewers

mwolff

Group Reviewers

KDevelop

Commits

R32:90a36b8b1e11: Replace deprecated QSignalMapper by lambda expressions

Summary

QSignalMapper can mostly be replaced using straightforward lambdas, reducing the code size

Test Plan

switch scope and arrangement of problems in ProblemView
create an application using the AppWizard
add a compiler
switch representation of register variables
add code to an existing file using a file template
open a file with an external application
switch to header/source file using the context menu action
track changes of a variable in the debugger
status bar seems to work
untested: runcontroller

Diff Detail

Repository

R32 KDevelop

Branch

deprSignalMapper

Lint

No Linters Available

Unit

No Unit Test Coverage

croick created this revision.Jan 14 2018, 10:03 PM

Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJan 14 2018, 10:03 PM

croick requested review of this revision.Jan 14 2018, 10:03 PM

All in all, LGTM. +1

plugins/debuggercommon/midebuggerplugin.cpp
88 ↗	(On Diff #25353)	Shouldn't this line go to the end of the file?

move moc inclusion to the end of the file

croick marked an inline comment as done.Jan 16 2018, 8:47 PM

croick added inline comments.

plugins/debuggercommon/midebuggerplugin.cpp
88 ↗	(On Diff #25353)	That's probably more common, thanks.

excellent work! some nits, otherwise lgtm

plugins/debuggercommon/midebuggerplugin.cpp
64	style: remove space before &
70	dito
151	the old code was odd here already, but shouldn't you first check whether the service is already known and a proxy exists? or at least ensure that its freed by calling `slotDBusServiceUnregistered` also, add a comment here please saying that we use the proxy to map N services to the one slot that takes the proxy. Finally, can't we just use old-style connect + `sender()` here to simplify the code? Yes, `sender()` isn't the nicest thing on earth, but it certainly is less code than this here.
plugins/debuggercommon/registers/registersview.cpp
244	why not capture a and then call `a->text()` in the lambda?

This revision now requires changes to proceed.Jan 25 2018, 8:59 AM

croick marked an inline comment as done.Jan 26 2018, 12:41 AM

croick added inline comments.

plugins/debuggercommon/midebuggerplugin.cpp
151	Actually, since I didn't know what the thing is supposed to do, I checked the DrKonqi side of the code: The whole thing is never called, since DrKonqi never starts a service called "org.kde.drkonqi". There is a service foreseen called "org.kde.Krash", but it is not registered to the DBus. So I think, I will drop those changes completely for now and fix this independently within DrKonqi and KDevelop in separate patches if you agree.
plugins/debuggercommon/registers/registersview.cpp
244	Sure, if there is no `a`, there is no connection. Thanks.

you forgot to upload the new version, no?

directly access QAction inside lambda
revert changes to MIDebuggerPlugin for postponed changes to DBus connection

In D9881#198116, @mwolff wrote:

you forgot to upload the new version, no?

I first wanted to evaluate if it makes sense to drop the changes here. But it does and I will propose that patch soon.

May this go in then? The problematic files became part of the other commit.

sorry for the delay, this lgtm - thanks!

This revision is now accepted and ready to land.Mar 14 2018, 8:00 AM

Closed by commit R32:90a36b8b1e11: Replace deprecated QSignalMapper by lambda expressions (authored by croick). · Explain WhyMar 14 2018, 10:37 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			kdevplatform/debugger/variable/variabletooltip.cpp (11 lines)
M			kdevplatform/debugger/variable/variablewidget.h (3 lines)
M			kdevplatform/debugger/variable/variablewidget.cpp (6 lines)
M			kdevplatform/shell/problemmodel.h (4 lines)
M			kdevplatform/shell/runcontroller.cpp (15 lines)
M			kdevplatform/shell/statusbar.h (2 lines)
M			kdevplatform/shell/statusbar.cpp (7 lines)
M			plugins/appwizard/appwizarddialog.h (3 lines)
M			plugins/appwizard/appwizarddialog.cpp (20 lines)
M			plugins/custom-definesandincludes/compilerprovider/widget/compilerswidget.h (2 lines)
M			plugins/custom-definesandincludes/compilerprovider/widget/compilerswidget.cpp (10 lines)
M			plugins/debuggercommon/midebuggerplugin.h (8 lines)
M			plugins/debuggercommon/midebuggerplugin.cpp (61 lines)
M			plugins/debuggercommon/registers/registersview.h (2 lines)
M			plugins/debuggercommon/registers/registersview.cpp (9 lines)
M			plugins/filetemplates/outputpage.cpp (15 lines)
M			plugins/openwith/openwithplugin.h (3 lines)
M			plugins/openwith/openwithplugin.cpp (12 lines)
M			plugins/problemreporter/problemsview.cpp (50 lines)
M			plugins/switchtobuddy/switchtobuddyplugin.h (3 lines)
M			plugins/switchtobuddy/switchtobuddyplugin.cpp (12 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	25353	71f7bed		Jan 14 2018, 10:03 PM	★	★
Diff 2	25506	71f7bed	- move moc inclusion to the end of the file	Jan 16 2018, 8:44 PM	★	★
Diff 3	26225	1599bbe		Jan 30 2018, 9:00 PM	★	★
Diff 4	29543	cc99e43	R32:90a36b8b1e11cdc811558146944d0e310e2724b0	Mar 14 2018, 10:36 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
edff061a17fe	b1f8c391f42f	0ac2207626b8	Christoph Roick	move moc inclusion to the end of the file	Jan 16 2018, 8:42 PM
0ac2207626b8	13d56957c038	71f7bed96dd0	Christoph Roick	Replace deprecated QSignalMapper by lambda expressions (Show More…)	Jan 14 2018, 10:01 PM