GNOME Bugzilla – Bug 736435
Implement input source switching
Last modified: 2014-09-11 17:36:52 UTC
These patches move the input source handling that currently happens in gnome-settings-daemon into gnome-shell so that we can make it work when running as a Wayland compositor using the same logic as when running as an X compositor.
Created attachment 285851 [details] [review] status/keyboard: Move IBusManager into its own file This also makes it a singleton for easier access from multiple places.
Created attachment 285852 [details] [review] status/keyboard: Factor out a KeyboardManager class This allows us to have different implementations for X11 and Wayland. For now, the Wayland implementation just blindly sets a layout using a mutter API.
Created attachment 285853 [details] [review] status/keyboard: Factor out an InputSourceManager class
Created attachment 285854 [details] [review] status/keyboard: Move all UI elements to the indicator class This will allow us to have multiple indicator instances, e.g. show a indicator next to text entries in modal shell dialogs.
Created attachment 285855 [details] [review] ibusManager: Spawn ibus-daemon gnome-settings-daemon doesn't this for us anymore.
Created attachment 285856 [details] [review] Implement input source switching Instead of calling out into gnome-settings-manager we'll just implement the switching logic ourselves and use mutter APIs that allow this functionality to work both in X sessions and when we're a Wayland compositor. Switching IBus engines is done transparently as well just like g-s-d used to do.
Review of attachment 285855 [details] [review]: Ugh. Can this not be DBus-activated?
Review of attachment 285853 [details] [review]: Sure.
Review of attachment 285851 [details] [review]: Sure.
Review of attachment 285854 [details] [review]: Sure. Seems unrelated, but sure.
Review of attachment 285856 [details] [review]: "gnome-settings-manager" should probably be "gnome-settings-daemon". Can you replace the earlier patch in the stack which introduces KeyboardManager with this one? ::: js/misc/keyboardManager.js @@ +141,3 @@ + + if (group.length > 0) { + layouts = group[0].layout; I'd do this as: let layouts = group.map(function(g) { return g.layout; }).join(','); let variants = ... @@ +164,3 @@ + options = this._xkbOptions[0]; + for (let i = 1; i < this._xkbOptions.length; ++i) + options += ',' + this._xkbOptions[i]; this._xkbOptions.join(',')
Review of attachment 285852 [details] [review]: We don't use any of this patch, so just get rid of it.
Created attachment 285922 [details] [review] status/keyboard: Factor out a KeyboardManager class This code will grow in a forthcoming patch so let's move it out. -- I didn't completly remove the patch since it creates some conflicts with the patches ahead of it but I removed the spurious stuff that was being removed in the last patch before.
Created attachment 285923 [details] [review] ibusManager: Spawn ibus-daemon gnome-settings-daemon doesn't this for us anymore. Note that ibus-daemon isn't DBus activatable but just spawning it is fine because it does its own single instance management. The library notifies us when it shows up and goes away through the connected and disconnected signals. -- Expanded the commit message.
Created attachment 285924 [details] [review] Implement input source switching Instead of calling out to gnome-settings-daemon we'll just implement the switching logic ourselves and use mutter APIs that allow this functionality to work both in X sessions and when we're a Wayland compositor. Switching IBus engines is done transparently as well just like g-s-d used to do. -- Rebased and fixed according to the review.
Review of attachment 285923 [details] [review]: I still really don't like it, but whatever. We can fix this in the future.
Review of attachment 285924 [details] [review]: Much better.
Review of attachment 285922 [details] [review]: Yep.
Attachment 285851 [details] pushed as 58aabfc - status/keyboard: Move IBusManager into its own file Attachment 285853 [details] pushed as effe6fa - status/keyboard: Factor out an InputSourceManager class Attachment 285854 [details] pushed as a0a7017 - status/keyboard: Move all UI elements to the indicator class Attachment 285922 [details] pushed as 8e560f9 - status/keyboard: Factor out a KeyboardManager class Attachment 285923 [details] pushed as 6a36a68 - ibusManager: Spawn ibus-daemon Attachment 285924 [details] pushed as 8589bfb - Implement input source switching