tiling_drag: Allow swapping containers (#6084)

This was originally mentioned in #3085 but left for a future PR.

One of the noticeable limitations is that pressing the modifier while
the drag is already initiated, will not swap the containers but instead
cancel the drag. This is because of how `drag_pointer()` is written and
would be quite an involved case to handle it.

docs/userguide

@@ -216,6 +216,10 @@ Drop on container::
     This happens when the mouse is relatively near the center of a container.
     If the mouse is released, the result is exactly as if you had run the
     +move container to mark+ command. See <<move_to_mark>>.
+    If the swap modifier is pressed before initiating the drag (+tiling_drag
+    swap_modifier+ set to Shift by default), the containers are swapped
+    instead. In that case, the result is exactly as if you had run the +swap
+    container with mark+ command. See <<swapping_containers>>.
 Drop as sibling::
     This happens when the mouse is relatively near the edge of a container. If
     the mouse is released, the dragged container will become a sibling of the
@@ -1429,10 +1433,16 @@ You can configure how to initiate the tiling drag feature (see <<tiling_drag>>).
 
 The default is +modifier+.
 
+Since i3 4.24, you can configure a modifier key which, when pressed, will swap
+instead of moving containers when dropping directly onto another container.
+Defaults to +Shift+. Note that you have to be pressing both the floating
+modifer and the swap modifier before the drag is initiated.
+
 *Syntax*:
 --------------------------------
 tiling_drag off
 tiling_drag modifier|titlebar [modifier|titlebar]
+tiling_drag swap_modifier <modifier>
 --------------------------------
 
 *Examples*:
@@ -1445,6 +1455,14 @@ tiling_drag modifier titlebar
 
 # Disable tiling drag altogether
 tiling_drag off
+
+# Use Control to swap containers
+tiling_drag swap_modifier Control
+
+# Setting the swap_modifier to be the same key as the floating modifier will
+# always swap without the need to hold two keys
+floating_modifier Mod4
+tiling_drag swap_modifier Mod4
 --------------------------------
 
 [[gaps]]
@@ -2546,6 +2564,7 @@ bindsym $mod+c move absolute position center
 bindsym $mod+m move position mouse
 -------------------------------------------------------
 
+[[swapping_containers]]
 === Swapping containers
 
 Two containers can be swapped (i.e., move to each other's position) by using

include/config_directives.h

@@ -69,6 +69,7 @@ CFGFUN(no_focus);
 CFGFUN(ipc_socket, const char *path);
 CFGFUN(ipc_kill_timeout, const long timeout_ms);
 CFGFUN(tiling_drag, const char *value);
+CFGFUN(tiling_drag_swap_modifier, const char *modifiers);
 CFGFUN(restart_state, const char *path);
 CFGFUN(popup_during_fullscreen, const char *value);
 CFGFUN(color, const char *colorclass, const char *border, const char *background, const char *text, const char *indicator, const char *child_border);

include/configuration.h

@@ -227,6 +227,9 @@ struct Config {
     /** The modifier which needs to be pressed in combination with your mouse
      * buttons to do things with floating windows (move, resize) */
     uint32_t floating_modifier;
+    /** The modifier which needs to be pressed in combination with the floating
+     * modifier and your mouse buttons to swap containers during tiling drag */
+    uint32_t swap_modifier;
 
     /** Maximum and minimum dimensions of a floating window */
     int32_t floating_maximum_width;

parser-specs/config.spec

@@ -140,6 +140,15 @@ state FLOATING_MODIFIER:
   end
       -> call cfg_floating_modifier($modifiers)
 
+# tiling_drag swap_modifier <modifier>
+state TILING_DRAG_SWAP_MODIFIER:
+  modifiers = 'Mod1', 'Mod2', 'Mod3', 'Mod4', 'Mod5', 'Shift', 'Control', 'Ctrl'
+      ->
+  '+'
+      ->
+  end
+      -> call cfg_tiling_drag_swap_modifier($modifiers)
+
 # default_orientation <horizontal|vertical|auto>
 state DEFAULT_ORIENTATION:
   orientation = 'horizontal', 'vertical', 'auto'
@@ -378,6 +387,8 @@ state TILING_DRAG_MODE:
 state TILING_DRAG:
   off = '0', 'no', 'false', 'off', 'disable', 'inactive'
       -> call cfg_tiling_drag($off)
+  swap_modifier = 'swap_modifier'
+      -> TILING_DRAG_SWAP_MODIFIER
   value = 'modifier', 'titlebar'
       -> TILING_DRAG_MODE
 

release-notes/changes/1-swap-drag

@@ -0,0 +1 @@
+swap containers with the mouse

src/click.c

@@ -166,7 +166,10 @@ static void allow_replay_pointer(xcb_timestamp_t time) {
  * functions for resizing/dragging.
  *
  */
-static void route_click(Con *con, xcb_button_press_event_t *event, const bool mod_pressed, const click_destination_t dest) {
+static void route_click(Con *con, xcb_button_press_event_t *event, const click_destination_t dest) {
+    const uint32_t mod = (config.floating_modifier & 0xFFFF);
+    const bool mod_pressed = (mod != 0 && (event->state & mod) == mod);
+
     DLOG("--> click properties: mod = %d, destination = %d\n", mod_pressed, dest);
     DLOG("--> OUTCOME = %p\n", con);
     DLOG("type = %d, name = %s\n", con->type, con->name);
@@ -375,11 +378,8 @@ void handle_button_press(xcb_button_press_event_t *event) {
 
     last_timestamp = event->time;
 
-    const uint32_t mod = (config.floating_modifier & 0xFFFF);
-    const bool mod_pressed = (mod != 0 && (event->state & mod) == mod);
-    DLOG("floating_mod = %d, detail = %d\n", mod_pressed, event->detail);
     if ((con = con_by_window_id(event->event))) {
-        route_click(con, event, mod_pressed, CLICK_INSIDE);
+        route_click(con, event, CLICK_INSIDE);
         return;
     }
 
@@ -424,7 +424,7 @@ void handle_button_press(xcb_button_press_event_t *event) {
     /* Check if the click was on the decoration of a child */
     if (con->window != NULL) {
         if (rect_contains(con->deco_rect, event->event_x, event->event_y)) {
-            route_click(con, event, mod_pressed, CLICK_DECORATION);
+            route_click(con, event, CLICK_DECORATION);
             return;
         }
     } else {
@@ -434,16 +434,16 @@ void handle_button_press(xcb_button_press_event_t *event) {
                 continue;
             }
 
-            route_click(child, event, mod_pressed, CLICK_DECORATION);
+            route_click(child, event, CLICK_DECORATION);
             return;
         }
     }
 
     if (event->child != XCB_NONE) {
         DLOG("event->child not XCB_NONE, so this is an event which originated from a click into the application, but the application did not handle it.\n");
-        route_click(con, event, mod_pressed, CLICK_INSIDE);
+        route_click(con, event, CLICK_INSIDE);
         return;
     }
 
-    route_click(con, event, mod_pressed, CLICK_BORDER);
+    route_click(con, event, CLICK_BORDER);
 }

src/config.c

@@ -233,6 +233,7 @@ bool load_configuration(const char *override_configpath, config_load_t load_type
     config.focus_wrapping = FOCUS_WRAPPING_ON;
 
     config.tiling_drag = TILING_DRAG_MODIFIER;
+    config.swap_modifier = XCB_KEY_BUT_MASK_SHIFT;
 
     FREE(current_configpath);
     current_configpath = get_config_path(override_configpath, true);

src/config_directives.c

@@ -361,6 +361,10 @@ CFGFUN(floating_modifier, const char *modifiers) {
     config.floating_modifier = event_state_from_str(modifiers);
 }
 
+CFGFUN(tiling_drag_swap_modifier, const char *modifiers) {
+    config.swap_modifier = event_state_from_str(modifiers);
+}
+
 CFGFUN(default_orientation, const char *orientation) {
     if (strcmp(orientation, "horizontal") == 0) {
         config.default_orientation = HORIZ;

src/tiling_drag.c

@@ -338,7 +338,15 @@ void tiling_drag(Con *con, xcb_button_press_event_t *event, bool use_threshold)
         case DT_CENTER:
             /* Also handles workspaces.*/
             DLOG("drop to center of %p\n", target);
-            con_move_to_target(con, target);
+            const uint32_t mod = (config.swap_modifier & 0xFFFF);
+            const bool swap_pressed = (mod != 0 && (event->state & mod) == mod);
+            if (swap_pressed) {
+                if (!con_swap(con, target)) {
+                    return;
+                }
+            } else {
+                con_move_to_target(con, target);
+            }
             break;
         case DT_SIBLING:
             DLOG("drop %s %p\n", position_to_string(position), target);

testcases/t/316-drag-container.t

@@ -43,7 +43,7 @@ sub start_drag {
     $x->root->warp_pointer($pos_x, $pos_y);
     sync_with_i3;
 
-    xtest_key_press(64);        # Alt_L
+    xtest_key_press(64); # Alt_L
     xtest_button_press(1, $pos_x, $pos_y);
     xtest_sync_with_i3;
 }
@@ -56,7 +56,7 @@ sub end_drag {
     sync_with_i3;
 
     xtest_button_release(1, $pos_x, $pos_y);
-    xtest_key_release(64);      # Alt_L
+    xtest_key_release(64); # Alt_L
     xtest_sync_with_i3;
 }
 
@@ -104,6 +104,30 @@ end_drag(1050, 50);
 
 is($x->input_focus, $A->id, 'Tiling window moved to the right workspace');
 is($ws2, focused_ws, 'Empty workspace focused after tiling window dragged to it');
+is(@{get_ws_content($ws1)}, 0, 'No container left in ws1');
+is(@{get_ws_content($ws2)}, 1, 'One container in ws2');
+
+};
+
+###############################################################################
+# Swap-drag tiling container onto an empty workspace.
+###############################################################################
+
+subtest "Swap tiling container with an empty workspace does nothing", sub {
+
+$ws2 = fresh_workspace(output => 1);
+$ws1 = fresh_workspace(output => 0);
+$A = open_window;
+
+xtest_key_press(50); # Shift
+start_drag(50, 50);
+end_drag(1050, 50);
+xtest_key_release(50); # Shift
+
+is($x->input_focus, $A->id, 'Tiling window still focused');
+is($ws1, focused_ws, 'Same workspace focused');
+is(@{get_ws_content($ws1)}, 1, 'One container still in ws1');
+is(@{get_ws_content($ws2)}, 0, 'No container in ws2');
 
 };
 
@@ -152,6 +176,32 @@ is($ws2->{focus}[1], $B_id, 'B focused second');
 
 };
 
+###############################################################################
+# Swap-drag tiling container onto a tiling container on an other workspace.
+###############################################################################
+
+subtest "Swap tiling container with a tiling container on an other workspace produces move event", sub {
+
+$ws2 = fresh_workspace(output => 1);
+open_window;
+$B_id = get_focused($ws2);
+$ws1 = fresh_workspace(output => 0);
+$A = open_window;
+$A_id = get_focused($ws1);
+
+xtest_key_press(50); # Shift
+start_drag(50, 50);
+end_drag(1500, 250);  # Center of right output, inner region.
+xtest_key_release(50); # Shift
+
+is($ws2, focused_ws, 'Workspace focused after tiling window dragged to it');
+$ws2 = get_ws($ws2);
+is($ws2->{focus}[0], $A_id, 'A focused first, dragged container kept focus');
+$ws1 = get_ws($ws1);
+is($ws1->{focus}[0], $B_id, 'B now in first workspace');
+
+};
+
 ###############################################################################
 # Drag tiling container onto a floating container on an other workspace.
 ###############################################################################