Differential D6956

Stop unnecessarily killing IBus when applying keyboard layouts config
ClosedPublic
Actions

Authored by achauvel on Jul 27 2017, 11:13 PM.

Details

Reviewers
xapienz
xuetianweng
Commits
R119:999568ada13b: Stop unnecessarily killing IBus when applying keyboard layouts config
Summary

More details here: https://bugs.kde.org/show_bug.cgi?id=379930

Diff Detail

Repository
R119 Plasma Desktop
Lint
Lint Skipped
Unit
Unit Tests Skipped
achauvel created this revision.Jul 27 2017, 11:13 PM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptJul 27 2017, 11:13 PM
xapienz added a subscriber: xapienz.Jul 28 2017, 6:33 AM

I think it would also be good to remove the line static bool preInitialize(); in kcms/keyboard/xkb_helper.h, because it will be unused.

achauvel updated this revision to Diff 17363.Jul 29 2017, 5:35 PM

The new patch remove declaration of preInitialize() too.

xapienz accepted this revision.Jul 30 2017, 9:39 AM
This revision is now accepted and ready to land.Jul 30 2017, 9:39 AM
xapienz added a comment.Jul 30 2017, 9:42 AM

I'm not sure if I have the right to approve the review (I'm not part of KDE team), but it seems I could do it without any problem.

hein added a subscriber: hein.Jul 31 2017, 8:04 AM

Do you have commit access?

hein added a reviewer: xuetianweng.Jul 31 2017, 8:04 AM

csslayer, can you have a look at this too?

xapienz added a comment.Jul 31 2017, 8:31 AM
In D6956#130369, @hein wrote:

Do you have commit access?

No commit access, only approval access

hein added a comment.Jul 31 2017, 8:36 AM

Your patch looks good to me. I'll await Weng's opinion and then commit it for you.

xuetianweng accepted this revision.Jul 31 2017, 9:07 PM

Yeah, killing input method is not a great idea, not to mention ibus is not the only one messing with keyboard layout. If people really want to know what's messing around I'd rather check the input method's dbus name and throw a warning.

Closed by commit R119:999568ada13b: Stop unnecessarily killing IBus when applying keyboard layouts config (authored by hein). · Explain WhyAug 1 2017, 11:23 AM
This revision was automatically updated to reflect the committed changes.
cfeck added a subscriber: cfeck.Aug 2 2017, 9:56 AM

Eike, next time you commit patches from contributors, please use the --author option in git commit to give proper attribution. Additionally, a bug number was given, but BUG: keyword was missing in the commit message.

hein added a comment.Aug 2 2017, 10:10 AM

I used arc patch which usually preserves authorship, I don't know why it didn't this time - I blame the tool. :)

davidedmundson added a subscriber: davidedmundson.Aug 2 2017, 10:28 AM

It does when uploaded with arc diff, it doesn't otherwise.

Revision Contents
Changeset List

Diff 17363

View Options

kcms/keyboard/keyboard_daemon.cpp

Loading...
View Options

kcms/keyboard/xkb_helper.h

Loading...
View Options

kcms/keyboard/xkb_helper.cpp

Loading...