Allow multiple marks on windows.

This patch allows multiple marks to be set on a single window. The restriction that a mark may
only be on one window at a time is still upheld as this is necessary for commands like
"move window to mark" to make sense.

relates to #2014

docs/userguide

@@ -1659,7 +1659,9 @@ workspace::
 	the special value +\_\_focused__+ to match all windows in the currently
 	focused workspace.
 con_mark::
-	Compares the mark set for this container, see <<vim_like_marks>>.
+        Compares the marks set for this container, see <<vim_like_marks>>. A
+        match is made if any of the container's marks matches the specified
+        mark.
 con_id::
 	Compares the i3-internal container ID, which you can get via the IPC
 	interface. Handy for scripting.

include/con.h

@@ -146,6 +146,12 @@ Con *con_by_frame_id(xcb_window_t frame);
  */
 Con *con_by_mark(const char *mark);
 
+/**
+ * Returns true if and only if the given containers holds the mark.
+ *
+ */
+bool con_has_mark(Con *con, const char *mark);
+
 /**
  * Toggles the mark on a container.
  * If the container already has this mark, the mark is removed.

include/data.h

@@ -46,6 +46,7 @@ typedef struct Con Con;
 typedef struct Match Match;
 typedef struct Assignment Assignment;
 typedef struct Window i3Window;
+typedef struct mark_t mark_t;
 
 /******************************************************************************
  * Helper types
@@ -523,6 +524,12 @@ typedef enum { CF_NONE = 0,
                CF_OUTPUT = 1,
                CF_GLOBAL = 2 } fullscreen_mode_t;
 
+struct mark_t {
+    char *name;
+
+    TAILQ_ENTRY(mark_t) marks;
+};
+
 /**
  * A 'Con' represents everything from the X11 root window down to a single X11 window.
  *
@@ -575,8 +582,8 @@ struct Con {
      * displayed on whichever of the containers is currently visible */
     char *sticky_group;
 
-    /* user-definable mark to jump to this container later */
+    /* user-definable marks to jump to this container later */
-    char *mark;
+    TAILQ_HEAD(marks_head, mark_t) marks_head;
     /* cached to decide whether a redraw is needed */
     bool mark_changed;
 

src/commands.c

@@ -290,10 +290,16 @@ void cmd_criteria_match_windows(I3_CMD) {
                 DLOG("doesnt match\n");
                 free(current);
             }
-        } else if (current_match->mark != NULL && current->con->mark != NULL &&
+        } else if (current_match->mark != NULL && !TAILQ_EMPTY(&(current->con->marks_head))) {
-                   regex_matches(current_match->mark, current->con->mark)) {
+            mark_t *mark;
+            TAILQ_FOREACH(mark, &(current->con->marks_head), marks) {
+                if (!regex_matches(current_match->mark, mark->name))
+                    continue;
+
                 DLOG("match by mark\n");
                 TAILQ_INSERT_TAIL(&owindows, current, owindows);
+                break;
+            }
         } else {
             if (current->con->window && match_matches_window(current_match, current->con->window)) {
                 DLOG("matches window!\n");

src/con.c

@@ -55,6 +55,7 @@ Con *con_new_skeleton(Con *parent, i3Window *window) {
     TAILQ_INIT(&(new->nodes_head));
     TAILQ_INIT(&(new->focus_head));
     TAILQ_INIT(&(new->swallow_head));
+    TAILQ_INIT(&(new->marks_head));
 
     if (parent != NULL)
         con_attach(new, parent, false);
@@ -512,13 +513,27 @@ Con *con_by_frame_id(xcb_window_t frame) {
 Con *con_by_mark(const char *mark) {
     Con *con;
     TAILQ_FOREACH(con, &all_cons, all_cons) {
-        if (con->mark != NULL && strcmp(con->mark, mark) == 0)
+        if (con_has_mark(con, mark))
             return con;
     }
 
     return NULL;
 }
 
+/*
+ * Returns true if and only if the given containers holds the mark.
+ *
+ */
+bool con_has_mark(Con *con, const char *mark) {
+    mark_t *current;
+    TAILQ_FOREACH(current, &(con->marks_head), marks) {
+        if (strcmp(current->name, mark) == 0)
+            return true;
+    }
+
+    return false;
+}
+
 /*
  * Toggles the mark on a container.
  * If the container already has this mark, the mark is removed.
@@ -529,7 +544,7 @@ void con_mark_toggle(Con *con, const char *mark) {
     assert(con != NULL);
     DLOG("Toggling mark \"%s\" on con = %p.\n", mark, con);
 
-    if (con->mark != NULL && strcmp(con->mark, mark) == 0) {
+    if (con_has_mark(con, mark)) {
         con_unmark(mark);
     } else {
         con_mark(con, mark);
@@ -544,22 +559,13 @@ void con_mark(Con *con, const char *mark) {
     assert(con != NULL);
     DLOG("Setting mark \"%s\" on con = %p.\n", mark, con);
 
-    FREE(con->mark);
+    con_unmark(mark);
-    con->mark = sstrdup(mark);
+
+    mark_t *new = scalloc(1, sizeof(mark_t));
+    new->name = sstrdup(mark);
+    TAILQ_INSERT_TAIL(&(con->marks_head), new, marks);
+
     con->mark_changed = true;
-
-    DLOG("Clearing the mark from all other windows.\n");
-    Con *other;
-    TAILQ_FOREACH(other, &all_cons, all_cons) {
-        /* Skip the window we actually handled since we took care of it already. */
-        if (con == other)
-            continue;
-
-        if (other->mark != NULL && strcmp(other->mark, mark) == 0) {
-            FREE(other->mark);
-            other->mark_changed = true;
-        }
-    }
 }
 
 /*
@@ -572,10 +578,17 @@ void con_unmark(const char *mark) {
     if (mark == NULL) {
         DLOG("Unmarking all containers.\n");
         TAILQ_FOREACH(con, &all_cons, all_cons) {
-            if (con->mark == NULL)
+            if (TAILQ_EMPTY(&(con->marks_head)))
                 continue;
 
-            FREE(con->mark);
+            mark_t *current;
+            while (!TAILQ_EMPTY(&(con->marks_head))) {
+                current = TAILQ_FIRST(&(con->marks_head));
+                FREE(current->name);
+                TAILQ_REMOVE(&(con->marks_head), current, marks);
+                FREE(current);
+            }
+
             con->mark_changed = true;
         }
     } else {
@@ -587,8 +600,18 @@ void con_unmark(const char *mark) {
         }
 
         DLOG("Found mark on con = %p. Removing it now.\n", con);
-        FREE(con->mark);
         con->mark_changed = true;
+
+        mark_t *current;
+        TAILQ_FOREACH(current, &(con->marks_head), marks) {
+            if (strcmp(current->name, mark) != 0)
+                continue;
+
+            FREE(current->name);
+            TAILQ_REMOVE(&(con->marks_head), current, marks);
+            FREE(current);
+            break;
+        }
     }
 }
 

src/ipc.c

@@ -275,9 +275,16 @@ void dump_node(yajl_gen gen, struct Con *con, bool inplace_restart) {
     ystr("urgent");
     y(bool, con->urgent);
 
-    if (con->mark != NULL) {
+    if (!TAILQ_EMPTY(&(con->marks_head))) {
-        ystr("mark");
+        ystr("marks");
-        ystr(con->mark);
+        y(array_open);
+
+        mark_t *mark;
+        TAILQ_FOREACH(mark, &(con->marks_head), marks) {
+            ystr(mark->name);
+        }
+
+        y(array_close);
     }
 
     ystr("focused");
@@ -819,9 +826,12 @@ IPC_HANDLER(get_marks) {
     y(array_open);
 
     Con *con;
-    TAILQ_FOREACH(con, &all_cons, all_cons)
+    TAILQ_FOREACH(con, &all_cons, all_cons) {
-    if (con->mark != NULL)
+        mark_t *mark;
-        ystr(con->mark);
+        TAILQ_FOREACH(mark, &(con->marks_head), marks) {
+            ystr(mark->name);
+        }
+    }
 
     y(array_close);
 

src/load_layout.c

@@ -28,6 +28,7 @@ static bool parsing_deco_rect;
 static bool parsing_window_rect;
 static bool parsing_geometry;
 static bool parsing_focus;
+static bool parsing_marks;
 struct Match *current_swallow;
 
 /* This list is used for reordering the focus stack after parsing the 'focus'
@@ -159,12 +160,16 @@ static int json_end_map(void *ctx) {
 
 static int json_end_array(void *ctx) {
     LOG("end of array\n");
-    if (!parsing_swallows && !parsing_focus) {
+    if (!parsing_swallows && !parsing_focus && !parsing_marks) {
         con_fix_percent(json_node);
     }
     if (parsing_swallows) {
         parsing_swallows = false;
     }
+    if (parsing_marks) {
+        parsing_marks = false;
+    }
+
     if (parsing_focus) {
         /* Clear the list of focus mappings */
         struct focus_mapping *mapping;
@@ -214,6 +219,9 @@ static int json_key(void *ctx, const unsigned char *val, size_t len) {
     if (strcasecmp(last_key, "focus") == 0)
         parsing_focus = true;
 
+    if (strcasecmp(last_key, "marks") == 0)
+        parsing_marks = true;
+
     return 1;
 }
 
@@ -234,6 +242,11 @@ static int json_string(void *ctx, const unsigned char *val, size_t len) {
             ELOG("swallow key %s unknown\n", last_key);
         }
         free(sval);
+    } else if (parsing_marks) {
+        char *mark;
+        sasprintf(&mark, "%.*s", (int)len, val);
+
+        con_mark(json_node, mark);
     } else {
         if (strcasecmp(last_key, "name") == 0) {
             json_node->name = scalloc(len + 1, 1);
@@ -336,13 +349,12 @@ static int json_string(void *ctx, const unsigned char *val, size_t len) {
                 LOG("Unhandled \"last_splitlayout\": %s\n", buf);
             free(buf);
         } else if (strcasecmp(last_key, "mark") == 0) {
+            DLOG("Found deprecated key \"mark\".\n");
+
             char *buf = NULL;
             sasprintf(&buf, "%.*s", (int)len, val);
 
-            /* We unmark any containers using this mark to avoid duplicates. */
+            con_mark(json_node, buf);
-            con_unmark(buf);
-
-            json_node->mark = buf;
         } else if (strcasecmp(last_key, "floating") == 0) {
             char *buf = NULL;
             sasprintf(&buf, "%.*s", (int)len, val);
@@ -589,6 +601,7 @@ void tree_append_json(Con *con, const char *filename, char **errormsg) {
     parsing_window_rect = false;
     parsing_geometry = false;
     parsing_focus = false;
+    parsing_marks = false;
     setlocale(LC_NUMERIC, "C");
     stat = yajl_parse(hand, (const unsigned char *)buf, n);
     if (stat != yajl_status_ok) {

src/x.c

@@ -545,11 +545,24 @@ void x_draw_decoration(Con *con) {
     int indent_px = (indent_level * 5) * indent_mult;
 
     int mark_width = 0;
-    if (config.show_marks && con->mark != NULL && (con->mark)[0] != '_') {
+    if (config.show_marks && !TAILQ_EMPTY(&(con->marks_head))) {
-        char *formatted_mark;
+        char *formatted_mark = sstrdup("");
-        sasprintf(&formatted_mark, "[%s]", con->mark);
+        bool had_visible_mark = false;
+
+        mark_t *mark;
+        TAILQ_FOREACH(mark, &(con->marks_head), marks) {
+            if (mark->name[0] == '_')
+                continue;
+            had_visible_mark = true;
+
+            char *buf;
+            sasprintf(&buf, "%s[%s]", formatted_mark, mark->name);
+            free(formatted_mark);
+            formatted_mark = buf;
+        }
+
+        if (had_visible_mark) {
             i3String *mark = i3string_from_utf8(formatted_mark);
-        FREE(formatted_mark);
             mark_width = predict_text_width(mark);
 
             draw_text(mark, parent->pixmap, parent->pm_gc, NULL,
@@ -559,6 +572,9 @@ void x_draw_decoration(Con *con) {
             I3STRING_FREE(mark);
         }
 
+        FREE(formatted_mark);
+    }
+
     i3String *title = win->title_format == NULL ? win->name : window_parse_title_format(win);
     draw_text(title,
               parent->pixmap, parent->pm_gc, NULL,

testcases/t/165-for_window.t

@@ -398,7 +398,7 @@ EOT
 
     my @nodes = @{get_ws($tmp)->{floating_nodes}};
     cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
-    is($nodes[0]->{nodes}[0]->{mark}, 'branded', "mark set (window_type = $atom)");
+    is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'branded' ], "mark set (window_type = $atom)");
 
     exit_gracefully($pid);
 
@@ -431,7 +431,7 @@ EOT
 
     my @nodes = @{get_ws($tmp)->{floating_nodes}};
     cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
-    is($nodes[0]->{nodes}[0]->{mark}, 'branded', "mark set (window_type = $atom)");
+    is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'branded' ], "mark set (window_type = $atom)");
 
     exit_gracefully($pid);
 
@@ -454,7 +454,7 @@ $window = open_window;
 
 @nodes = @{get_ws('trigger')->{floating_nodes}};
 cmp_ok(@nodes, '==', 1, 'one floating container on this workspace');
-is($nodes[0]->{nodes}[0]->{mark}, 'triggered', "mark set for workspace criterion");
+is_deeply($nodes[0]->{nodes}[0]->{marks}, [ 'triggered' ], "mark set for workspace criterion");
 
 exit_gracefully($pid);
 

testcases/t/210-mark-unmark.t

@@ -28,7 +28,7 @@ sub get_mark_for_window_on_workspace {
     my ($ws, $con) = @_;
 
     my $current = first { $_->{window} == $con->{id} } @{get_ws_content($ws)};
-    return $current->{mark};
+    return $current->{marks};
 }
 
 ##############################################################
@@ -41,7 +41,6 @@ cmd 'split h';
 
 is_deeply(get_marks(), [], 'no marks set yet');
 
-
 ##############################################################
 # 2: mark a con, check that it's marked, unmark it, check that
 ##############################################################
@@ -98,7 +97,7 @@ cmd 'mark important';
 cmd 'focus left';
 cmd 'mark important';
 
-is(get_mark_for_window_on_workspace($tmp, $first), 'important', 'first container now has the mark');
+is_deeply(get_mark_for_window_on_workspace($tmp, $first), [ 'important' ], 'first container now has the mark');
 ok(!get_mark_for_window_on_workspace($tmp, $second), 'second container lost the mark');
 
 ##############################################################
@@ -116,20 +115,10 @@ ok(!get_mark_for_window_on_workspace($tmp, $con), 'container no longer has the m
 
 $con = open_window;
 cmd 'mark --toggle important';
-is(get_mark_for_window_on_workspace($tmp, $con), 'important', 'container now has the mark');
+is_deeply(get_mark_for_window_on_workspace($tmp, $con), [ 'important' ], 'container now has the mark');
 
 ##############################################################
-# 7: mark a con, toggle a different mark, check it is marked
+# 7: mark a con, toggle the mark on another con,
-#    with the new mark
-##############################################################
-
-$con = open_window;
-cmd 'mark boring';
-cmd 'mark --toggle important';
-is(get_mark_for_window_on_workspace($tmp, $con), 'important', 'container has the most recent mark');
-
-##############################################################
-# 8: mark a con, toggle the mark on another con,
 #    check only the latter has the mark
 ##############################################################
 
@@ -140,11 +129,11 @@ cmd 'mark important';
 cmd 'focus left';
 cmd 'mark --toggle important';
 
-is(get_mark_for_window_on_workspace($tmp, $first), 'important', 'left container has the mark now');
+is_deeply(get_mark_for_window_on_workspace($tmp, $first), [ 'important' ], 'left container has the mark now');
 ok(!get_mark_for_window_on_workspace($tmp, $second), 'second containr no longer has the mark');
 
 ##############################################################
-# 9: try to mark two cons with the same mark and check that
+# 8: try to mark two cons with the same mark and check that
 #    it fails
 ##############################################################
 

testcases/t/235-wm-class-change-handler.t

@@ -63,7 +63,7 @@ is($con->{window_properties}->{instance}, 'special',
 
 # The mark `special_class_mark` is added in a `for_window` assignment in the
 # config for testing purposes
-is($con->{mark}, 'special_class_mark',
+is_deeply($con->{marks}, [ 'special_class_mark' ],
     'A `for_window` assignment should run for a match when the window changes class');
 
 change_window_class($win, "abcdefghijklmnopqrstuv\0abcd", 24);

testcases/t/255-multiple-marks.t

@@ -0,0 +1,96 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • http://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • http://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • http://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Tests for mark/unmark with multiple marks on a single window.
+# Ticket: #2014
+use i3test;
+use List::Util qw(first);
+
+my ($ws, $con, $first, $second);
+
+sub get_marks {
+    return i3(get_socket_path())->get_marks->recv;
+}
+
+sub get_mark_for_window_on_workspace {
+    my ($ws, $con) = @_;
+
+    my $current = first { $_->{window} == $con->{id} } @{get_ws_content($ws)};
+    return $current->{marks};
+}
+
+###############################################################################
+# Verify that multiple marks can be set on a window.
+###############################################################################
+
+$ws = fresh_workspace;
+$con = open_window;
+cmd 'mark A';
+cmd 'mark B';
+
+is_deeply(sort(get_marks()), [ 'A', 'B' ], 'both marks exist');
+is_deeply(get_mark_for_window_on_workspace($ws, $con), [ 'A', 'B' ], 'both marks are on the same window');
+
+cmd 'unmark';
+
+###############################################################################
+# Verify that toggling a mark can affect only the specified mark.
+###############################################################################
+
+$ws = fresh_workspace;
+$con = open_window;
+cmd 'mark A';
+
+cmd 'mark --toggle B';
+is_deeply(get_mark_for_window_on_workspace($ws, $con), [ 'A', 'B' ], 'both marks are on the same window');
+cmd 'mark --toggle B';
+is_deeply(get_mark_for_window_on_workspace($ws, $con), [ 'A' ], 'only mark B has been removed');
+
+cmd 'unmark';
+
+###############################################################################
+# Verify that unmarking a mark leaves other marks on the same window intact.
+###############################################################################
+
+$ws = fresh_workspace;
+$con = open_window;
+cmd 'mark A';
+cmd 'mark B';
+cmd 'mark C';
+
+cmd 'unmark B';
+is_deeply(get_mark_for_window_on_workspace($ws, $con), [ 'A', 'C' ], 'only mark B has been removed');
+
+cmd 'unmark';
+
+###############################################################################
+# Verify that matching via mark works on windows with multiple marks.
+###############################################################################
+
+$ws = fresh_workspace;
+$con = open_window;
+cmd 'mark A';
+cmd 'mark B';
+open_window;
+
+cmd '[con_mark=B] mark C';
+is_deeply(get_mark_for_window_on_workspace($ws, $con), [ 'A', 'B', 'C' ], 'matching on a mark works with multiple marks');
+
+cmd 'unmark';
+
+###############################################################################
+
+done_testing;