[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] command cleanup



Attached are four more patches. That kind of coding happens when I'm
in coding-mood but not able to think properly. :P

0001 avoids an unneccesary call to get_current_monitor() like
discussed in IRC already. There are probably more places where this
could be done, but I'm not sure what the exact implications are and
when the monitor *could* change and when not, so I only change this
one.

0002 cleans up g_parameter_expected by making it less confusing (index
1 instead of 0 for commands with no arguments) and also sorts the
commands like they are sorted in the manpage.

0003 and 0004 also sort g_completions and g_commands like in the
manpage.

Florian

-- 
() ascii ribbon campaign - stop html mail    www.asciiribbon.org
/\ www.the-compiler.org  | I love long mails http://email.is-not-s.ms/
"A verbal contract isn't worth the paper it's printed on." - Samuel Goldwyn 
>From 9064779d56d348a4eba449500c42ef0a2da9c956 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git _at_ the _minus_ compiler _dot_ org>
Date: Thu, 1 Nov 2012 18:00:01 +0100
Subject: [PATCH 1/4] Avoid unneccesary get_current_monitor()-call

---
 src/monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/monitor.c b/src/monitor.c
index 105e701..7cec851 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -626,7 +626,7 @@ int monitor_set_tag_command(int argc, char** argv) {
     HSMonitor* monitor = get_current_monitor();
     HSTag*  tag = find_tag(argv[1]);
     if (monitor && tag) {
-        monitor_set_tag(get_current_monitor(), tag);
+        monitor_set_tag(monitor, tag);
         return 0;
     } else {
         return HERBST_INVALID_ARGUMENT;
-- 
1.8.0

>From be610c2a071e0e3eabce77183adb5669027b5e52 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git _at_ the _minus_ compiler _dot_ org>
Date: Thu, 1 Nov 2012 18:33:07 +0100
Subject: [PATCH 2/4] Clean up g_parameter_expected

 - Change all 0 indexes to 1
   It does not actually matter, but there's probably less confusing this way
   because it's always (number of parameters + 1).
 - Change min_index for lock_tag from 1 to 2 (it has 1 argument, not 0)
 - Add lines for rotate/focus_monitor/detect_monitors/raise_monitor/stack
 - Sort commands like they are in the manpage
---
 src/command.c | 95 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 50 insertions(+), 45 deletions(-)

diff --git a/src/command.c b/src/command.c
index 54296c9..4b7df53 100644
--- a/src/command.c
+++ b/src/command.c
@@ -51,64 +51,69 @@ struct {
                         /* if current pos >= min_index */
     bool    (*function)(int argc, char** argv, int pos);
 } g_parameter_expected[] = {
-    { "quit",           0,  no_completion },
-    { "reload",         0,  no_completion },
-    { "version",        0,  no_completion },
-    { "list_monitors",  0,  no_completion },
-    { "list_commands",  0,  no_completion },
-    { "list_keybinds",  0,  no_completion },
-    { "list_padding",   0,  no_completion },
-    { "add_monitor",    7,  no_completion },
-    { "bring",          2,  no_completion },
+    { "quit",           1,  no_completion },
+    { "reload",         1,  no_completion },
+    { "version",        1,  no_completion },
+    { "list_commands",  1,  no_completion },
+    { "list_monitors",  1,  no_completion },
+    { "list_keybinds",  1,  no_completion },
+    { "lock",           1,  no_completion },
+    { "unlock",         1,  no_completion },
+    { "keybind",        2,  keybind_parameter_expected },
+    { "keyunbind",      2,  no_completion },
+    { "mousebind",      3,  no_completion },
+    { "mouseunbind",    1,  no_completion },
     { "focus_nth",      2,  no_completion },
     { "cycle",          2,  no_completion },
     { "cycle_all",      3,  no_completion },
     { "cycle_layout",   LAYOUT_COUNT+2, no_completion },
-    { "cycle_monitor",  2,  no_completion },
-    { "close",          0,  no_completion },
-    { "close_or_remove",0,  no_completion },
-    { "dump",           2,  no_completion },
-    { "floating",       3,  no_completion },
-    { "floating",       2,  first_parameter_is_tag },
-    { "merge_tag",      3,  no_completion },
+    { "set_layout",     2,  no_completion },
+    { "close",          1,  no_completion },
+    { "close_or_remove",1,  no_completion },
+    { "split",          3,  no_completion },
     { "focus",          3,  no_completion },
     { "focus",          2,  first_parameter_is_flag },
+    { "raise",          2,  no_completion },
+    { "jumpto",         2,  no_completion },
+    { "bring",          2,  no_completion },
+    { "resize",         3,  no_completion },
     { "shift",          3,  no_completion },
     { "shift",          2,  first_parameter_is_flag },
-    { "split",          3,  no_completion },
-    { "fullscreen",     2,  no_completion },
-    { "pseudotile",     2,  no_completion },
-    { "pad",            6,  no_completion },
-    { "keybind",        2,  keybind_parameter_expected },
-    { "keyunbind",      2,  no_completion },
-    { "monitor_rect",   3,  no_completion },
-    { "mousebind",      3,  no_completion },
-    { "mouseunbind",    0,  no_completion },
+    { "remove",         1,  no_completion },
+    { "rotate",         1,  no_completion },
+    { "set",            3,  no_completion },
+    { "get",            2,  no_completion },
+    { "toggle",         2,  no_completion },
+    { "cycle_monitor",  2,  no_completion },
+    { "focus_monitor",  2,  no_completion },
+    { "add",            2,  no_completion },
+    { "use",            2,  no_completion },
+    { "use_index",      3,  no_completion },
+    { "merge_tag",      3,  no_completion },
+    { "rename",         3,  no_completion },
+    { "move",           2,  no_completion },
+    { "move_index",     3,  no_completion },
+    { "lock_tag",       2,  no_completion },
+    { "unlock_tag",     2,  no_completion },
+    { "detect_monitors",1,  no_completion },
+    { "add_monitor",    7,  no_completion },
+    { "remove_monitor", 2,  no_completion },
     { "move_monitor",   7,  no_completion },
+    { "raise_monitor",  2,  no_completion },
+    { "stack",          2,  no_completion },
+    { "monitor_rect",   3,  no_completion },
+    { "pad",            6,  no_completion },
+    { "list_padding",   1,  no_completion },
     { "layout",         2,  no_completion },
+    { "dump",           2,  no_completion },
     { "load",           3,  no_completion },
     { "load",           2,  first_parameter_is_tag },
-    { "lock",           0,  no_completion },
-    { "unlock",         0,  no_completion },
-    { "lock_tag",       1,  no_completion },
-    { "unlock_tag",     1,  no_completion },
-    { "move",           2,  no_completion },
-    { "move_index",     3,  no_completion },
-    { "raise",          2,  no_completion },
-    { "jumpto",         2,  no_completion },
-    { "rename",         3,  no_completion },
-    { "remove",         0,  no_completion },
-    { "remove_monitor", 2,  no_completion },
-    { "resize",         3,  no_completion },
-    { "unrule",         2,  no_completion },
-    { "use",            2,  no_completion },
-    { "use_index",      3,  no_completion },
-    { "add",            2,  no_completion },
-    { "get",            2,  no_completion },
-    { "toggle",         2,  no_completion },
-    { "set",            3,  no_completion },
-    { "set_layout",     2,  no_completion },
     { "tag_status",     2,  no_completion },
+    { "floating",       3,  no_completion },
+    { "floating",       2,  first_parameter_is_tag },
+    { "unrule",         2,  no_completion },
+    { "fullscreen",     2,  no_completion },
+    { "pseudotile",     2,  no_completion },
     { 0 },
 };
 
-- 
1.8.0

>From 457c4b32f2160899a115b47f38e53421d150c8d4 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git _at_ the _minus_ compiler _dot_ org>
Date: Thu, 1 Nov 2012 18:52:33 +0100
Subject: [PATCH 3/4] Sort commands in g_completions like in the manpage

---
 src/command.c | 60 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/command.c b/src/command.c
index 4b7df53..a7cf741 100644
--- a/src/command.c
+++ b/src/command.c
@@ -137,59 +137,59 @@ struct {
     char** list;
 } g_completions[] = {
     /* name , relation, index,  completion method                   */
-    { "add_monitor",    EQ, 2,  .function = complete_against_tags },
+    { "keybind",        GE, 2,  .function = complete_against_keybind_command },
+    { "keyunbind",      EQ, 1,  .list = completion_keyunbind_args },
+    { "keyunbind",      EQ, 1,  .function = complete_against_keybinds },
+    { "chain",          GE, 1,  .function = complete_chain },
     { "and",            GE, 1,  .function = complete_chain },
-    { "bring",          EQ, 1,  .list = completion_special_winids },
-    { "bring",          EQ, 1,  .function = complete_against_winids },
+    { "or",             GE, 1,  .function = complete_chain },
     { "cycle",          EQ, 1,  .list = completion_pm_one },
-    { "chain",          GE, 1,  .function = complete_chain },
     { "cycle_all",      EQ, 1,  .list = completion_cycle_all_args },
     { "cycle_all",      EQ, 1,  .list = completion_pm_one },
     { "cycle_all",      EQ, 2,  .list = completion_pm_one },
-    { "cycle_monitor",  EQ, 1,  .list = completion_pm_one },
-    { "dump",           EQ, 1,  .function = complete_against_tags },
-    { "floating",       EQ, 1,  .function = complete_against_tags },
-    { "floating",       EQ, 1,  .list = completion_flag_args },
-    { "floating",       EQ, 1,  .list = completion_status },
-    { "floating",       EQ, 2,  .list = completion_flag_args },
-    { "floating",       EQ, 2,  .list = completion_status },
+    { "cycle_layout",   EQ, 1,  .list = completion_pm_one },
+    { "cycle_layout",   GE, 2,  .list = g_layout_names },
+    { "set_layout",     EQ, 1,  .list = g_layout_names },
+    { "split",          EQ, 1,  .list = completion_split_modes },
+    { "split",          EQ, 2,  .list = completion_split_ratios },
     { "focus",          EQ, 1,  .list = completion_directions },
     { "focus",          EQ, 1,  .list = completion_focus_args },
     { "focus",          EQ, 2,  .list = completion_directions },
-    { "fullscreen",     EQ, 1,  .list = completion_flag_args },
-    { "layout",         EQ, 1,  .function = complete_against_tags },
-    { "load",           EQ, 1,  .function = complete_against_tags },
-    { "merge_tag",      EQ, 1,  .function = complete_against_tags },
-    { "merge_tag",      EQ, 2,  .function = complete_merge_tag },
-    { "move",           EQ, 1,  .function = complete_against_tags },
-    { "move_index",     EQ, 2,  .list = completion_use_index_args },
-    { "or",             GE, 1,  .function = complete_chain },
-    { "pseudotile",     EQ, 1,  .list = completion_flag_args },
-    { "keybind",        GE, 2,  .function = complete_against_keybind_command },
-    { "keyunbind",      EQ, 1,  .list = completion_keyunbind_args },
-    { "keyunbind",      EQ, 1,  .function = complete_against_keybinds },
-    { "rename",         EQ, 1,  .function = complete_against_tags },
     { "raise",          EQ, 1,  .list = completion_special_winids },
     { "raise",          EQ, 1,  .function = complete_against_winids },
     { "jumpto",         EQ, 1,  .list = completion_special_winids },
     { "jumpto",         EQ, 1,  .function = complete_against_winids },
+    { "bring",          EQ, 1,  .list = completion_special_winids },
+    { "bring",          EQ, 1,  .function = complete_against_winids },
     { "resize",         EQ, 1,  .list = completion_directions },
     { "shift",          EQ, 1,  .list = completion_directions },
     { "shift",          EQ, 1,  .list = completion_focus_args },
     { "shift",          EQ, 2,  .list = completion_directions },
     { "set",            EQ, 1,  .function = complete_against_settings },
-    { "split",          EQ, 1,  .list = completion_split_modes },
-    { "split",          EQ, 2,  .list = completion_split_ratios },
     { "get",            EQ, 1,  .function = complete_against_settings },
     { "toggle",         EQ, 1,  .function = complete_against_settings },
     { "cycle_value",    EQ, 1,  .function = complete_against_settings },
-    { "set_layout",     EQ, 1,  .list = g_layout_names },
-    { "cycle_layout",   EQ, 1,  .list = completion_pm_one },
-    { "cycle_layout",   GE, 2,  .list = g_layout_names },
-    { "unrule",         EQ, 1,  .list = completion_unrule_args },
+    { "cycle_monitor",  EQ, 1,  .list = completion_pm_one },
     { "use",            EQ, 1,  .function = complete_against_tags },
     { "use_index",      EQ, 1,  .list = completion_pm_one },
     { "use_index",      EQ, 2,  .list = completion_use_index_args },
+    { "merge_tag",      EQ, 1,  .function = complete_against_tags },
+    { "merge_tag",      EQ, 2,  .function = complete_merge_tag },
+    { "rename",         EQ, 1,  .function = complete_against_tags },
+    { "move",           EQ, 1,  .function = complete_against_tags },
+    { "move_index",     EQ, 2,  .list = completion_use_index_args },
+    { "add_monitor",    EQ, 2,  .function = complete_against_tags },
+    { "layout",         EQ, 1,  .function = complete_against_tags },
+    { "dump",           EQ, 1,  .function = complete_against_tags },
+    { "load",           EQ, 1,  .function = complete_against_tags },
+    { "floating",       EQ, 1,  .function = complete_against_tags },
+    { "floating",       EQ, 1,  .list = completion_flag_args },
+    { "floating",       EQ, 1,  .list = completion_status },
+    { "floating",       EQ, 2,  .list = completion_flag_args },
+    { "floating",       EQ, 2,  .list = completion_status },
+    { "unrule",         EQ, 1,  .list = completion_unrule_args },
+    { "fullscreen",     EQ, 1,  .list = completion_flag_args },
+    { "pseudotile",     EQ, 1,  .list = completion_flag_args },
     { 0 },
 };
 
-- 
1.8.0

>From c15706856065fa19a03f2608559022a5b261e5c2 Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git _at_ the _minus_ compiler _dot_ org>
Date: Thu, 1 Nov 2012 19:01:35 +0100
Subject: [PATCH 4/4] Sort commands in g_commands like in the manpage

---
 src/main.c | 54 +++++++++++++++++++++++++++---------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/src/main.c b/src/main.c
index 07be5e6..f968e5e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -85,71 +85,71 @@ CommandBinding g_commands[] = {
     CMD_BIND(             "version",        version),
     CMD_BIND(             "list_commands",  list_commands),
     CMD_BIND(             "list_monitors",  list_monitors),
-    CMD_BIND(             "set_monitors",   set_monitor_rects_command),
-    CMD_BIND(             "disjoin_rects",  disjoin_rects_command),
     CMD_BIND(             "list_keybinds",  key_list_binds),
-    CMD_BIND(             "list_padding",   list_padding),
+    CMD_BIND_NO_OUTPUT(   "lock",           monitors_lock_command),
+    CMD_BIND_NO_OUTPUT(   "unlock",         monitors_unlock_command),
     CMD_BIND_NO_OUTPUT(   "keybind",        keybind),
     CMD_BIND_NO_OUTPUT(   "keyunbind",      keyunbind),
     CMD_BIND_NO_OUTPUT(   "mousebind",      mouse_bind_command),
     CMD_BIND_NO_OUTPUT(   "mouseunbind",    mouse_unbind_all),
     CMD_BIND_NO_OUTPUT(   "spawn",          spawn),
     CMD_BIND_NO_OUTPUT(   "wmexec",         wmexec),
-    CMD_BIND_NO_OUTPUT(   "emit_hook",      custom_hook_emit),
-    CMD_BIND_NO_OUTPUT(   "bring",          frame_current_bring),
+    CMD_BIND(             "chain",          command_chain_command),
+    CMD_BIND(             "and",            command_chain_command),
+    CMD_BIND(             "or",            command_chain_command),
     CMD_BIND_NO_OUTPUT(   "focus_nth",      frame_current_set_selection),
     CMD_BIND_NO_OUTPUT(   "cycle",          frame_current_cycle_selection),
     CMD_BIND_NO_OUTPUT(   "cycle_all",      cycle_all_command),
     CMD_BIND_NO_OUTPUT(   "cycle_layout",   frame_current_cycle_client_layout),
+    CMD_BIND_NO_OUTPUT(   "set_layout",     frame_current_set_client_layout),
     CMD_BIND_NO_OUTPUT(   "close",          window_close_current),
     CMD_BIND_NO_OUTPUT(   "close_or_remove",close_or_remove_command),
     CMD_BIND_NO_OUTPUT(   "split",          frame_split_command),
-    CMD_BIND_NO_OUTPUT(   "resize",         frame_change_fraction_command),
     CMD_BIND_NO_OUTPUT(   "focus",          frame_focus_command),
+    CMD_BIND_NO_OUTPUT(   "raise",          raise_command),
+    CMD_BIND_NO_OUTPUT(   "jumpto",         jumpto_command),
+    CMD_BIND_NO_OUTPUT(   "bring",          frame_current_bring),
+    CMD_BIND_NO_OUTPUT(   "resize",         frame_change_fraction_command),
     CMD_BIND_NO_OUTPUT(   "shift",          frame_move_window_command),
     CMD_BIND_NO_OUTPUT(   "remove",         frame_remove_command),
+    CMD_BIND_NO_OUTPUT(   "rotate",         layout_rotate_command),
     CMD_BIND_NO_OUTPUT(   "set",            settings_set_command),
+    CMD_BIND(             "get",            settings_get),
     CMD_BIND_NO_OUTPUT(   "toggle",         settings_toggle),
     CMD_BIND_NO_OUTPUT(   "cycle_value",    settings_cycle_value),
     CMD_BIND_NO_OUTPUT(   "cycle_monitor",  monitor_cycle_command),
     CMD_BIND_NO_OUTPUT(   "focus_monitor",  monitor_focus_command),
-    CMD_BIND(             "get",            settings_get),
     CMD_BIND_NO_OUTPUT(   "add",            tag_add_command),
     CMD_BIND_NO_OUTPUT(   "use",            monitor_set_tag_command),
     CMD_BIND_NO_OUTPUT(   "use_index",      monitor_set_tag_by_index_command),
-    CMD_BIND_NO_OUTPUT(   "jumpto",         jumpto_command),
-    CMD_BIND(             "floating",       tag_set_floating_command),
-    CMD_BIND_NO_OUTPUT(   "fullscreen",     client_set_property_command),
-    CMD_BIND_NO_OUTPUT(   "pseudotile",     client_set_property_command),
-    CMD_BIND(             "tag_status",     print_tag_status_command),
     CMD_BIND_NO_OUTPUT(   "merge_tag",      tag_remove_command),
     CMD_BIND_NO_OUTPUT(   "rename",         tag_rename_command),
     CMD_BIND_NO_OUTPUT(   "move",           tag_move_window_command),
-    CMD_BIND_NO_OUTPUT(   "rotate",         layout_rotate_command),
     CMD_BIND_NO_OUTPUT(   "move_index",     tag_move_window_by_index_command),
+    CMD_BIND_NO_OUTPUT(   "lock_tag",       monitor_lock_tag_command),
+    CMD_BIND_NO_OUTPUT(   "unlock_tag",     monitor_unlock_tag_command),
+    CMD_BIND(             "disjoin_rects",  disjoin_rects_command),
+    CMD_BIND(             "set_monitors",   set_monitor_rects_command),
+    CMD_BIND_NO_OUTPUT(   "detect_monitors",detect_monitors_command),
     CMD_BIND_NO_OUTPUT(   "add_monitor",    add_monitor_command),
-    CMD_BIND_NO_OUTPUT(   "raise_monitor",  monitor_raise_command),
     CMD_BIND_NO_OUTPUT(   "remove_monitor", remove_monitor_command),
     CMD_BIND_NO_OUTPUT(   "move_monitor",   move_monitor_command),
+    CMD_BIND_NO_OUTPUT(   "raise_monitor",  monitor_raise_command),
+    CMD_BIND(             "stack",          print_stack_command),
     CMD_BIND(             "monitor_rect",   monitor_rect_command),
     CMD_BIND_NO_OUTPUT(   "pad",            monitor_set_pad_command),
-    CMD_BIND_NO_OUTPUT(   "raise",          raise_command),
-    CMD_BIND_NO_OUTPUT(   "rule",           rule_add_command),
-    CMD_BIND_NO_OUTPUT(   "unrule",         rule_remove_command),
+    CMD_BIND(             "list_padding",   list_padding),
     CMD_BIND(             "layout",         print_layout_command),
-    CMD_BIND(             "stack",          print_stack_command),
     CMD_BIND(             "dump",           print_layout_command),
     CMD_BIND(             "load",           load_command),
     CMD_BIND(             "complete",       complete_command),
-    CMD_BIND_NO_OUTPUT(   "lock",           monitors_lock_command),
-    CMD_BIND_NO_OUTPUT(   "unlock",         monitors_unlock_command),
-    CMD_BIND_NO_OUTPUT(   "lock_tag",       monitor_lock_tag_command),
-    CMD_BIND_NO_OUTPUT(   "unlock_tag",     monitor_unlock_tag_command),
-    CMD_BIND_NO_OUTPUT(   "set_layout",     frame_current_set_client_layout),
-    CMD_BIND_NO_OUTPUT(   "detect_monitors",detect_monitors_command),
-    CMD_BIND(             "chain",          command_chain_command),
-    CMD_BIND(             "and",            command_chain_command),
-    CMD_BIND(             "or",            command_chain_command),
+    CMD_BIND_NO_OUTPUT(   "emit_hook",      custom_hook_emit),
+    CMD_BIND(             "tag_status",     print_tag_status_command),
+    CMD_BIND(             "floating",       tag_set_floating_command),
+    CMD_BIND_NO_OUTPUT(   "rule",           rule_add_command),
+    CMD_BIND_NO_OUTPUT(   "unrule",         rule_remove_command),
+    CMD_BIND_NO_OUTPUT(   "fullscreen",     client_set_property_command),
+    CMD_BIND_NO_OUTPUT(   "pseudotile",     client_set_property_command),
     {{ NULL }}
 };
 
-- 
1.8.0