diff --git a/include/workspace.h b/include/workspace.h index db544ce8..69974a2e 100644 --- a/include/workspace.h +++ b/include/workspace.h @@ -218,7 +218,6 @@ 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, Output *output); +void workspace_move_to_output(Con *ws, Output *output); diff --git a/src/commands.c b/src/commands.c index 778b4a1a..3cf5a57c 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1114,22 +1114,13 @@ void cmd_move_workspace_to_output(I3_CMD, const char *name) { } Output *current_output = get_output_for_con(ws); - if (current_output == NULL) { - yerror("Cannot get current output. This is a bug in i3."); - return; - } - Output *target_output = get_output_from_string(current_output, name); if (!target_output) { yerror("Could not get output from string \"%s\"", name); return; } - bool success = workspace_move_to_output(ws, target_output); - if (!success) { - yerror("Failed to move workspace to output."); - return; - } + workspace_move_to_output(ws, target_output); } cmd_output->needs_tree_render = true; @@ -1635,24 +1626,18 @@ void cmd_open(I3_CMD) { * */ void cmd_focus_output(I3_CMD, const char *name) { - owindow *current; - - DLOG("name = %s\n", name); - HANDLE_EMPTY_MATCH; - /* get the output */ - Output *current_output = NULL; - Output *output; + if (TAILQ_EMPTY(&owindows)) { + ysuccess(true); + return; + } - TAILQ_FOREACH(current, &owindows, owindows) - current_output = get_output_for_con(current->con); - assert(current_output != NULL); - - output = get_output_from_string(current_output, name); + Output *current_output = get_output_for_con(TAILQ_FIRST(&owindows)->con); + Output *output = get_output_from_string(current_output, name); if (!output) { - yerror("No such output found."); + yerror("Output %s not found.", name); return; } @@ -1667,7 +1652,6 @@ void cmd_focus_output(I3_CMD, const char *name) { workspace_show(ws); cmd_output->needs_tree_render = true; - // XXX: default reply for now, make this a better reply ysuccess(true); } diff --git a/src/con.c b/src/con.c index f904bfe8..a88909e1 100644 --- a/src/con.c +++ b/src/con.c @@ -1400,8 +1400,6 @@ void con_move_to_output(Con *con, Output *output, bool fix_coordinates) { */ bool con_move_to_output_name(Con *con, const char *name, bool fix_coordinates) { Output *current_output = get_output_for_con(con); - assert(current_output != NULL); - Output *output = get_output_from_string(current_output, name); if (output == NULL) { ELOG("Could not find output \"%s\"\n", name); diff --git a/src/output.c b/src/output.c index ebba5c77..3d931b1b 100644 --- a/src/output.c +++ b/src/output.c @@ -54,16 +54,8 @@ char *output_primary_name(Output *output) { Output *get_output_for_con(Con *con) { Con *output_con = con_get_output(con); - if (output_con == NULL) { - ELOG("Could not get the output container for con = %p.\n", con); - return NULL; - } - Output *output = get_output_by_name(output_con->name, true); - if (output == NULL) { - ELOG("Could not get output from name \"%s\".\n", output_con->name); - return NULL; - } + assert(output != NULL); return output; } diff --git a/src/workspace.c b/src/workspace.c index 5c2c48fa..3cf74754 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -961,19 +961,18 @@ 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, Output *output) { - LOG("Trying to move workspace %p / %s to output %p / \"%s\".\n", ws, ws->name, output, output_primary_name(output)); +void workspace_move_to_output(Con *ws, Output *output) { + DLOG("Moving workspace %p / %s to output %p / \"%s\".\n", ws, ws->name, output, output_primary_name(output)); Output *current_output = get_output_for_con(ws); - if (current_output == NULL) { - ELOG("Cannot get current output. This is a bug in i3.\n"); - return false; - } - Con *content = output_get_content(output->con); - LOG("got output %p with content %p\n", output, content); + DLOG("got output %p with content %p\n", output, content); + + if (ws->parent == content) { + DLOG("Nothing to do, workspace already there\n"); + return; + } Con *previously_visible_ws = TAILQ_FIRST(&(content->focus_head)); if (previously_visible_ws) { @@ -984,7 +983,7 @@ bool workspace_move_to_output(Con *ws, Output *output) { 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); + DLOG("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; @@ -999,7 +998,7 @@ bool workspace_move_to_output(Con *ws, Output *output) { } /* so create the workspace referenced to by this assignment */ - LOG("Creating workspace from assignment %s.\n", assignment->name); + DLOG("Creating workspace from assignment %s.\n", assignment->name); workspace_get(assignment->name, NULL); used_assignment = true; break; @@ -1008,8 +1007,9 @@ bool workspace_move_to_output(Con *ws, Output *output) { /* if we couldn't create the workspace using an assignment, create it on * the output. Workspace init IPC events are sent either by * workspace_get or create_workspace_on_output. */ - if (!used_assignment) + if (!used_assignment) { create_workspace_on_output(current_output, ws->parent); + } } DLOG("Detaching\n"); @@ -1017,18 +1017,19 @@ bool workspace_move_to_output(Con *ws, Output *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. */ + /* 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); + DLOG("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)); + 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) { @@ -1037,24 +1038,23 @@ bool workspace_move_to_output(Con *ws, Output *output) { } if (!previously_visible_ws) { - return true; + return; } - /* 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. */ + /* 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) + 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. */ + * 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; } diff --git a/testcases/t/502-focus-output.t b/testcases/t/502-focus-output.t index 01d8db33..118aba16 100644 --- a/testcases/t/502-focus-output.t +++ b/testcases/t/502-focus-output.t @@ -86,4 +86,33 @@ is(focused_output, 'fake-1', 'focus on second output'); cmd 'focus output fake-0'; is(focused_output, 'fake-0', 'focus on first output'); +################################################################################ +# use 'focus output' with command criteria and verify that i3 does not crash +# when they don't match any window +################################################################################ + +is(focused_output, 'fake-0', 'focus on first output'); + +cmd '[con_mark=doesnotexist] focus output right'; +does_i3_live; +is(focused_output, 'fake-0', 'focus remained on first output'); + +################################################################################ +# use 'focus output' with command criteria and verify that focus gets changed +# appropriately +################################################################################ + +is(focused_output, 'fake-0', 'focus on first output'); + +my $window = open_window; + +cmd 'focus output right'; +is(focused_output, 'fake-1', 'focus on second output'); + +cmd '[id= . ' . $window->id . '] focus output right'; +is(focused_output, 'fake-1', 'focus on second output after command with criteria'); + +cmd 'focus output right'; +is(focused_output, 'fake-0', 'focus on first output after command without criteria'); + done_testing;