i3 - improved tiling WM

commands: (rename) check workspace number

Patch status: rejected

Patch by Vivien Didelot

Long description:

Without this patch, you can rename a workspace with a number which is
already in use, which makes the navigation a bit inconsistent (for
instance workspace next_on_output won't work as expected).

For example, assuming you have a workspace 1, you can actually rename
another workspace to "1: foo", but will have issue switching to it.

This patch updates the cmd_rename_workspace() function to check the
workspace number and forbid using a number already used by a workspace
different than the target.

Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>

To apply this patch, use:
curl http://cr.i3wm.org/patch/245/raw.patch | git am

b/src/commands.c

25 
@@ -1856,6 +1856,7 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) {
26 
         LOG("Renaming current workspace to \"%s\"\n", new_name);
27 
     }
28
29 
+    /* Define the target workspace. */
30 
     Con *output, *workspace = NULL;
31 
     if (old_name) {
32 
         TAILQ_FOREACH(output, &(croot->nodes_head), nodes)
33 
@@ -1869,10 +1870,11 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) {
34 
         // TODO: we should include the old workspace name here and use yajl for
35 
         // generating the reply.
36 
         // TODO: better error message
37 
-        yerror("Old workspace not found");
38 
+        yerror("Target workspace not found");
39 
         return;
40 
     }
41
42 
+    /* Verify the full name is not already used. */
43 
     Con *check_dest = NULL;
44 
     TAILQ_FOREACH(output, &(croot->nodes_head), nodes)
45 
         GREP_FIRST(check_dest, output_get_content(output),
46 
@@ -1886,18 +1888,32 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) {
47 
         return;
48 
     }
49
50 
-    /* Change the name and try to parse it as a number. */
51 
-    FREE(workspace->name);
52 
-    workspace->name = sstrdup(new_name);
53 
+    /* Parse and check the workspace number. */
54 
     char *endptr = NULL;
55 
     long parsed_num = strtol(new_name, &endptr, 10);
56 
     if (parsed_num == LONG_MIN ||
57 
         parsed_num == LONG_MAX ||
58 
         parsed_num < 0 ||
59 
         endptr == new_name)
60 
-        workspace->num = -1;
61 
-    else workspace->num = parsed_num;
62 
-    LOG("num = %d\n", workspace->num);
63 
+        parsed_num = -1;
64 
+    LOG("num = %ld\n", parsed_num);
65 
+    if (parsed_num != -1) {
66 
+        Con *ws = NULL;
67 
+        TAILQ_FOREACH(output, &(croot->nodes_head), nodes)
68 
+            GREP_FIRST(ws, output_get_content(output),
69 
+                child->num == parsed_num);
70 
+
71 
+        /* It is ok to rename a workspace with its actual number. */
72 
+        if (ws && ws != workspace) {
73 
+            yerror("Workspace number already in use");
74 
+            return;
75 
+        }
76 
+    }
77 
+
78 
+    /* Finally change the name and the number. */
79 
+    FREE(workspace->name);
80 
+    workspace->name = sstrdup(new_name);
81 
+    workspace->num = parsed_num;
82
83 
     /* By re-attaching, the sort order will be correct afterwards. */
84 
     Con *previously_focused = focused;