Merge lp:~elementary-apps/switchboard/cleanup into lp:~elementary-pantheon/switchboard/switchboard

Proposed by Raphael Isemann
Status: Rejected
Rejected by: Julián Unrrein
Proposed branch: lp:~elementary-apps/switchboard/cleanup
Merge into: lp:~elementary-pantheon/switchboard/switchboard
Diff against target: 577 lines (+148/-120)
3 files modified
src/CategoryView.vala (+1/-1)
src/EmbeddedAlert.vala (+1/-1)
src/Switchboard.vala (+146/-118)
To merge this branch: bzr merge lp:~elementary-apps/switchboard/cleanup
Reviewer Review Type Date Requested Status
Julián Unrrein (community) Disapprove
David Gomes (community) Needs Fixing
Review via email: mp+188034@code.launchpad.net

Description of the change

Fixing codestyle.

To post a comment you must log in.
Revision history for this message
Julián Unrrein (junrrein) wrote :

Code style

Diff lines 244-245, 262, 265, 310, 445: Put the blocks of code in separate lines from the "try" and "catch" statements:

try {
    //...
} catch (Error e) {
    //...
}

Diff line 303: A space after "else if" and before the parenthesis.

Diff line 320: A space on the other side of the parenthesis :)

Diff line 380: A space before the opening parenthesis.

Diff line 450: A space after the opening curly brace.

Revision history for this message
Julián Unrrein (junrrein) wrote :

I would offer to submit the changes myself (since I'm in ~elementary-apps too) but I am not able to compile this on Luna (GTK >= 3.6 required).

406. By David Gomes

More fixes.

Revision history for this message
David Gomes (davidgomes) wrote :

Junrrein, I fixed the stuff. Care to re-review?

review: Needs Fixing
Revision history for this message
Julián Unrrein (junrrein) wrote :

Re-indent diff lines 150, 153, 163-164, 391, they are part of foreach blocks.

You can close lines 163-164 in curly braces for clarity.

Diff line 225: Move the warning to its own line.

Diff line 373: Space after get_file_type.

Diff line 475: Space after search_box_activated.

I see you also fixed stuff I didn't catch, nice!

review: Needs Fixing
407. By David Gomes

Fixed yet more code style.

Revision history for this message
Julián Unrrein (junrrein) wrote :

It's ready to go.

Good work Raphael and Munchor!

review: Approve
Revision history for this message
Julián Unrrein (junrrein) wrote :

Rejecting this branch because there are major changes coming to Switchboard (see https://code.launchpad.net/~elementary-apps/switchboard/libpeas-isis/+merge/195417).

We should review that branch for code style later instead.

review: Disapprove

Unmerged revisions

407. By David Gomes

Fixed yet more code style.

406. By David Gomes

More fixes.

405. By Raphael Isemann

cleaning CategoryView.vala

404. By Raphael Isemann

cleaning EmbeddedAlert.vala

403. By Raphael Isemann

cleaning Switchboard.vala

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/CategoryView.vala'
2--- src/CategoryView.vala 2013-09-29 19:41:03 +0000
3+++ src/CategoryView.vala 2013-12-03 21:13:09 +0000
4@@ -53,7 +53,7 @@
5 category_label.margin_left = 12;
6 category_label.margin_right = 8;
7 var filtered = new Gtk.TreeModelFilter (store, null);
8- filtered.set_visible_column(3);
9+ filtered.set_visible_column (3);
10 filtered.refilter ();
11
12 var category_plugs = new Gtk.IconView.with_model (filtered);
13
14=== modified file 'src/EmbeddedAlert.vala'
15--- src/EmbeddedAlert.vala 2013-01-08 15:21:51 +0000
16+++ src/EmbeddedAlert.vala 2013-12-03 21:13:09 +0000
17@@ -113,7 +113,7 @@
18 primary_text_label.halign = secondary_text_label.halign = Gtk.Align.START;
19 primary_text_label.justify = Gtk.Justification.LEFT;
20 secondary_text_label.justify = Gtk.Justification.FILL;
21- image.set_pixel_size(64);
22+ image.set_pixel_size (64);
23
24 // TODO: More intelligent icon support, I guess. Or support all the
25 // MessageType flags.
26
27=== modified file 'src/Switchboard.vala'
28--- src/Switchboard.vala 2013-09-26 08:05:15 +0000
29+++ src/Switchboard.vala 2013-12-03 21:13:09 +0000
30@@ -1,18 +1,18 @@
31 /***
32-BEGIN LICENSE
33-Copyright (C) 2011-2012 Avi Romanoff <aviromanoff@gmail.com>
34-This program is free software: you can redistribute it and/or modify it
35-under the terms of the GNU Lesser General Public License version 3, as published
36-by the Free Software Foundation.
37-
38-This program is distributed in the hope that it will be useful, but
39-WITHOUT ANY WARRANTY; without even the implied warranties of
40-MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
41-PURPOSE. See the GNU General Public License for more details.
42-
43-You should have received a copy of the GNU General Public License along
44-with this program. If not, see <http://www.gnu.org/licenses/>.
45-END LICENSE
46+ BEGIN LICENSE
47+ Copyright (C) 2011-2012 Avi Romanoff <aviromanoff@gmail.com>
48+ This program is free software: you can redistribute it and/or modify it
49+ under the terms of the GNU Lesser General Public License version 3, as published
50+ by the Free Software Foundation.
51+
52+ This program is distributed in the hope that it will be useful, but
53+ WITHOUT ANY WARRANTY; without even the implied warranties of
54+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
55+ PURPOSE. See the GNU General Public License for more details.
56+
57+ You should have received a copy of the GNU General Public License along
58+ with this program. If not, see <http://www.gnu.org/licenses/>.
59+ END LICENSE
60 ***/
61
62 namespace Switchboard {
63@@ -21,13 +21,13 @@
64
65 [DBus (name = "org.elementary.switchboard")]
66 public class SwitchboardApp : Granite.Application {
67-
68+
69 construct {
70 application_id = "org.elementary.Switchboard";
71 program_name = "Switchboard";
72 app_years = "2011-2012";
73 exec_name = "switchboard";
74- app_launcher = exec_name+".desktop";
75+ app_launcher = exec_name + ".desktop";
76
77 build_version = VERSION;
78 app_icon = APP_ICON;
79@@ -51,9 +51,9 @@
80 // These two wrap the progress bar
81 Gtk.ToolItem lspace = new Gtk.ToolItem ();
82 Gtk.ToolItem rspace = new Gtk.ToolItem ();
83-
84+
85 Gtk.Spinner loading;
86-
87+
88 Gtk.Window main_window;
89
90 // Content area widgets
91@@ -65,12 +65,12 @@
92
93 // Plug data
94 bool socket_shown;
95- Gee.HashMap<string, string> current_plug = new Gee.HashMap<string, string>();
96+ Gee.HashMap<string, string> current_plug = new Gee.HashMap<string, string> ();
97 Gee.HashMap<string, string>[] plugs;
98
99 string[] plug_places = {"/usr/share/plugs/",
100 "/usr/lib/plugs/",
101- "/usr/local/share/plugs/",
102+ "/usr/local/share/plugs/",
103 "/usr/local/lib/plugs/"};
104 string search_box_buffer = "";
105
106@@ -78,9 +78,9 @@
107
108 public SwitchboardApp () {
109 }
110-
111+
112 void build () {
113- main_window = new Gtk.Window();
114+ main_window = new Gtk.Window ();
115
116 // Set up defaults
117 main_window.title = APP_TITLE;
118@@ -95,8 +95,8 @@
119
120 // Set up socket
121 socket = new Gtk.Socket ();
122- socket.set_hexpand(true);
123- socket.set_vexpand(true);
124+ socket.set_hexpand (true);
125+ socket.set_vexpand (true);
126
127 socket.plug_removed.connect (() => {
128 plug_closed ();
129@@ -108,11 +108,12 @@
130 uint accel_key;
131 Gdk.ModifierType accel_mod;
132 var accel_flags = Gtk.AccelFlags.LOCKED;
133- Gtk.accelerator_parse("<Control>q", out accel_key, out accel_mod);
134- accel_group.connect(accel_key, accel_mod, accel_flags, () => {
135- main_window.destroy();
136+ Gtk.accelerator_parse ("<Control>q", out accel_key, out accel_mod);
137+ accel_group.connect (accel_key, accel_mod, accel_flags, () => {
138+ main_window.destroy ();
139 return true;
140 });
141+
142 main_window.add_accel_group (accel_group);
143
144 // ??? Why?
145@@ -151,6 +152,7 @@
146
147 foreach (var label in category_view.category_labels.values)
148 label.hide ();
149+
150 foreach (var view in category_view.category_views.values)
151 view.hide ();
152
153@@ -178,17 +180,19 @@
154 }
155 });
156
157- foreach (string place in plug_places)
158- if (enumerate_plugs (place))
159+ foreach (string place in plug_places) {
160+ if (enumerate_plugs (place)) {
161 any_plugs = true;
162+ }
163+ }
164
165 if (!any_plugs) {
166- show_alert(_("No settings found"), _("Install some and re-launch Switchboard"), Gtk.MessageType.WARNING);
167+ show_alert (_("No settings found"), _("Install some and re-launch Switchboard"), Gtk.MessageType.WARNING);
168 search_box.sensitive = false;
169 } else {
170 update_libunity_quicklist ();
171 }
172-
173+
174 bool found = false;
175 if (plug_to_open != null) {
176 foreach (var plug in plugs) {
177@@ -201,17 +205,17 @@
178 critical ("Couldn't find %s among the loaded settings.", plug_to_open);
179 }
180 }
181-
182+
183 foreach (var store in category_view.category_store.values) {
184 store.foreach ((model, path, iter) => {
185 store.set_value (iter, 3, true);
186 return false;
187 });
188 }
189-
190+
191 progress_toolitem.hide ();
192 }
193-
194+
195 void shut_down () {
196 plug_closed ();
197 }
198@@ -234,28 +238,30 @@
199 if (current_plug["title"] != title || !socket.visible) {
200 try {
201 // The plug is already selected
202- debug(_("Closing plug \"%s\" in Switchboard controller..."), current_plug["title"]);
203+ debug (_("Closing plug \"%s\" in Switchboard controller..."), current_plug["title"]);
204 plug_closed ();
205-
206+
207 string[] cmd_exploded = (executable!=null)?executable.split (" "):null;
208 GLib.Process.spawn_async (File.new_for_path (cmd_exploded[0]).get_parent ().
209- get_path (), cmd_exploded, null, SpawnFlags.SEARCH_PATH, null, null);
210-
211+ get_path (), cmd_exploded, null, SpawnFlags.SEARCH_PATH, null, null);
212+
213 // ensure the button is sensitive; it might be the first plug loaded
214 if (!@extern) {
215- navigation_button.set_sensitive(true);
216+ navigation_button.set_sensitive (true);
217 navigation_button.set_icon_name ("go-home");
218 current_plug["title"] = title;
219 current_plug["executable"] = executable;
220 switch_to_socket ();
221 main_window.title = @"$APP_TITLE - $title";
222 }
223- } catch { warning(_("Failed to launch plug: title %s | executable %s"), title, executable); }
224+ } catch {
225+ warning (_("Failed to launch plug: title %s | executable %s"), title, executable);
226+ }
227 } else {
228- navigation_button.set_sensitive(true);
229+ navigation_button.set_sensitive (true);
230 navigation_button.set_icon_name ("go-home");
231 }
232-
233+
234 if (@extern) {
235 switch_to_icons ();
236 }
237@@ -270,7 +276,7 @@
238 void handle_navigation_button_clicked () {
239 string icon_name = navigation_button.get_icon_name ();
240 if (icon_name == "go-home") {
241- switch_to_icons();
242+ switch_to_icons ();
243 navigation_button.set_icon_name ("go-previous");
244 }
245 else {
246@@ -289,25 +295,25 @@
247 socket.hide ();
248 loading.show_all ();
249 }
250-
251+
252 // Switches back to the icons
253 bool switch_to_icons () {
254 socket.hide ();
255 loading.hide ();
256 category_view.show ();
257-
258+
259 socket_shown = false;
260
261 // Reset state
262 reset_title ();
263- search_box.set_text("");
264- search_box.sensitive = count_plugs() > 0;
265- progress_label.set_text("");
266+ search_box.set_text ("");
267+ search_box.sensitive = count_plugs () > 0;
268+ progress_label.set_text ("");
269 progress_bar.fraction = 0.0;
270 progress_toolitem.visible = false;
271-
272+
273 plug_closed ();
274-
275+
276 return true;
277 }
278
279@@ -318,19 +324,24 @@
280
281 // <keyfile's absolute path, keyfile's directory>
282 List<string> keyfiles = find_plugs (plug_root_dir);
283- if (keyfiles.length() == 0) {
284+ if (keyfiles.length () == 0) {
285 return false;
286 } else {
287 foreach (string keyfile in keyfiles) {
288- KeyFile kf = new KeyFile();
289+ KeyFile kf = new KeyFile ();
290
291- string head = File.new_for_path(keyfile).get_basename();
292- string parent = File.new_for_path(keyfile).get_parent().get_path();
293+ string head = File.new_for_path (keyfile).get_basename ();
294+ string parent = File.new_for_path (keyfile).get_parent ().get_path ();
295
296 Gee.HashMap<string, string> plug = new Gee.HashMap<string, string> ();
297- try { kf.load_from_file(keyfile, KeyFileFlags.NONE);
298- } catch (Error e) { warning("Couldn't load this keyfile, %s (path: %s)", e.message, keyfile); }
299- plug["id"] = kf.get_start_group();
300+ try {
301+ kf.load_from_file (keyfile, KeyFileFlags.NONE);
302+ } catch (Error e) {
303+ warning ("Couldn't load this keyfile, %s (path: %s)", e.message, keyfile);
304+ }
305+
306+ plug["id"] = kf.get_start_group ();
307+
308 try {
309 var exec = kf.get_string (head, "exec");
310 //if a path starts with a double slash, we take it as an absolute path
311@@ -338,14 +349,21 @@
312 exec = exec.substring (1);
313 plug["extern"] = "1";
314 } else {
315- exec = Path.build_filename(parent, exec);
316+ exec = Path.build_filename (parent, exec);
317 plug["extern"] = "0";
318 }
319-
320+
321 plug["exec"] = exec;
322- } catch (Error e) { warning("Couldn't read exec field in file %s, %s", keyfile, e.message); }
323- try { plug["icon"] = kf.get_string (head, "icon");
324- } catch (Error e) { warning("Couldn't read icon field in file %s, %s", keyfile, e.message); }
325+ } catch (Error e) {
326+ warning ("Couldn't read exec field in file %s, %s", keyfile, e.message);
327+ }
328+
329+ try {
330+ plug["icon"] = kf.get_string (head, "icon");
331+ } catch (Error e) {
332+ warning ("Couldn't read icon field in file %s, %s", keyfile, e.message);
333+ }
334+
335 try {
336 plug["title"] = kf.get_locale_string (head, "title");
337 string? textdomain = null;
338@@ -371,7 +389,7 @@
339
340 // Checks if the file is a .plug file
341 bool is_plug_file (string filename) {
342- return (filename.down().has_suffix(".plug"));
343+ return (filename.down ().has_suffix (".plug"));
344 }
345
346 // Find all .plug files
347@@ -382,10 +400,10 @@
348 } else {
349 keyfiles = new List<string> ();
350 foreach(var keyfile in keyfiles_list) {
351- keyfiles.append(keyfile);
352+ keyfiles.append (keyfile);
353 }
354 }
355-
356+
357 var directory = File.new_for_path (path);
358 if (!directory.query_exists ()) {
359 return null;
360@@ -395,30 +413,33 @@
361 FileAttribute.STANDARD_NAME + "," + FileAttribute.STANDARD_TYPE, 0);
362 FileInfo file_info;
363 while ((file_info = enumerator.next_file ()) != null) {
364- string file_path = Path.build_filename(path, file_info.get_name());
365- if (file_info.get_file_type() == GLib.FileType.REGULAR &&
366- is_plug_file(file_info.get_name())) {
367- keyfiles.append(file_path);
368- } else if(file_info.get_file_type() == GLib.FileType.DIRECTORY) {
369- keyfiles = find_plugs(file_path, keyfiles);
370+ string file_path = Path.build_filename (path, file_info.get_name ());
371+ if (file_info.get_file_type () == GLib.FileType.REGULAR &&
372+ is_plug_file (file_info.get_name ())) {
373+ keyfiles.append (file_path);
374+ } else if (file_info.get_file_type () == GLib.FileType.DIRECTORY) {
375+ keyfiles = find_plugs (file_path, keyfiles);
376 }
377 }
378- } catch { warning(_(@"Unable to iterate over enumerated plug directory \"$path\"'s contents")); }
379-
380+ } catch {
381+ warning (_(@"Unable to iterate over enumerated plug directory \"$path\"'s contents"));
382+ }
383+
384 return keyfiles;
385 }
386
387 // Counts how many plugs exist at the moment
388 int count_plugs () {
389-
390 uint count = 0;
391+
392 foreach (string place in plug_places)
393 count += find_plugs (place).length ();
394+
395 return (int) count;
396 }
397
398 /*
399- D-Bus ONLY methods
400+ D-Bus ONLY methods
401 */
402
403 public int get_socket_wid () {
404@@ -430,12 +451,12 @@
405
406 public void progress_bar_set_visible (bool visibility) {
407
408- progress_toolitem.set_visible(visibility);
409+ progress_toolitem.set_visible (visibility);
410 }
411
412 public void progress_bar_set_text (string text) {
413
414- progress_label.set_text(text);
415+ progress_label.set_text (text);
416 }
417
418 public void progress_bar_set_fraction (double fraction) {
419@@ -445,7 +466,7 @@
420
421 public void progress_bar_pulse () {
422
423- progress_bar.pulse();
424+ progress_bar.pulse ();
425 }
426
427 public signal void search_box_activated ();
428@@ -458,7 +479,7 @@
429
430 public void search_box_set_text (string text) {
431
432- plug_closed();
433+ plug_closed ();
434 search_box.set_text (text);
435 }
436
437@@ -468,7 +489,7 @@
438 }
439
440 /*
441- End D-Bus ONLY methods
442+ End D-Bus ONLY methods
443 */
444
445 // Sets up the toolbar for the Switchboard app
446@@ -479,28 +500,30 @@
447 toolbar.get_style_context ().add_class ("primary-toolbar");
448
449 // Spacing
450- lspace.set_expand(true);
451- rspace.set_expand(true);
452+ lspace.set_expand (true);
453+ rspace.set_expand (true);
454
455 // Progressbar
456 var progress_vbox = new Gtk.Box (Gtk.Orientation.VERTICAL, 0);
457 progress_label = new Gtk.Label ("");
458- progress_label.set_use_markup(true);
459+ progress_label.set_use_markup (true);
460 progress_bar = new Gtk.ProgressBar ();
461 progress_toolitem = new Gtk.ToolItem ();
462- progress_vbox.pack_start(progress_label, true, false, 0);
463- progress_vbox.pack_end(progress_bar, false, false, 0);
464- progress_toolitem.add(progress_vbox);
465- progress_toolitem.set_expand(true);
466+ progress_vbox.pack_start (progress_label, true, false, 0);
467+ progress_vbox.pack_end (progress_bar, false, false, 0);
468+ progress_toolitem.add (progress_vbox);
469+ progress_toolitem.set_expand (true);
470
471 // Searchbar
472 search_box = new Gtk.SearchEntry ();
473 search_box.set_placeholder_text (_("Search Settings"));
474- search_box.activate.connect(() => search_box_activated());
475- search_box.changed.connect(() => {
476- category_view.filter_plugs(search_box.get_text (), this);
477- search_box_text_changed();
478+ search_box.activate.connect (() => search_box_activated ());
479+
480+ search_box.changed.connect (() => {
481+ category_view.filter_plugs (search_box.get_text (), this);
482+ search_box_text_changed ();
483 });
484+
485 search_box.sensitive = (count_plugs () > 0);
486 var find_toolitem = new Gtk.ToolItem ();
487 find_toolitem.add(search_box);
488@@ -519,7 +542,7 @@
489 navigation_button = new Gtk.ToolButton (null, null);
490 navigation_button.set_icon_name ("go-previous");
491 navigation_button.clicked.connect (handle_navigation_button_clicked);
492- navigation_button.set_sensitive(false);
493+ navigation_button.set_sensitive (false);
494
495 // Add everything to the toolbar
496 toolbar.insert (navigation_button, -1);
497@@ -528,7 +551,7 @@
498 toolbar.insert (rspace, -1);
499 toolbar.insert (find_toolitem, -1);
500 }
501-
502+
503 public override void activate () {
504 // If app is already running, present the current window.
505 if (get_windows () != null) {
506@@ -582,41 +605,46 @@
507 }
508
509 static const OptionEntry[] entries = {
510- { "open-plug", 'o', 0, OptionArg.STRING, ref plug_to_open, N_("Open a plug"), "PLUG_NAME" },
511- { null }
512+ { "open-plug", 'o', 0, OptionArg.STRING, ref plug_to_open, N_("Open a plug"), "PLUG_NAME" },
513+ { null }
514 };
515
516 public static int main (string[] args) {
517-
518 var logger = new Granite.Services.Logger ();
519- logger.initialize(APP_TITLE);
520+ logger.initialize (APP_TITLE);
521 logger.DisplayLevel = Granite.Services.LogLevel.INFO;
522- message(_(@"Welcome to $APP_TITLE"));
523- message(_(@"Version: $VERSION"));
524- message(_("Report any issues/bugs you mind find to lp:switchboard"));
525-
526+ message (_(@"Welcome to $APP_TITLE"));
527+ message (_(@"Version: $VERSION"));
528+ message (_("Report any issues/bugs you mind find to lp:switchboard"));
529+
530 Gtk.init (ref args);
531-
532- var context = new OptionContext("");
533- context.add_main_entries(entries, "switchboard ");
534- context.add_group(Gtk.get_option_group(true));
535+
536+ var context = new OptionContext ("");
537+ context.add_main_entries (entries, "switchboard ");
538+ context.add_group (Gtk.get_option_group (true));
539+
540 try {
541- context.parse(ref args);
542- } catch(Error e) { warning (e.message); }
543-
544+ context.parse (ref args);
545+ } catch(Error e) {
546+ warning (e.message);
547+ }
548+
549 // In the future, the plug_root_dir should be overridable by CLI flags.
550 var switchboard_app = new SwitchboardApp ();
551-
552+
553 GLib.Bus.own_name (BusType.SESSION, "org.elementary.switchboard",
554- BusNameOwnerFlags.NONE,
555- (conn) => {
556- try {
557- conn.register_object("/org/elementary/switchboard", switchboard_app);
558- } catch (IOError e) { warning (e.message); }
559- },
560- () => {},
561- () => {logger.notification(_("Switchboard already running. Exiting..")); Process.exit(1);});
562-
563+ BusNameOwnerFlags.NONE,
564+ (conn) => {
565+ try {
566+ conn.register_object ("/org/elementary/switchboard", switchboard_app);
567+ } catch (IOError e) {
568+ warning (e.message);
569+ }
570+ },
571+
572+ () => {},
573+ () => { logger.notification (_("Switchboard already running. Exiting..")); Process.exit (1); });
574+
575 return switchboard_app.run (args);
576 }
577 }

Subscribers

People subscribed via source and target branches

to all changes: