From 9bb2f038ab6229a53588771af400d5a8e0ba61ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Mon, 19 Oct 2015 18:10:20 +0200 Subject: [PATCH] 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 | 4 +- include/con.h | 6 ++ include/data.h | 11 ++- src/commands.c | 14 +++- src/con.c | 63 ++++++++++----- src/ipc.c | 22 ++++-- src/load_layout.c | 23 ++++-- src/x.c | 38 ++++++--- testcases/t/165-for_window.t | 6 +- testcases/t/210-mark-unmark.t | 23 ++---- testcases/t/235-wm-class-change-handler.t | 2 +- testcases/t/255-multiple-marks.t | 96 +++++++++++++++++++++++ 12 files changed, 238 insertions(+), 70 deletions(-) create mode 100644 testcases/t/255-multiple-marks.t diff --git a/docs/userguide b/docs/userguide index a94896cc..f80f19f9 100644 --- a/docs/userguide +++ b/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 <>. + Compares the marks set for this container, see <>. 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. diff --git a/include/con.h b/include/con.h index 16ea6cfa..df94c1ae 100644 --- a/include/con.h +++ b/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. diff --git a/include/data.h b/include/data.h index 58e4a00d..ee7f82c0 100644 --- a/include/data.h +++ b/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 */ - char *mark; + /* user-definable marks to jump to this container later */ + TAILQ_HEAD(marks_head, mark_t) marks_head; /* cached to decide whether a redraw is needed */ bool mark_changed; diff --git a/src/commands.c b/src/commands.c index c9746ba4..ab9a4135 100644 --- a/src/commands.c +++ b/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 && - regex_matches(current_match->mark, current->con->mark)) { - DLOG("match by mark\n"); - TAILQ_INSERT_TAIL(&owindows, current, owindows); + } else if (current_match->mark != NULL && !TAILQ_EMPTY(&(current->con->marks_head))) { + 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"); diff --git a/src/con.c b/src/con.c index 5b0cc6c8..bf7dd545 100644 --- a/src/con.c +++ b/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->mark = sstrdup(mark); + con_unmark(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; + } } } diff --git a/src/ipc.c b/src/ipc.c index c884cc8e..68cc417a 100644 --- a/src/ipc.c +++ b/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) { - ystr("mark"); - ystr(con->mark); + if (!TAILQ_EMPTY(&(con->marks_head))) { + ystr("marks"); + 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) - if (con->mark != NULL) - ystr(con->mark); + TAILQ_FOREACH(con, &all_cons, all_cons) { + mark_t *mark; + TAILQ_FOREACH(mark, &(con->marks_head), marks) { + ystr(mark->name); + } + } y(array_close); diff --git a/src/load_layout.c b/src/load_layout.c index 4a67e6b1..68c4f4a2 100644 --- a/src/load_layout.c +++ b/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_unmark(buf); - - json_node->mark = buf; + con_mark(json_node, 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) { diff --git a/src/x.c b/src/x.c index d312666b..094a33f0 100644 --- a/src/x.c +++ b/src/x.c @@ -545,18 +545,34 @@ 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] != '_') { - char *formatted_mark; - sasprintf(&formatted_mark, "[%s]", con->mark); - i3String *mark = i3string_from_utf8(formatted_mark); + if (config.show_marks && !TAILQ_EMPTY(&(con->marks_head))) { + char *formatted_mark = sstrdup(""); + 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); + mark_width = predict_text_width(mark); + + draw_text(mark, parent->pixmap, parent->pm_gc, NULL, + con->deco_rect.x + con->deco_rect.width - mark_width - logical_px(2), + con->deco_rect.y + text_offset_y, mark_width); + + I3STRING_FREE(mark); + } + FREE(formatted_mark); - mark_width = predict_text_width(mark); - - draw_text(mark, parent->pixmap, parent->pm_gc, NULL, - con->deco_rect.x + con->deco_rect.width - mark_width - logical_px(2), - con->deco_rect.y + text_offset_y, mark_width); - - I3STRING_FREE(mark); } i3String *title = win->title_format == NULL ? win->name : window_parse_title_format(win); diff --git a/testcases/t/165-for_window.t b/testcases/t/165-for_window.t index 985a7bfd..476bcc9f 100644 --- a/testcases/t/165-for_window.t +++ b/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); diff --git a/testcases/t/210-mark-unmark.t b/testcases/t/210-mark-unmark.t index 93b26d94..39fc0904 100644 --- a/testcases/t/210-mark-unmark.t +++ b/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 -# 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, +# 7: 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 ############################################################## diff --git a/testcases/t/235-wm-class-change-handler.t b/testcases/t/235-wm-class-change-handler.t index 3685b30c..ce237b57 100644 --- a/testcases/t/235-wm-class-change-handler.t +++ b/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); diff --git a/testcases/t/255-multiple-marks.t b/testcases/t/255-multiple-marks.t new file mode 100644 index 00000000..d6d86e23 --- /dev/null +++ b/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;