Differential D18204

Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui
ClosedPublic
Actions

Authored by aacid on Jan 12 2019, 12:33 AM.

Details

Reviewers
anthonyfieroni
Commits
R263:02b523bad09a: Make KCheckAccelerators less invasive for apps that don't directly link to…
Summary

BUGS: 337491

Test Plan

Ran qtcreator and it no longer has the & in Details
Added some debug and checked that okular still gets the code called

Diff Detail

Repository
R263 KXmlGui
Branch
arcpatch-D18204
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7003
Build 7021: arc lint + arc unit
aacid created this revision.Jan 12 2019, 12:33 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 12 2019, 12:33 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
aacid requested review of this revision.Jan 12 2019, 12:33 AM
anthonyfieroni added a subscriber: anthonyfieroni.Jan 12 2019, 5:24 AM
anthonyfieroni added inline comments.
src/kcheckaccelerators.cpp
97 ↗(On Diff #49308)

That's static member, you don't need instance. BTW can we check only for null app, i mean if it's not directly linked library qapp will be null, it's not checked.

davidedmundson added a subscriber: davidedmundson.Jan 12 2019, 8:07 AM

That's bloody clever!
+1

aacid updated this revision to Diff 49319.Jan 12 2019, 11:24 AM

Review comments update

anthonyfieroni added inline comments.Jan 12 2019, 1:02 PM
src/kcheckaccelerators.cpp
87

We can cheet here

if (!doCheckAccelerators) {
    return;
}
114

Don't need check :)

aacid updated this revision to Diff 49340.Jan 12 2019, 5:32 PM

Change style to make Anthony happy

anthonyfieroni accepted this revision.Jan 12 2019, 5:37 PM
This revision is now accepted and ready to land.Jan 12 2019, 5:37 PM
Closed by commit R263:02b523bad09a: Make KCheckAccelerators less invasive for apps that don't directly link to… (authored by aacid). · Explain WhyJan 12 2019, 5:39 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/kcheckaccelerators.cpp (40 lines)

Diff 49319

View Options

src/kcheckaccelerators.cpp

Loading...