Merge lp:~tombeckmann/switchboard/cli-open-fixes into lp:~elementary-pantheon/switchboard/switchboard

Proposed by Tom Beckmann
Status: Merged
Approved by: Cody Garver
Approved revision: 462
Merged at revision: 459
Proposed branch: lp:~tombeckmann/switchboard/cli-open-fixes
Merge into: lp:~elementary-pantheon/switchboard/switchboard
Diff against target: 102 lines (+46/-3)
2 files modified
CMakeLists.txt (+3/-1)
src/Switchboard.vala (+43/-2)
To merge this branch: bzr merge lp:~tombeckmann/switchboard/cli-open-fixes
Reviewer Review Type Date Requested Status
Corentin Noël Approve
Sergey "Shnatsel" Davidoff (community) Abstain
Review via email: mp+225763@code.launchpad.net

Commit message

Initialize cheese-gtk before gtk, required for GNOME Control Center compatibility layer

Description of the change

This fixes the crash when opening a gcc plug via commandline, it was caused by the user-accounts plug requiring cheese-gtk to be initialized before gtk. We fix this by initializing cheese-gtk for it. A different solution would have been to have all plugins initialize themselves when switchboard starts, but this approach appears easier and better for performance.

The branch also introduces a translation from gcc plug names to switchboard plug names. The names are taken directly from gcc's source, so they are definitely correct and we can easily swap out our plugs if we replace gcc ones. As shnatsel already mentioned here https://bugs.launchpad.net/elementaryos/+bug/1291738/comments/5 the app that is calling switchboard shouldn't be concerned whether the plug that is opened is native or from gcc, this translation layer allows this behavior. A simple "switchboard sound" or "switchboard keyboard" will now open the corresponding plug, instead of a "switchboard -o hardware-gcc-sound" for example (but it continues to work of course), which would break for all third-parties using this, once we get a sound plug ourselves, as the "gcc" part would be replaced with "pantheon". So translating this centrally in switchboard is definitely the best option.

The gnome-control-center replacement script can now be reduced to basically this http://pastebin.com/BUTPJBNF which appears to work perfectly, including support for additional flags like --debug. I can propose a merge for this when this branch is accepted.

To post a comment you must log in.
460. By Tom Beckmann

fix whitespaces

Revision history for this message
Tom Beckmann (tombeckmann) wrote :

Oups, accidentally moved the translate function in the HAVE_UNITY conditional block. Will fix that once I get back to my computer in around 8h.

461. By Tom Beckmann

move translation method out of HAVE_UNITY block

Revision history for this message
Tom Beckmann (tombeckmann) wrote :

As ricotz pointed out: https://git.gnome.org/browse/gnome-control-center/commit/?id=45d4944b6db031dbc77e515bdae0fac9dbdba50f Initializing cheese-gtk is apparently the way it's supposed to work now.

I fixed the HAVE_UNITY issue now.

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

I'm not a fan of the way translation from short names like "sound" to fully-qualified names is done currently. As far as I understand this will require us to manually track upstream G-C-C plug list changes, which we would of course would like to avoid.

What I tried to do in BASH (and gave up) is to enumerate all existing plugs and then choose the appropriate one automatically, depending on the environment. So if I have e.g. both hardware-pantheon-sound and hardware-gcc-sound and a hypothetical hardware-xfce-sound installed and run "switchboard sound", it should infer the DE from XDG_CURRENT_DESKTOP environment variable (e.g. launch pantheon-sound in pantheon, gcc-sound in GNOME, etc) and fall back to pantheon plugs if no match is found.

I've tried to code that as a BASH wrapper and I could probably even do it if somebody explains me how to find all available plugs. Although working with XDG_DATA_DIRS is complicated in BASH, so I'd prefer to have it in Vala.

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) :
review: Abstain
Revision history for this message
Corentin Noël (tintou) wrote :

Can you please add a note before the cheese-gtk dependency in CMake that we remember that it is needed by the gcc-compatibility-layer.
While this patch will work for Freya, I really want to push a new standard for settings opening (see https://docs.google.com/a/elementaryos.org/document/d/1N0uqNtVXEFn3cLgNMeN75mP_dpMpCco-7uw5PKow-_Q/edit?usp=sharing )

462. By Tom Beckmann

add note to cmake about cheese dependency

Revision history for this message
Corentin Noël (tintou) wrote :

Okay, let's go ahead.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2014-03-17 19:18:13 +0000
3+++ CMakeLists.txt 2014-07-08 15:13:36 +0000
4@@ -95,6 +95,8 @@
5 gmodule-2.0
6 gtk+-3.0>=3.10
7 gio-unix-2.0
8+ # cheese is required for the GCC panels to work
9+ cheese-gtk
10 )
11 find_package(PkgConfig)
12
13@@ -140,4 +142,4 @@
14 endif ()
15
16 add_subdirectory (schemas)
17-install (FILES ${CMAKE_CURRENT_SOURCE_DIR}/schemas/org.pantheon.switchboard.gschema.xml DESTINATION ${GSETTINGSDIR})
18\ No newline at end of file
19+install (FILES ${CMAKE_CURRENT_SOURCE_DIR}/schemas/org.pantheon.switchboard.gschema.xml DESTINATION ${GSETTINGSDIR})
20
21=== modified file 'src/Switchboard.vala'
22--- src/Switchboard.vala 2014-05-27 05:48:46 +0000
23+++ src/Switchboard.vala 2014-07-08 15:13:36 +0000
24@@ -17,6 +17,8 @@
25
26 namespace Switchboard {
27
28+ [CCode (cname = "cheese_gtk_init")]
29+ public extern static bool cheese_gtk_init ([CCode (array_length_pos = 0.9)] ref unowned string[] argv);
30
31 public enum WindowState {
32 NORMAL = 0,
33@@ -24,7 +26,11 @@
34 }
35
36 public static int main (string[] args) {
37- Gtk.init (ref args);
38+
39+ // FIXME the gcc user-accounts plug requires cheese to be initialized before gtk,
40+ // otherwise it will crash switchboard if it's loaded before the window and
41+ // all its widgets are displayed.
42+ cheese_gtk_init (ref args);
43
44 var app = new SwitchboardApp ();
45 return app.run (args);
46@@ -50,7 +56,7 @@
47 private int default_width = 0;
48 private int default_height = 0;
49
50- private static string? plug_to_open = null;
51+ private static string? plug_to_open = null;
52 private static bool should_animate_next_transition = true;
53
54 static const OptionEntry[] entries = {
55@@ -94,6 +100,11 @@
56 try {
57 unowned string[] tmp = args;
58 context.parse (ref tmp);
59+
60+ // we have an unparsed argument. Assume that it's a gcc plug name
61+ if (tmp.length > 1) {
62+ plug_to_open = gcc_to_switchboard_code_name (tmp[1]);
63+ }
64 } catch (Error e) {
65 warning (e.message);
66 return 0;
67@@ -439,6 +450,36 @@
68 return (empty ? null : category_item);
69 }
70 #endif
71+
72+ private string? gcc_to_switchboard_code_name (string gcc_name) {
73+ // list of names taken from GCC's shell/cc-panel-loader.c
74+ switch (gcc_name) {
75+ case "background": return "pantheon-desktop";
76+ case "bluetooth": return "network-gcc-bluetooth";
77+ case "color": return "hardware-gcc-color";
78+ case "datetime": return "system-gcc-date";
79+ case "display": return "hardware-gcc-display";
80+ case "info": return "system-pantheon-about";
81+ case "keyboard": return "hardware-pantheon-keyboard";
82+ case "network": return "network-gcc-network";
83+ case "power": return "system-pantheon-power";
84+ case "printers": return "hardware-gcc-printer";
85+ case "privacy": return "pantheon-security-privacy";
86+ case "region": return "hardware-gcc-region";
87+ case "sound": return "hardware-gcc-sound";
88+ case "universal-access": return "system-gcc-universalaccess";
89+ case "user-accounts": return "hardware-gcc-user";
90+ case "wacom": return "hardware-gcc-wacom";
91+
92+ // not available on our system
93+ case "notifications":
94+ case "search":
95+ case "sharing":
96+ return null;
97+ }
98+
99+ return null;
100+ }
101 }
102 }
103

Subscribers

People subscribed via source and target branches

to all changes: