Use mask to determine workspace gaps assignments (#5283)

Fixes #5282
2022-11-13 16:03:58 +01:00 · 2022-11-13 16:03:58 +01:00 · 029cb8af19
parent 60c3fedb73
commit 029cb8af19
4 changed files with 176 additions and 141 deletions
--- 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;
 };
--- 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;
-    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);
+    /* 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);
    }
-    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")) {
+        return GAPS_INNER;
+    } else if (!strcmp(scope, "outer")) {
+        return GAPS_OUTER;
+    } else if (!strcmp(scope, "vertical")) {
+        return GAPS_VERTICAL;
+    } else if (!strcmp(scope, "horizontal")) {
+        return GAPS_HORIZONTAL;
+    } else if (!strcmp(scope, "top")) {
+        return GAPS_TOP;
+    } else if (!strcmp(scope, "right")) {
+        return GAPS_RIGHT;
+    } else if (!strcmp(scope, "bottom")) {
+        return GAPS_BOTTOM;
+    } else if (!strcmp(scope, "left")) {
+        return GAPS_LEFT;
+    }
+    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);
    }
 }

--- 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;
    }

--- 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;