Use mask to determine workspace gaps assignments (#5283)

Fixes #5282

4 files changed, 167 insertions, 132 deletions

diff --git a/include/data.h b/include/data.h
index 67a573c2..483aecab 100644
--- a/include/data.h
+++ b/include/data.h
@@ -155,6 +155,17 @@ struct gaps_t {
     int left;
 };
 
+typedef enum {
+    GAPS_INNER = (1 << 0),
+    GAPS_TOP = (1 << 1),
+    GAPS_RIGHT = (1 << 2),
+    GAPS_BOTTOM = (1 << 3),
+    GAPS_LEFT = (1 << 4),
+    GAPS_VERTICAL = (GAPS_TOP | GAPS_BOTTOM),
+    GAPS_HORIZONTAL = (GAPS_RIGHT | GAPS_LEFT),
+    GAPS_OUTER = (GAPS_VERTICAL | GAPS_HORIZONTAL),
+} gaps_mask_t;
+
 /**
  * Focus wrapping modes.
  */
@@ -229,6 +240,7 @@ struct Workspace_Assignment {
     char *name;
     char *output;
     gaps_t gaps;
+    gaps_mask_t gaps_mask;
 
     TAILQ_ENTRY(Workspace_Assignment) ws_assignments;
 };
diff --git a/src/config_directives.c b/src/config_directives.c
index 7f50b933..9077fe98 100644
--- a/src/config_directives.c
+++ b/src/config_directives.c
@@ -231,143 +231,85 @@ CFGFUN(for_window, const char *command) {
     TAILQ_INSERT_TAIL(&assignments, assignment, assignments);
 }
 
-static void create_gaps_assignment(const char *workspace, const char *scope, gaps_t gaps) {
+static void apply_gaps(gaps_t *gaps, gaps_mask_t mask, int value) {
+    if (gaps == NULL) {
+        return;
+    }
+    if (mask & GAPS_INNER) {
+        gaps->inner = value;
+    }
+    if (mask & GAPS_TOP) {
+        gaps->top = value;
+    }
+    if (mask & GAPS_RIGHT) {
+        gaps->right = value;
+    }
+    if (mask & GAPS_BOTTOM) {
+        gaps->bottom = value;
+    }
+    if (mask & GAPS_LEFT) {
+        gaps->left = value;
+    }
+}
+
+static void create_gaps_assignment(const char *workspace, const gaps_mask_t mask, const int pixels) {
+    if (mask == 0) {
+        return;
+    }
+
     DLOG("Setting gaps for workspace %s", workspace);
 
+    bool found = false;
     struct Workspace_Assignment *assignment;
     TAILQ_FOREACH (assignment, &ws_assignments, ws_assignments) {
         if (strcasecmp(assignment->name, workspace) == 0) {
-            if (!strcmp(scope, "inner")) {
-                assignment->gaps.inner = gaps.inner;
-            } else if (!strcmp(scope, "outer")) {
-                assignment->gaps.top = gaps.top;
-                assignment->gaps.right = gaps.right;
-                assignment->gaps.bottom = gaps.bottom;
-                assignment->gaps.left = gaps.left;
-            } else if (!strcmp(scope, "vertical")) {
-                assignment->gaps.top = gaps.top;
-                assignment->gaps.bottom = gaps.bottom;
-            } else if (!strcmp(scope, "horizontal")) {
-                assignment->gaps.right = gaps.right;
-                assignment->gaps.left = gaps.left;
-            } else if (!strcmp(scope, "top")) {
-                assignment->gaps.top = gaps.top;
-            } else if (!strcmp(scope, "right")) {
-                assignment->gaps.right = gaps.right;
-            } else if (!strcmp(scope, "bottom")) {
-                assignment->gaps.bottom = gaps.bottom;
-            } else if (!strcmp(scope, "left")) {
-                assignment->gaps.left = gaps.left;
-            } else {
-                ELOG("Invalid command, cannot process scope %s", scope);
-            }
-
-            return;
+            found = true;
+            break;
         }
     }
 
-    // Assignment does not yet exist, let's create it.
-    assignment = scalloc(1, sizeof(struct Workspace_Assignment));
-    assignment->name = sstrdup(workspace);
-    assignment->output = NULL;
+    /* Assignment does not yet exist, let's create it. */
+    if (!found) {
+        assignment = scalloc(1, sizeof(struct Workspace_Assignment));
+        assignment->name = sstrdup(workspace);
+        assignment->output = NULL;
+        TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments);
+    }
+
+    assignment->gaps_mask |= mask;
+    apply_gaps(&assignment->gaps, mask, pixels);
+}
+
+static gaps_mask_t gaps_scope_to_mask(const char *scope) {
     if (!strcmp(scope, "inner")) {
-        assignment->gaps.inner = gaps.inner;
+        return GAPS_INNER;
     } else if (!strcmp(scope, "outer")) {
-        assignment->gaps.top = gaps.top;
-        assignment->gaps.right = gaps.right;
-        assignment->gaps.bottom = gaps.bottom;
-        assignment->gaps.left = gaps.left;
+        return GAPS_OUTER;
     } else if (!strcmp(scope, "vertical")) {
-        assignment->gaps.top = gaps.top;
-        assignment->gaps.bottom = gaps.bottom;
+        return GAPS_VERTICAL;
     } else if (!strcmp(scope, "horizontal")) {
-        assignment->gaps.right = gaps.right;
-        assignment->gaps.left = gaps.left;
+        return GAPS_HORIZONTAL;
     } else if (!strcmp(scope, "top")) {
-        assignment->gaps.top = gaps.top;
+        return GAPS_TOP;
     } else if (!strcmp(scope, "right")) {
-        assignment->gaps.right = gaps.right;
+        return GAPS_RIGHT;
     } else if (!strcmp(scope, "bottom")) {
-        assignment->gaps.bottom = gaps.bottom;
+        return GAPS_BOTTOM;
     } else if (!strcmp(scope, "left")) {
-        assignment->gaps.left = gaps.left;
-    } else {
-        ELOG("Invalid command, cannot process scope %s", scope);
+        return GAPS_LEFT;
     }
-    TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments);
+    ELOG("Invalid command, cannot process scope %s", scope);
+    return 0;
 }
 
 CFGFUN(gaps, const char *workspace, const char *scope, const long value) {
     int pixels = logical_px(value);
-    gaps_t gaps = (gaps_t){0, 0, 0, 0, 0};
-    if (!strcmp(scope, "inner")) {
-        if (workspace == NULL)
-            config.gaps.inner = pixels;
-        else {
-            gaps.inner = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "outer")) {
-        if (workspace == NULL) {
-            config.gaps.top = pixels;
-            config.gaps.right = pixels;
-            config.gaps.bottom = pixels;
-            config.gaps.left = pixels;
-        } else {
-            gaps.top = pixels;
-            gaps.right = pixels;
-            gaps.bottom = pixels;
-            gaps.left = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "vertical")) {
-        if (workspace == NULL) {
-            config.gaps.top = pixels;
-            config.gaps.bottom = pixels;
-        } else {
-            gaps.top = pixels;
-            gaps.bottom = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "horizontal")) {
-        if (workspace == NULL) {
-            config.gaps.right = pixels;
-            config.gaps.left = pixels;
-        } else {
-            gaps.right = pixels;
-            gaps.left = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "top")) {
-        if (workspace == NULL)
-            config.gaps.top = pixels;
-        else {
-            gaps.top = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "right")) {
-        if (workspace == NULL)
-            config.gaps.right = pixels;
-        else {
-            gaps.right = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "bottom")) {
-        if (workspace == NULL)
-            config.gaps.bottom = pixels;
-        else {
-            gaps.bottom = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
-    } else if (!strcmp(scope, "left")) {
-        if (workspace == NULL)
-            config.gaps.left = pixels;
-        else {
-            gaps.left = pixels;
-            create_gaps_assignment(workspace, scope, gaps);
-        }
+    gaps_mask_t mask = gaps_scope_to_mask(scope);
+
+    if (workspace == NULL) {
+        apply_gaps(&config.gaps, mask, pixels);
     } else {
-        ELOG("Invalid command, cannot process scope %s", scope);
+        create_gaps_assignment(workspace, mask, pixels);
     }
 }
 
diff --git a/src/gaps.c b/src/gaps.c
index 59c25a34..5d29faa9 100644
--- a/src/gaps.c
+++ b/src/gaps.c
@@ -117,29 +117,35 @@ bool gaps_has_adjacent_container(Con *con, direction_t direction) {
  */
 gaps_t gaps_for_workspace(Con *ws) {
     gaps_t gaps = (gaps_t){0, 0, 0, 0, 0};
+    gaps_mask_t mask = 0;
     struct Workspace_Assignment *assignment;
     TAILQ_FOREACH (assignment, &ws_assignments, ws_assignments) {
         if (strcmp(assignment->name, ws->name) == 0) {
             gaps = assignment->gaps;
+            mask = assignment->gaps_mask;
             break;
         } else if (ws->num != -1 && name_is_digits(assignment->name) && ws_name_to_number(assignment->name) == ws->num) {
             gaps = assignment->gaps;
+            mask = assignment->gaps_mask;
         }
     }
+    if (mask == 0) {
+        return gaps;
+    }
 
-    if (gaps.inner != 0) {
+    if (mask & GAPS_INNER) {
         gaps.inner -= config.gaps.inner;
     }
-    if (gaps.top != 0) {
+    if (mask & GAPS_TOP) {
         gaps.top -= config.gaps.top;
     }
-    if (gaps.right != 0) {
+    if (mask & GAPS_RIGHT) {
         gaps.right -= config.gaps.right;
     }
-    if (gaps.bottom != 0) {
+    if (mask & GAPS_BOTTOM) {
         gaps.bottom -= config.gaps.bottom;
     }
-    if (gaps.left != 0) {
+    if (mask & GAPS_LEFT) {
         gaps.left -= config.gaps.left;
     }
 
diff --git a/testcases/t/319-gaps.t b/testcases/t/319-gaps.t
index 903a4845..fa723dfc 100644
--- a/testcases/t/319-gaps.t
+++ b/testcases/t/319-gaps.t
@@ -44,6 +44,10 @@ my $inner_gaps = 10;
 my $total_gaps = $outer_gaps + $inner_gaps;
 
 sub is_gaps {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+    sync_with_i3;
+
     my $left_rect = $left->rect;
     my $right_rect = $right->rect;
 
@@ -79,12 +83,10 @@ is_gaps();
 fresh_workspace;
 cmd 'gaps outer current set 30px';
 cmd "workspace $tmp";
-sync_with_i3;
 is_gaps();
 
 # Verify global gaps do influence all workspaces
 cmd 'gaps outer all set 30px';
-sync_with_i3;
 
 $outer_gaps = 30;
 $total_gaps = $outer_gaps + $inner_gaps;
@@ -93,9 +95,12 @@ is_gaps();
 # Verify negative outer gaps compensate inner gaps, resulting only in gaps
 # in between adjacent windows or split containers, not towards the screen edges.
 cmd 'gaps outer all set -10px';
-sync_with_i3;
 
 sub is_gaps_in_between_only {
+    local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+    sync_with_i3;
+
     my $left_rect = $left->rect;
     my $right_rect = $right->rect;
 
@@ -128,7 +133,6 @@ is_gaps_in_between_only();
 cmd 'gaps inner all set 6px';
 $inner_gaps = 6;
 $total_gaps = $outer_gaps + $inner_gaps;
-sync_with_i3;
 is_gaps_in_between_only();
 
 exit_gracefully($pid);
@@ -154,12 +158,10 @@ cmd 'workspace 2';
 
 $left = open_window;
 $right = open_window;
-sync_with_i3;
 
 $inner_gaps = 16;
 $outer_gaps = 0;
 $total_gaps = $outer_gaps + $inner_gaps;
-
 is_gaps();
 
 exit_gracefully($pid);
@@ -193,12 +195,10 @@ cmd 'splith';
 cmd 'layout stacked';
 sync_with_i3;
 $helper->destroy;
-sync_with_i3;
 
 $inner_gaps = 10;
 $outer_gaps = 0;
 $total_gaps = $outer_gaps + $inner_gaps;
-
 is_gaps();
 
 exit_gracefully($pid);
@@ -253,8 +253,6 @@ cmd 'workspace 2';
 
 $left = open_window;
 $right = open_window;
-sync_with_i3;
-
 is_gaps();
 
 my $version = i3()->get_version()->recv;
@@ -273,14 +271,91 @@ close($configfh);
 
 cmd 'reload';
 
-sync_with_i3;
-
 $inner_gaps = 16;
 $outer_gaps = 0;
 $total_gaps = $outer_gaps + $inner_gaps;
+is_gaps();
+
+exit_gracefully($pid);
+
+################################################################################
+# Ensure removing gaps from workspace works (issue #5282).
+################################################################################
+
+$config = <<EOT;
+# i3 config file (v4)
+font -misc-fixed-medium-r-normal--13-120-75-75-C-70-iso10646-1
+
+gaps inner 33
+gaps outer 22
+workspace 1 gaps outer 0
+workspace 1 gaps inner 0
+workspace 2 gaps outer 10
+workspace 2 gaps inner 0
+workspace 3 gaps outer 0
+workspace 3 gaps inner 10
+workspace 4 gaps left 10
+workspace 4 gaps top 20
+workspace 4 gaps inner 0
+workspace 4 gaps bottom 0
+workspace 4 gaps right 0
 
+default_border pixel 0
+EOT
+
+$pid = launch_with_config($config);
+
+# Everything disabled
+cmd 'workspace 1';
+kill_all_windows;
+$left = open_window;
+$right = open_window;
+
+$inner_gaps = 0;
+$total_gaps = 0;
+is_gaps();
+
+# Inner disabled
+cmd 'workspace 2';
+$left = open_window;
+$right = open_window;
+
+$inner_gaps = 0;
+$total_gaps = 10;
+is_gaps();
+
+# Outer disabled
+cmd 'workspace 3';
+$left = open_window;
+$right = open_window;
+
+$inner_gaps = 10;
+$total_gaps = 10;
 is_gaps();
 
+# More complicated example
+cmd 'workspace 4';
+$left = open_window;
+$right = open_window;
+sync_with_i3;
+
+my $left_rect = $left->rect;
+my $right_rect = $right->rect;
+is_deeply($left_rect, {
+x => 10,
+y => 20,
+width => $screen_width/2 - 10/2,
+height => $screen_height - 20,
+}, 'left window position and size matches gaps expectations');
+
+is_deeply($right_rect, {
+x => $left_rect->x + $left_rect->width,
+y => 20,
+width => $screen_width/2 - 10/2,
+height => $left_rect->height,
+}, 'right window position and size matches gaps expectations');
+
+
 exit_gracefully($pid);
 
 done_testing;