i3 - improved tiling WM

Reviews
Bugs

move scratchpad on a scratchpad windows inserts in head

Patch status: rejected

Patch by Philippe Virouleau

Long description:

Implement the behaviour described in #1035 : 'scratchpad show' cycles
between windows, 'scratchpad move' on a scratchpad window makes it the
next to be displayed by 'scratchpad show'.

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

b/include/data.h

@@ -585,7 +585,11 @@ struct Con {
         SCRATCHPAD_FRESH = 1,
 
         /* The user changed position/size of the scratchpad window. */
-        SCRATCHPAD_CHANGED = 2
+        SCRATCHPAD_CHANGED = 2,
+
+        /* A scratchpad window moving to scratchpad again through an
+         * actual 'scratchpad move'. */
+        SCRATCHPAD_MOVE = 3
     } scratchpad_state;
 
     /* The ID of this container before restarting. Necessary to correctly

b/include/scratchpad.h

@@ -17,7 +17,7 @@
  * Gets called upon the command 'move scratchpad'.
  *
  */
-void scratchpad_move(Con *con);
+void scratchpad_move(Con *con, bool from_show);
 
 /**
  * Either shows the top-most scratchpad window (con == NULL) or shows the

b/src/commands.c

@@ -1786,7 +1786,7 @@ void cmd_move_scratchpad(I3_CMD) {
 
     TAILQ_FOREACH(current, &owindows, owindows) {
         DLOG("matching: %p / %s\n", current->con, current->con->name);
-        scratchpad_move(current->con);
+        scratchpad_move(current->con, false);
     }
 
     cmd_output->needs_tree_render = true;

b/src/con.c

@@ -138,7 +138,12 @@ void con_attach(Con *con, Con *parent, bool ignore_focus) {
 
     if (con->type == CT_FLOATING_CON) {
         DLOG("Inserting into floating containers\n");
-        TAILQ_INSERT_TAIL(&(parent->floating_head), con, floating_windows);
+        Con *floating;
+        if ((floating = con_inside_floating(con))
+                && floating->scratchpad_state == SCRATCHPAD_MOVE)
+            TAILQ_INSERT_HEAD(&(parent->floating_head), con, floating_windows);
+        else
+            TAILQ_INSERT_TAIL(&(parent->floating_head), con, floating_windows);
     } else {
         if (!ignore_focus) {
             /* Get the first tiling container in focus stack */

b/src/ipc.c

@@ -176,6 +176,9 @@ void dump_node(yajl_gen gen, struct Con *con, bool inplace_restart) {
         case SCRATCHPAD_CHANGED:
             ystr("changed");
             break;
+        case SCRATCHPAD_MOVE:
+            ystr("move");
+            break;
     }
 
     ystr("percent");

b/src/load_layout.c

@@ -254,6 +254,8 @@ static int json_string(void *ctx, const unsigned char *val, unsigned int len) {
                 json_node->scratchpad_state = SCRATCHPAD_FRESH;
             else if (strcasecmp(buf, "changed") == 0)
                 json_node->scratchpad_state = SCRATCHPAD_CHANGED;
+            else if (strcasecmp(buf, "move") == 0)
+                json_node->scratchpad_state = SCRATCHPAD_MOVE;
             free(buf);
         }
     }

b/src/scratchpad.c

@@ -18,7 +18,7 @@
  * Gets called upon the command 'move scratchpad'.
  *
  */
-void scratchpad_move(Con *con) {
+void scratchpad_move(Con *con, bool from_show) {
     if (con->type == CT_WORKSPACE) {
         LOG("'move scratchpad' used on a workspace \"%s\". Calling it "
             "recursively on all windows on this workspace.\n", con->name);
@@ -26,7 +26,7 @@ void scratchpad_move(Con *con) {
         current = TAILQ_FIRST(&(con->focus_head));
         while (current) {
             Con *next = TAILQ_NEXT(current, focused);
-            scratchpad_move(current);
+            scratchpad_move(current, from_show);
             current = next;
         }
         return;
@@ -57,10 +57,21 @@ void scratchpad_move(Con *con) {
         con = maybe_floating_con;
     }
 
+    /*
+     * If this is an actual 'scratchpad move' and the window is a scratchpad window,
+     * set the scratchpad_state to SCRATCHPAD_MOVE so that it's moved to
+     * top of the queue
+     */
+    int tmp_state = con->scratchpad_state;
+    if (!from_show && con->scratchpad_state != SCRATCHPAD_NONE)
+        con->scratchpad_state = SCRATCHPAD_MOVE;
+
     /* 2: Send the window to the __i3_scratch workspace, mainting its
      * coordinates and not warping the pointer. */
     con_move_to_workspace(con, __i3_scratch, true, true);
 
+    con->scratchpad_state = tmp_state;
+
     /* 3: If this is the first time this window is used as a scratchpad, we set
      * the scratchpad_state to SCRATCHPAD_FRESH. The window will then be
      * adjusted in size according to what the user specifies. */
@@ -97,7 +108,7 @@ void scratchpad_show(Con *con) {
         (floating = con_inside_floating(focused)) &&
         floating->scratchpad_state != SCRATCHPAD_NONE) {
         DLOG("Focused window is a scratchpad window, hiding it.\n");
-        scratchpad_move(focused);
+        scratchpad_move(focused, true);
         return;
     }
 
@@ -165,7 +176,7 @@ void scratchpad_show(Con *con) {
          * it, otherwise we should move it to the active workspace. */
         if (current == active) {
             DLOG("Window is a scratchpad window, hiding it.\n");
-            scratchpad_move(con);
+            scratchpad_move(con, true);
             return;
         }
     }

b/testcases/t/185-scratchpad.t

@@ -398,10 +398,11 @@ sub verify_scratchpad_move_with_visible_scratch_con {
     # hide window 1 again
     cmd 'move scratchpad';
 
-    # this should bring up window 2
+    # Since #1035
+    # this should bring up window 1
     cmd "workspace $first";
     cmd 'scratchpad show';
-    is($x->input_focus, $window2->id, "showed the correct scratchpad window");
+    is($x->input_focus, $window1->id, "showed the correct scratchpad window");
 }
 
 # let's clear the scratchpad first
@@ -428,21 +429,91 @@ does_i3_live;
 # when another window on the same workspace has focus
 ################################################################################
 
+sub test_scratchpad_show_moves_focus {
+    my $ws = fresh_workspace;
+    cmd "workspace $ws";
+
+    open_window;
+    my $scratch = get_focused($ws);
+    cmd 'move scratchpad';
+    cmd 'scratchpad show';
+
+    open_window;
+    my $not_scratch = get_focused($ws);
+    is(get_focused($ws), $not_scratch, 'not scratch window has focus');
+
+    cmd 'scratchpad show';
+
+    is(get_focused($ws), $scratch, 'scratchpad is focused');
+
+    #Kill the scratchpad window so that it doesn't interfere with other tests
+    cmd 'kill';
+
+    is(get_focused($ws), $not_scratch, 'not scratch window has focus');
+}
+
 clear_scratchpad;
-my $ws = fresh_workspace;
+is (scalar @{get_ws('__i3_scratch')->{floating_nodes}}, 0, "scratchpad is empty");
+test_scratchpad_show_moves_focus;
 
-open_window;
-my $scratch = get_focused($ws);
-cmd 'move scratchpad';
-cmd 'scratchpad show';
+################################################################################
+# 13bis: Test the following behaviour :
+# With multiple windows in the scratchpad, 'scratchpad_show' cycles between
+# them, and 'scratchpad move' on a scratchpad window makes it the next to be
+# displayed with 'scratchpad show'
+################################################################################
 
-open_window;
-my $not_scratch = get_focused($ws);
-is(get_focused($ws), $not_scratch, 'not scratch window has focus');
+sub test_scratchpad_move_to_head {
 
-cmd 'scratchpad show';
+    my $ws = fresh_workspace;
+    cmd "workspace $ws";
+
+    my $window1 = open_window;
+    cmd 'move scratchpad';
+
+    my $window2 = open_window;
+    cmd 'move scratchpad';
 
-is(get_focused($ws), $scratch, 'scratchpad is focused');
+    # this should bring up window 1
+    cmd 'scratchpad show';
+
+    is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
+    is($x->input_focus, $window1->id, "showed the correct scratchpad window1");
+
+    #this should hide window 1
+    cmd 'scratchpad show';
+
+    is(scalar @{get_ws($ws)->{floating_nodes}}, 0, 'no floating node on ws');
+
+    #this should show window 2
+    cmd 'scratchpad show';
+
+    is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
+    is($x->input_focus, $window2->id, "showed the correct scratchpad window2");
+
+    #this should hide window 2 and make it the next window to be displayed
+    cmd 'move scratchpad';
+
+
+    is(scalar @{get_ws($ws)->{floating_nodes}}, 0, 'no floating node on ws');
+
+    #this should show window 2 (#1035)
+    cmd 'scratchpad show';
+
+    is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
+    is($x->input_focus, $window2->id, "showed the correct scratchpad window2");
+
+    #clean
+    cmd 'kill';
+    cmd 'scratchpad show';
+    cmd 'kill';
+
+
+}
+
+clear_scratchpad;
+is (scalar @{get_ws('__i3_scratch')->{floating_nodes}}, 0, "scratchpad is empty");
+test_scratchpad_move_to_head;
 
 # TODO: make i3bar display *something* when a window on the scratchpad has the urgency hint
 

111	@@ -18,7 +18,7 @@
112	* Gets called upon the command 'move scratchpad'.
113	*
114	*/
115	-void scratchpad_move(Con *con) {
116	+void scratchpad_move(Con *con, bool from_show) {
117	if (con->type == CT_WORKSPACE) {
118	LOG("'move scratchpad' used on a workspace \"%s\". Calling it "
119	"recursively on all windows on this workspace.\n", con->name);
120	@@ -26,7 +26,7 @@ void scratchpad_move(Con *con) {
121	current = TAILQ_FIRST(&(con->focus_head));
122	while (current) {
123	Con *next = TAILQ_NEXT(current, focused);
124	- scratchpad_move(current);
125	+ scratchpad_move(current, from_show);
126	current = next;
127	}
128	return;
129	@@ -57,10 +57,21 @@ void scratchpad_move(Con *con) {
130	con = maybe_floating_con;
131	}
132
133	+ /*
134	+ * If this is an actual 'scratchpad move' and the window is a scratchpad window,
135	+ * set the scratchpad_state to SCRATCHPAD_MOVE so that it's moved to
136	+ * top of the queue
137	+ */
138	+ int tmp_state = con->scratchpad_state;
139	+ if (!from_show && con->scratchpad_state != SCRATCHPAD_NONE)
140	+ con->scratchpad_state = SCRATCHPAD_MOVE;
141	+
142	/* 2: Send the window to the __i3_scratch workspace, mainting its
143	* coordinates and not warping the pointer. */
144	con_move_to_workspace(con, __i3_scratch, true, true);
145
146	+ con->scratchpad_state = tmp_state;
147	+
148	/* 3: If this is the first time this window is used as a scratchpad, we set
149	* the scratchpad_state to SCRATCHPAD_FRESH. The window will then be
150	* adjusted in size according to what the user specifies. */
151	@@ -97,7 +108,7 @@ void scratchpad_show(Con *con) {
152	(floating = con_inside_floating(focused)) &&
153	floating->scratchpad_state != SCRATCHPAD_NONE) {
154	DLOG("Focused window is a scratchpad window, hiding it.\n");
155	- scratchpad_move(focused);
156	+ scratchpad_move(focused, true);
157	return;
158	}
159
160	@@ -165,7 +176,7 @@ void scratchpad_show(Con *con) {
161	* it, otherwise we should move it to the active workspace. */
162	if (current == active) {
163	DLOG("Window is a scratchpad window, hiding it.\n");
164	- scratchpad_move(con);
165	+ scratchpad_move(con, true);
166	return;
167	}
168	}

173	@@ -398,10 +398,11 @@ sub verify_scratchpad_move_with_visible_scratch_con {
174	# hide window 1 again
175	cmd 'move scratchpad';
176
177	- # this should bring up window 2
178	+ # Since #1035
179	+ # this should bring up window 1
180	cmd "workspace $first";
181	cmd 'scratchpad show';
182	- is($x->input_focus, $window2->id, "showed the correct scratchpad window");
183	+ is($x->input_focus, $window1->id, "showed the correct scratchpad window");
184	}
185
186	# let's clear the scratchpad first
187	@@ -428,21 +429,91 @@ does_i3_live;
188	# when another window on the same workspace has focus
189	################################################################################
190
191	+sub test_scratchpad_show_moves_focus {
192	+ my $ws = fresh_workspace;
193	+ cmd "workspace $ws";
194	+
195	+ open_window;
196	+ my $scratch = get_focused($ws);
197	+ cmd 'move scratchpad';
198	+ cmd 'scratchpad show';
199	+
200	+ open_window;
201	+ my $not_scratch = get_focused($ws);
202	+ is(get_focused($ws), $not_scratch, 'not scratch window has focus');
203	+
204	+ cmd 'scratchpad show';
205	+
206	+ is(get_focused($ws), $scratch, 'scratchpad is focused');
207	+
208	+ #Kill the scratchpad window so that it doesn't interfere with other tests
209	+ cmd 'kill';
210	+
211	+ is(get_focused($ws), $not_scratch, 'not scratch window has focus');
212	+}
213	+
214	clear_scratchpad;
215	-my $ws = fresh_workspace;
216	+is (scalar @{get_ws('__i3_scratch')->{floating_nodes}}, 0, "scratchpad is empty");
217	+test_scratchpad_show_moves_focus;
218
219	-open_window;
220	-my $scratch = get_focused($ws);
221	-cmd 'move scratchpad';
222	-cmd 'scratchpad show';
223	+################################################################################
224	+# 13bis: Test the following behaviour :
225	+# With multiple windows in the scratchpad, 'scratchpad_show' cycles between
226	+# them, and 'scratchpad move' on a scratchpad window makes it the next to be
227	+# displayed with 'scratchpad show'
228	+################################################################################
229
230	-open_window;
231	-my $not_scratch = get_focused($ws);
232	-is(get_focused($ws), $not_scratch, 'not scratch window has focus');
233	+sub test_scratchpad_move_to_head {
234
235	-cmd 'scratchpad show';
236	+ my $ws = fresh_workspace;
237	+ cmd "workspace $ws";
238	+
239	+ my $window1 = open_window;
240	+ cmd 'move scratchpad';
241	+
242	+ my $window2 = open_window;
243	+ cmd 'move scratchpad';
244
245	-is(get_focused($ws), $scratch, 'scratchpad is focused');
246	+ # this should bring up window 1
247	+ cmd 'scratchpad show';
248	+
249	+ is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
250	+ is($x->input_focus, $window1->id, "showed the correct scratchpad window1");
251	+
252	+ #this should hide window 1
253	+ cmd 'scratchpad show';
254	+
255	+ is(scalar @{get_ws($ws)->{floating_nodes}}, 0, 'no floating node on ws');
256	+
257	+ #this should show window 2
258	+ cmd 'scratchpad show';
259	+
260	+ is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
261	+ is($x->input_focus, $window2->id, "showed the correct scratchpad window2");
262	+
263	+ #this should hide window 2 and make it the next window to be displayed
264	+ cmd 'move scratchpad';
265	+
266	+
267	+ is(scalar @{get_ws($ws)->{floating_nodes}}, 0, 'no floating node on ws');
268	+
269	+ #this should show window 2 (#1035)
270	+ cmd 'scratchpad show';
271	+
272	+ is(scalar @{get_ws($ws)->{floating_nodes}}, 1, 'one floating node on ws');
273	+ is($x->input_focus, $window2->id, "showed the correct scratchpad window2");
274	+
275	+ #clean
276	+ cmd 'kill';
277	+ cmd 'scratchpad show';
278	+ cmd 'kill';
279	+
280	+
281	+}
282	+
283	+clear_scratchpad;
284	+is (scalar @{get_ws('__i3_scratch')->{floating_nodes}}, 0, "scratchpad is empty");
285	+test_scratchpad_move_to_head;
286
287	# TODO: make i3bar display something when a window on the scratchpad has the urgency hint
288

23	@@ -585,7 +585,11 @@ struct Con {
24	SCRATCHPAD_FRESH = 1,
25
26	/* The user changed position/size of the scratchpad window. */
27	- SCRATCHPAD_CHANGED = 2
28	+ SCRATCHPAD_CHANGED = 2,
29	+
30	+ /* A scratchpad window moving to scratchpad again through an
31	+ * actual 'scratchpad move'. */
32	+ SCRATCHPAD_MOVE = 3
33	} scratchpad_state;
34
35	/* The ID of this container before restarting. Necessary to correctly

40	@@ -17,7 +17,7 @@
41	* Gets called upon the command 'move scratchpad'.
42	*
43	*/
44	-void scratchpad_move(Con *con);
45	+void scratchpad_move(Con *con, bool from_show);
46
47	/**
48	* Either shows the top-most scratchpad window (con == NULL) or shows the

53	@@ -1786,7 +1786,7 @@ void cmd_move_scratchpad(I3_CMD) {
54
55	TAILQ_FOREACH(current, &owindows, owindows) {
56	DLOG("matching: %p / %s\n", current->con, current->con->name);
57	- scratchpad_move(current->con);
58	+ scratchpad_move(current->con, false);
59	}
60
61	cmd_output->needs_tree_render = true;

66	@@ -138,7 +138,12 @@ void con_attach(Con con, Con parent, bool ignore_focus) {
67
68	if (con->type == CT_FLOATING_CON) {
69	DLOG("Inserting into floating containers\n");
70	- TAILQ_INSERT_TAIL(&(parent->floating_head), con, floating_windows);
71	+ Con *floating;
72	+ if ((floating = con_inside_floating(con))
73	+ && floating->scratchpad_state == SCRATCHPAD_MOVE)
74	+ TAILQ_INSERT_HEAD(&(parent->floating_head), con, floating_windows);
75	+ else
76	+ TAILQ_INSERT_TAIL(&(parent->floating_head), con, floating_windows);
77	} else {
78	if (!ignore_focus) {
79	/* Get the first tiling container in focus stack */

84	@@ -176,6 +176,9 @@ void dump_node(yajl_gen gen, struct Con *con, bool inplace_restart) {
85	case SCRATCHPAD_CHANGED:
86	ystr("changed");
87	break;
88	+ case SCRATCHPAD_MOVE:
89	+ ystr("move");
90	+ break;
91	}
92
93	ystr("percent");

98	@@ -254,6 +254,8 @@ static int json_string(void ctx, const unsigned char val, unsigned int len) {
99	json_node->scratchpad_state = SCRATCHPAD_FRESH;
100	else if (strcasecmp(buf, "changed") == 0)
101	json_node->scratchpad_state = SCRATCHPAD_CHANGED;
102	+ else if (strcasecmp(buf, "move") == 0)
103	+ json_node->scratchpad_state = SCRATCHPAD_MOVE;
104	free(buf);
105	}
106	}