From bcc1314a2d43e0889a5f5bad5b02119cfb9f44a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Wed, 4 Mar 2015 09:06:44 +0100 Subject: [PATCH 1/3] When renaming a workspace, look for assignments and move the renamed workspace to the appropriate output. --- src/commands.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/commands.c b/src/commands.c index 498c25c8..7d3c9e6e 100644 --- a/src/commands.c +++ b/src/commands.c @@ -2034,6 +2034,24 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) { Con *parent = workspace->parent; con_detach(workspace); con_attach(workspace, parent, false); + + /* Move the workspace to the correct output if it has an assignment */ + struct Workspace_Assignment *assignment = NULL; + TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { + if (assignment->output == NULL) + continue; + if (strcmp(assignment->name, workspace->name) != 0 + && (!name_is_digits(assignment->name) || ws_name_to_number(assignment->name) != workspace->num)) { + + continue; + } + + /* Focus this workspace for now, we will restore the previous focus anyway. */ + con_focus(workspace); + cmd_move_workspace_to_output(current_match, cmd_output, assignment->output); + break; + } + /* Restore the previous focus since con_attach messes with the focus. */ con_focus(previously_focused); From eb73059c610752fc18e0fcc5c5402bb9b7770574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Wed, 4 Mar 2015 09:22:25 +0100 Subject: [PATCH 2/3] Refactor functions for easy reuse --- include/output.h | 7 +++ include/workspace.h | 7 +++ src/commands.c | 125 +++----------------------------------------- src/output.c | 22 ++++++++ src/workspace.c | 108 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 150 insertions(+), 119 deletions(-) diff --git a/include/output.h b/include/output.h index 10ee7d17..6514c477 100644 --- a/include/output.h +++ b/include/output.h @@ -14,3 +14,10 @@ * */ Con *output_get_content(Con *output); + +/** + * Returns an 'output' corresponding to one of left/right/down/up or a specific + * output name. + * + */ +Output *get_output_from_string(Output *current_output, const char *output_str); diff --git a/include/workspace.h b/include/workspace.h index 9ee6f156..d0f801e0 100644 --- a/include/workspace.h +++ b/include/workspace.h @@ -179,3 +179,10 @@ Con *workspace_attach_to(Con *ws); * The container inherits the layout from the workspace. */ Con *workspace_encapsulate(Con *ws); + +/** + * Move the given workspace to the specified output. + * This returns true if and only if moving the workspace was successful. + * + */ +bool workspace_move_to_output(Con *ws, char *output); diff --git a/src/commands.c b/src/commands.c index 7d3c9e6e..49005ba7 100644 --- a/src/commands.c +++ b/src/commands.c @@ -62,28 +62,6 @@ static bool definitelyGreaterThan(float a, float b, float epsilon) { return (a - b) > ((fabs(a) < fabs(b) ? fabs(b) : fabs(a)) * epsilon); } -/* - * Returns an 'output' corresponding to one of left/right/down/up or a specific - * output name. - * - */ -static Output *get_output_from_string(Output *current_output, const char *output_str) { - Output *output; - - if (strcasecmp(output_str, "left") == 0) - output = get_output_next_wrap(D_LEFT, current_output); - else if (strcasecmp(output_str, "right") == 0) - output = get_output_next_wrap(D_RIGHT, current_output); - else if (strcasecmp(output_str, "up") == 0) - output = get_output_next_wrap(D_UP, current_output); - else if (strcasecmp(output_str, "down") == 0) - output = get_output_next_wrap(D_DOWN, current_output); - else - output = get_output_by_name(output_str); - - return output; -} - /* * Returns the output containing the given container. */ @@ -1259,101 +1237,12 @@ void cmd_move_workspace_to_output(I3_CMD, char *name) { owindow *current; TAILQ_FOREACH(current, &owindows, owindows) { - Output *current_output = get_output_of_con(current->con); - if (!current_output) { - ELOG("Cannot get current output. This is a bug in i3.\n"); - ysuccess(false); - return; - } - Output *output = get_output_from_string(current_output, name); - if (!output) { - ELOG("Could not get output from string \"%s\"\n", name); - ysuccess(false); - return; - } - - Con *content = output_get_content(output->con); - LOG("got output %p with content %p\n", output, content); - - Con *previously_visible_ws = TAILQ_FIRST(&(content->nodes_head)); - LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name); - Con *ws = con_get_workspace(current->con); - LOG("should move workspace %p / %s\n", ws, ws->name); - bool workspace_was_visible = workspace_is_visible(ws); - - if (con_num_children(ws->parent) == 1) { - LOG("Creating a new workspace to replace \"%s\" (last on its output).\n", ws->name); - - /* check if we can find a workspace assigned to this output */ - bool used_assignment = false; - struct Workspace_Assignment *assignment; - TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (strcmp(assignment->output, current_output->name) != 0) - continue; - - /* check if this workspace is already attached to the tree */ - Con *workspace = NULL, *out; - TAILQ_FOREACH(out, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(out), - !strcasecmp(child->name, assignment->name)); - if (workspace != NULL) - continue; - - /* so create the workspace referenced to by this assignment */ - LOG("Creating workspace from assignment %s.\n", assignment->name); - workspace_get(assignment->name, NULL); - used_assignment = true; - break; - } - - /* if we couldn't create the workspace using an assignment, create - * it on the output */ - if (!used_assignment) - create_workspace_on_output(current_output, ws->parent); - - /* notify the IPC listeners */ - ipc_send_workspace_event("init", ws, NULL); - } - DLOG("Detaching\n"); - - /* detach from the old output and attach to the new output */ - Con *old_content = ws->parent; - con_detach(ws); - if (workspace_was_visible) { - /* The workspace which we just detached was visible, so focus - * the next one in the focus-stack. */ - Con *focus_ws = TAILQ_FIRST(&(old_content->focus_head)); - LOG("workspace was visible, focusing %p / %s now\n", focus_ws, focus_ws->name); - workspace_show(focus_ws); - } - con_attach(ws, content, false); - - /* fix the coordinates of the floating containers */ - Con *floating_con; - TAILQ_FOREACH(floating_con, &(ws->floating_head), floating_windows) - floating_fix_coordinates(floating_con, &(old_content->rect), &(content->rect)); - - ipc_send_workspace_event("move", ws, NULL); - if (workspace_was_visible) { - /* Focus the moved workspace on the destination output. */ - workspace_show(ws); - } - - /* NB: We cannot simply work with previously_visible_ws since it might - * have been cleaned up by workspace_show() already, depending on the - * focus order/number of other workspaces on the output. - * Instead, we loop through the available workspaces and only work with - * previously_visible_ws if we still find it. */ - TAILQ_FOREACH(ws, &(content->nodes_head), nodes) { - if (ws != previously_visible_ws) - continue; - - /* Call the on_remove_child callback of the workspace which previously - * was visible on the destination output. Since it is no longer - * visible, it might need to get cleaned up. */ - CALL(previously_visible_ws, on_remove_child); - break; + bool success = workspace_move_to_output(ws, name); + if (!success) { + ELOG("Failed to move workspace to output.\n"); + ysuccess(false); + return; } } @@ -2046,9 +1935,7 @@ void cmd_rename_workspace(I3_CMD, char *old_name, char *new_name) { continue; } - /* Focus this workspace for now, we will restore the previous focus anyway. */ - con_focus(workspace); - cmd_move_workspace_to_output(current_match, cmd_output, assignment->output); + workspace_move_to_output(workspace, assignment->output); break; } diff --git a/src/output.c b/src/output.c index 6499c65d..822a0f88 100644 --- a/src/output.c +++ b/src/output.c @@ -24,3 +24,25 @@ Con *output_get_content(Con *output) { return NULL; } + +/* + * Returns an 'output' corresponding to one of left/right/down/up or a specific + * output name. + * + */ +Output *get_output_from_string(Output *current_output, const char *output_str) { + Output *output; + + if (strcasecmp(output_str, "left") == 0) + output = get_output_next_wrap(D_LEFT, current_output); + else if (strcasecmp(output_str, "right") == 0) + output = get_output_next_wrap(D_RIGHT, current_output); + else if (strcasecmp(output_str, "up") == 0) + output = get_output_next_wrap(D_UP, current_output); + else if (strcasecmp(output_str, "down") == 0) + output = get_output_next_wrap(D_DOWN, current_output); + else + output = get_output_by_name(output_str); + + return output; +} diff --git a/src/workspace.c b/src/workspace.c index a3056633..f55c920e 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -879,3 +879,111 @@ Con *workspace_encapsulate(Con *ws) { return new; } + +/** + * Move the given workspace to the specified output. + * This returns true if and only if moving the workspace was successful. + */ +bool workspace_move_to_output(Con *ws, char *name) { + LOG("Trying to move workspace %p / %s to output \"%s\".\n", ws, ws->name, name); + + Con *current_output_con = con_get_output(ws); + if (!current_output_con) { + ELOG("Could not get the output container for workspace %p / %s.\n", ws, ws->name); + return false; + } + + Output *current_output = get_output_by_name(current_output_con->name); + if (!current_output) { + ELOG("Cannot get current output. This is a bug in i3.\n"); + return false; + } + Output *output = get_output_from_string(current_output, name); + if (!output) { + ELOG("Could not get output from string \"%s\"\n", name); + return false; + } + + Con *content = output_get_content(output->con); + LOG("got output %p with content %p\n", output, content); + + Con *previously_visible_ws = TAILQ_FIRST(&(content->nodes_head)); + LOG("Previously visible workspace = %p / %s\n", previously_visible_ws, previously_visible_ws->name); + + bool workspace_was_visible = workspace_is_visible(ws); + if (con_num_children(ws->parent) == 1) { + LOG("Creating a new workspace to replace \"%s\" (last on its output).\n", ws->name); + + /* check if we can find a workspace assigned to this output */ + bool used_assignment = false; + struct Workspace_Assignment *assignment; + TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { + if (assignment->output == NULL || strcmp(assignment->output, current_output->name) != 0) + continue; + + /* check if this workspace is already attached to the tree */ + Con *workspace = NULL, *out; + TAILQ_FOREACH(out, &(croot->nodes_head), nodes) + GREP_FIRST(workspace, output_get_content(out), + !strcasecmp(child->name, assignment->name)); + if (workspace != NULL) + continue; + + /* so create the workspace referenced to by this assignment */ + LOG("Creating workspace from assignment %s.\n", assignment->name); + workspace_get(assignment->name, NULL); + used_assignment = true; + break; + } + + /* if we couldn't create the workspace using an assignment, create + * it on the output */ + if (!used_assignment) + create_workspace_on_output(current_output, ws->parent); + + /* notify the IPC listeners */ + ipc_send_workspace_event("init", ws, NULL); + } + DLOG("Detaching\n"); + + /* detach from the old output and attach to the new output */ + Con *old_content = ws->parent; + con_detach(ws); + if (workspace_was_visible) { + /* The workspace which we just detached was visible, so focus + * the next one in the focus-stack. */ + Con *focus_ws = TAILQ_FIRST(&(old_content->focus_head)); + LOG("workspace was visible, focusing %p / %s now\n", focus_ws, focus_ws->name); + workspace_show(focus_ws); + } + con_attach(ws, content, false); + + /* fix the coordinates of the floating containers */ + Con *floating_con; + TAILQ_FOREACH(floating_con, &(ws->floating_head), floating_windows) + floating_fix_coordinates(floating_con, &(old_content->rect), &(content->rect)); + + ipc_send_workspace_event("move", ws, NULL); + if (workspace_was_visible) { + /* Focus the moved workspace on the destination output. */ + workspace_show(ws); + } + + /* NB: We cannot simply work with previously_visible_ws since it might + * have been cleaned up by workspace_show() already, depending on the + * focus order/number of other workspaces on the output. + * Instead, we loop through the available workspaces and only work with + * previously_visible_ws if we still find it. */ + TAILQ_FOREACH(ws, &(content->nodes_head), nodes) { + if (ws != previously_visible_ws) + continue; + + /* Call the on_remove_child callback of the workspace which previously + * was visible on the destination output. Since it is no longer + * visible, it might need to get cleaned up. */ + CALL(previously_visible_ws, on_remove_child); + break; + } + + return true; +} From 25ec389ecba8aef71bb6e729c438e1b0f3192258 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ingo=20B=C3=BCrk?= Date: Wed, 4 Mar 2015 09:45:47 +0100 Subject: [PATCH 3/3] Added tests for #1473 --- testcases/t/522-rename-assigned-workspace.t | 90 +++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 testcases/t/522-rename-assigned-workspace.t diff --git a/testcases/t/522-rename-assigned-workspace.t b/testcases/t/522-rename-assigned-workspace.t new file mode 100644 index 00000000..aa69d66d --- /dev/null +++ b/testcases/t/522-rename-assigned-workspace.t @@ -0,0 +1,90 @@ +#!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 that workspaces are moved to the assigned output if they +# are renamed to an assigned name. +# Ticket: #1473 + +use i3test i3_autostart => 0; + +my $config = <connect->recv; + +# Returns the name of the output on which this workspace resides +sub get_output_for_workspace { + my $ws_name = shift @_; + + foreach (grep { not $_->{name} =~ /^__/ } @{$i3->get_tree->recv->{nodes}}) { + my $output = $_->{name}; + foreach (grep { $_->{name} =~ "content" } @{$_->{nodes}}) { + return $output if $_->{nodes}[0]->{name} =~ $ws_name; + } + } +} + +########################################################################## +# Renaming the workspace to an unassigned name does not move the workspace +# (regression test) +########################################################################## + +cmd 'focus output fake-0'; +cmd 'rename workspace to unassigned'; +is(get_output_for_workspace('unassigned'), 'fake-0', + 'Unassigned workspace should stay on its output when being renamed'); + +########################################################################## +# Renaming a workspace by number only triggers the assignment +########################################################################## + +cmd 'focus output fake-0'; +cmd 'rename workspace to 2'; +is(get_output_for_workspace('2'), 'fake-1', + 'Renaming the workspace to a number should move it to the assigned output'); + +########################################################################## +# Renaming a workspace by number and name triggers the assignment +########################################################################## + +cmd 'focus output fake-0'; +cmd 'rename workspace to "2:foo"'; +is(get_output_for_workspace('2:foo'), 'fake-1', + 'Renaming the workspace to a number and name should move it to the assigned output'); + +########################################################################## +# Renaming a workspace by name only triggers the assignment +########################################################################## + +cmd 'focus output fake-0'; +cmd 'rename workspace to baz'; +is(get_output_for_workspace('baz'), 'fake-1', + 'Renaming the workspace to a number and name should move it to the assigned output'); + + +exit_gracefully($pid); +done_testing;