From bc439de755d23b7f0f7cad76aed23f975aa0c722 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Wed, 28 Mar 2018 03:35:40 +0300 Subject: [PATCH 1/4] Introduce get_assigned_output This also replaces code in create_workspace_on_output() that is theoretically more efficient but: 1. It isn't a huge difference since it depends on the number of outputs, that shouldn't be high. 2. get_assigned_output will be modified and used for #555, then its logic should be followed in create_workspace_on_output() too. Another note for create_workspace_on_output: if assigned is not NULL the condition (assigned != output->con) should never be false, ie if there is an assigned output to this name, it isn't the current one. This happens because the current callers check for assignments before calling create_workspace_on_output(). --- src/workspace.c | 59 ++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/src/workspace.c b/src/workspace.c index a16479a5..9441042e 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -67,6 +67,33 @@ static void _workspace_apply_default_orientation(Con *ws) { } } +/* + * Returns the first output that is assigned to a workspace specified by the + * given name or number or NULL if no such output exists. If there is a + * workspace with a matching name and another workspace with a matching number, + * the output assigned to the first one is returned. + * If 'name' is NULL it will be ignored. + * If 'parsed_num' is -1 it will be ignored. + * + */ +static Con *get_assigned_output(const char *name, long parsed_num) { + Con *output = NULL; + + struct Workspace_Assignment *assignment; + TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { + if (name && strcmp(assignment->name, name) == 0) { + DLOG("Found workspace name assignment to output \"%s\"\n", assignment->output); + GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); + break; + } else if (parsed_num != -1 && name_is_digits(assignment->name) && ws_name_to_number(assignment->name) == parsed_num) { + DLOG("Found workspace number assignment to output \"%s\"\n", assignment->output); + GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); + } + } + + return output; +} + /* * Returns a pointer to the workspace with the given number (starting at 0), * creating the workspace if necessary (by allocating the necessary amount of @@ -78,25 +105,16 @@ Con *workspace_get(const char *num, bool *created) { if (workspace == NULL) { LOG("Creating new workspace \"%s\"\n", num); - /* unless an assignment is found, we will create this workspace on the current output */ - Con *output = con_get_output(focused); - /* look for assignments */ - struct Workspace_Assignment *assignment; /* We set workspace->num to the number if this workspace’s name begins * with a positive number. Otherwise it’s a named ws and num will be * -1. */ long parsed_num = ws_name_to_number(num); - TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (strcmp(assignment->name, num) == 0) { - DLOG("Found workspace name assignment to output \"%s\"\n", assignment->output); - GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); - break; - } else if (parsed_num != -1 && name_is_digits(assignment->name) && ws_name_to_number(assignment->name) == parsed_num) { - DLOG("Found workspace number assignment to output \"%s\"\n", assignment->output); - GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); - } + Con *output = get_assigned_output(num, parsed_num); + /* if an assignment is not found, we create this workspace on the current output */ + if (!output) { + output = con_get_output(focused); } Con *content = output_get_content(output); @@ -208,19 +226,10 @@ Con *create_workspace_on_output(Output *output, Con *content) { /* Ensure that this workspace is not assigned to a different output — * otherwise we would create it, then move it over to its output, then * find a new workspace, etc… */ - bool assigned = false; - struct Workspace_Assignment *assignment; - TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (strcmp(assignment->name, target_name) != 0 || - strcmp(assignment->output, output_primary_name(output)) == 0) - continue; - - assigned = true; - break; - } - - if (assigned) + Con *assigned = get_assigned_output(target_name, -1); + if (assigned && assigned != output->con) { continue; + } exists = (get_existing_workspace_by_name(target_name) != NULL); if (!exists) { From 1d5b43c18f5de8e6282c4e78bb9db7988dab169d Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Wed, 28 Mar 2018 01:57:54 +0300 Subject: [PATCH 2/4] Move get_output_for_workspace() to i3test --- testcases/lib/i3test.pm.in | 24 +++++++++++++++++++ testcases/t/518-interpret-workspace-numbers.t | 15 ------------ testcases/t/522-rename-assigned-workspace.t | 15 ------------ 3 files changed, 24 insertions(+), 30 deletions(-) diff --git a/testcases/lib/i3test.pm.in b/testcases/lib/i3test.pm.in index 68ac1ee5..5734eca7 100644 --- a/testcases/lib/i3test.pm.in +++ b/testcases/lib/i3test.pm.in @@ -26,6 +26,7 @@ use Data::Dumper (); use Exporter (); our @EXPORT = qw( get_workspace_names + get_output_for_workspace get_unused_workspace fresh_workspace get_ws_content @@ -402,6 +403,29 @@ sub get_workspace_names { [ map { $_->{name} } @cons ] } +=head2 get_output_for_workspace() + +Returns the name of the output on which this workspace resides + + cmd 'focus output fake-1'; + cmd 'workspace 1'; + is(get_output_for_workspace('1', 'fake-0', 'Workspace 1 in output fake-0'); + +=cut +sub get_output_for_workspace { + my $ws_name = shift @_; + my $i3 = i3(get_socket_path()); + my $tree = $i3->get_tree->recv; + my @outputs = @{$tree->{nodes}}; + + foreach (grep { not $_->{name} =~ /^__/ } @outputs) { + my $output = $_->{name}; + foreach (grep { $_->{name} =~ "content" } @{$_->{nodes}}) { + return $output if $_->{nodes}[0]->{name} =~ $ws_name; + } + } +} + =head2 get_unused_workspace Returns a workspace name which has not yet been used. See also diff --git a/testcases/t/518-interpret-workspace-numbers.t b/testcases/t/518-interpret-workspace-numbers.t index e9ea76c4..7fd86d72 100644 --- a/testcases/t/518-interpret-workspace-numbers.t +++ b/testcases/t/518-interpret-workspace-numbers.t @@ -30,21 +30,6 @@ workspace 2:override output fake-1 fake-outputs 1024x768+0+0,1024x768+1024+0 EOT -my $i3 = i3(get_socket_path()); -$i3->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; - } - } -} - ################################################################################ # Workspace assignments with bare numbers should be interpreted as `workspace # number` config directives. Any workspace beginning with that number should be diff --git a/testcases/t/522-rename-assigned-workspace.t b/testcases/t/522-rename-assigned-workspace.t index 5c9f2ff3..f6319729 100644 --- a/testcases/t/522-rename-assigned-workspace.t +++ b/testcases/t/522-rename-assigned-workspace.t @@ -31,21 +31,6 @@ workspace baz output fake-1 workspace 5 output left EOT -my $i3 = i3(get_socket_path()); -$i3->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) From d525eb80aefcd596c5c40750eb9fa3570e5c55ed Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Wed, 28 Mar 2018 04:05:48 +0300 Subject: [PATCH 3/4] Use get_assigned_output for numbers This prohibits the usage of workspaces assigned to other outputs in create_workspace_on_output. Eg, with config: workspace 1 output fake-0 workspace 2 output fake-0 and 2 screens workspace 2 would be used for the second screen even though it is assigned to the first one. Also introduces a test for workspace assignments that includes the case described above and some tests that don't fail in the next branch. --- src/workspace.c | 4 +- testcases/t/297-assign-workspace-to-output.t | 60 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 testcases/t/297-assign-workspace-to-output.t diff --git a/src/workspace.c b/src/workspace.c index 9441042e..a27c6f4b 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -248,7 +248,9 @@ Con *create_workspace_on_output(Output *output, Con *content) { DLOG("Getting next unused workspace by number\n"); int c = 0; while (exists) { - exists = (get_existing_workspace_by_num(++c) != NULL); + c++; + Con *assigned = get_assigned_output(NULL, c); + exists = (get_existing_workspace_by_num(c) || (assigned && assigned != output->con)); DLOG("result for ws %d: exists = %d\n", c, exists); } ws->num = c; diff --git a/testcases/t/297-assign-workspace-to-output.t b/testcases/t/297-assign-workspace-to-output.t new file mode 100644 index 00000000..650efa12 --- /dev/null +++ b/testcases/t/297-assign-workspace-to-output.t @@ -0,0 +1,60 @@ +#!perl +# vim:ts=4:sw=4:expandtab +# +# Please read the following documents before working on tests: +# • https://build.i3wm.org/docs/testsuite.html +# (or docs/testsuite) +# +# • https://build.i3wm.org/docs/lib-i3test.html +# (alternatively: perldoc ./testcases/lib/i3test.pm) +# +# • https://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) +# +# Test assignments of workspaces to outputs. +use i3test i3_autostart => 0; + +################################################################################ +# Test initial workspaces. +################################################################################ + +my $config = < Date: Wed, 28 Mar 2018 00:55:20 +0300 Subject: [PATCH 4/4] Allow multiple assignments of workspaces to outputs Also makes get_assigned_output work with the primary output: workspace X output primary will now work. Fixes #555. --- docs/userguide | 7 +++- include/workspace.h | 7 ++++ parser-specs/config.spec | 2 +- src/commands.c | 3 ++ src/config_directives.c | 18 +++++--- src/randr.c | 7 ++-- src/workspace.c | 32 +++++++++++--- testcases/t/297-assign-workspace-to-output.t | 42 +++++++++++++++++++ testcases/t/518-interpret-workspace-numbers.t | 8 ++++ testcases/t/522-rename-assigned-workspace.t | 22 ++++++++++ 10 files changed, 130 insertions(+), 18 deletions(-) diff --git a/docs/userguide b/docs/userguide index d944bb39..2746f24e 100644 --- a/docs/userguide +++ b/docs/userguide @@ -888,7 +888,7 @@ the second screen and so on). *Syntax*: ------------------------------------- -workspace output +workspace output [output2]… ------------------------------------- The 'output' is the name of the RandR output you attach your screen to. On a @@ -907,12 +907,15 @@ monitor name is “Dell UP2414Q”. entire monitor, i3 will still use the entire area of the containing monitor rather than that of just the output's.) +You can specify multiple outputs. The first available will be used. + If you use named workspaces, they must be quoted: *Examples*: --------------------------- workspace 1 output LVDS1 -workspace 5 output VGA1 +workspace 2 output primary +workspace 5 output VGA1 LVDS1 workspace "2: vim" output VGA1 --------------------------- diff --git a/include/workspace.h b/include/workspace.h index ae287422..28d9eb66 100644 --- a/include/workspace.h +++ b/include/workspace.h @@ -38,6 +38,13 @@ Con *get_existing_workspace_by_name(const char *name); */ Con *get_existing_workspace_by_num(int num); +/** + * Returns true if the first output assigned to a workspace with the given + * workspace assignment is the same as the given output. + * + */ +bool output_triggers_assignment(Output *output, struct Workspace_Assignment *assignment); + /** * Returns a pointer to the workspace with the given number (starting at 0), * creating the workspace if necessary (by allocating the necessary amount of diff --git a/parser-specs/config.spec b/parser-specs/config.spec index c5c4651c..b15c9a80 100644 --- a/parser-specs/config.spec +++ b/parser-specs/config.spec @@ -273,7 +273,7 @@ state WORKSPACE_OUTPUT: -> WORKSPACE_OUTPUT_STR state WORKSPACE_OUTPUT_STR: - output = word + output = string -> call cfg_workspace($workspace, $output) # ipc-socket diff --git a/src/commands.c b/src/commands.c index c7b57ab3..a133daa6 100644 --- a/src/commands.c +++ b/src/commands.c @@ -2019,6 +2019,9 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { LOG("Could not get output named \"%s\"\n", assignment->output); continue; } + if (!output_triggers_assignment(target_output, assignment)) { + continue; + } workspace_move_to_output(workspace, target_output); bool can_restore_focus = previously_focused != NULL; diff --git a/src/config_directives.c b/src/config_directives.c index 4a31f79e..17e25cde 100644 --- a/src/config_directives.c +++ b/src/config_directives.c @@ -322,8 +322,8 @@ CFGFUN(show_marks, const char *value) { config.show_marks = eval_boolstr(value); } -CFGFUN(workspace, const char *workspace, const char *output) { - DLOG("Assigning workspace \"%s\" to output \"%s\"\n", workspace, output); +CFGFUN(workspace, const char *workspace, const char *outputs) { + DLOG("Assigning workspace \"%s\" to outputs \"%s\"\n", workspace, outputs); /* Check for earlier assignments of the same workspace so that we * don’t have assignments of a single workspace to different * outputs */ @@ -336,10 +336,16 @@ CFGFUN(workspace, const char *workspace, const char *output) { } } - assignment = scalloc(1, sizeof(struct Workspace_Assignment)); - assignment->name = sstrdup(workspace); - assignment->output = sstrdup(output); - TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments); + char *buf = sstrdup(outputs); + char *output = strtok(buf, " "); + while (output != NULL) { + assignment = scalloc(1, sizeof(struct Workspace_Assignment)); + assignment->name = sstrdup(workspace); + assignment->output = sstrdup(output); + TAILQ_INSERT_TAIL(&ws_assignments, assignment, ws_assignments); + output = strtok(NULL, " "); + } + free(buf); } CFGFUN(ipc_socket, const char *path) { diff --git a/src/randr.c b/src/randr.c index d4d7402a..38f1ee97 100644 --- a/src/randr.c +++ b/src/randr.c @@ -424,9 +424,9 @@ void init_ws_for_output(Output *output, Con *content) { /* go through all assignments and move the existing workspaces to this output */ struct Workspace_Assignment *assignment; TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (strcmp(assignment->output, output_primary_name(output)) != 0) + if (!output_triggers_assignment(output, assignment)) { continue; - + } Con *workspace = get_existing_workspace_by_name(assignment->name); if (workspace == NULL) continue; @@ -501,8 +501,9 @@ void init_ws_for_output(Output *output, Con *content) { /* otherwise, we create the first assigned ws for this output */ TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (strcmp(assignment->output, output_primary_name(output)) != 0) + if (!output_triggers_assignment(output, assignment)) { continue; + } LOG("Initializing first assigned workspace \"%s\" for output \"%s\"\n", assignment->name, assignment->output); diff --git a/src/workspace.c b/src/workspace.c index a27c6f4b..e92aaa5f 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -72,28 +72,47 @@ static void _workspace_apply_default_orientation(Con *ws) { * given name or number or NULL if no such output exists. If there is a * workspace with a matching name and another workspace with a matching number, * the output assigned to the first one is returned. + * The order of the 'ws_assignments' queue is respected: if multiple assignments + * match the specified workspace, the first one is returned. * If 'name' is NULL it will be ignored. * If 'parsed_num' is -1 it will be ignored. * */ static Con *get_assigned_output(const char *name, long parsed_num) { Con *output = NULL; - struct Workspace_Assignment *assignment; TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { if (name && strcmp(assignment->name, name) == 0) { DLOG("Found workspace name assignment to output \"%s\"\n", assignment->output); - GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); - break; - } else if (parsed_num != -1 && name_is_digits(assignment->name) && ws_name_to_number(assignment->name) == parsed_num) { + Output *assigned_by_name = get_output_by_name(assignment->output, true); + if (assigned_by_name) { + /* When the name matches exactly, skip numbered assignments. */ + return assigned_by_name->con; + } + } else if (!output && /* Only keep the first numbered assignment. */ + parsed_num != -1 && + name_is_digits(assignment->name) && + ws_name_to_number(assignment->name) == parsed_num) { DLOG("Found workspace number assignment to output \"%s\"\n", assignment->output); - GREP_FIRST(output, croot, !strcmp(child->name, assignment->output)); + Output *assigned_by_num = get_output_by_name(assignment->output, true); + if (assigned_by_num) { + output = assigned_by_num->con; + } } } return output; } +/* + * Returns true if the first output assigned to a workspace with the given + * workspace assignment is the same as the given output. + */ +bool output_triggers_assignment(Output *output, struct Workspace_Assignment *assignment) { + Con *assigned = get_assigned_output(assignment->name, -1); + return assigned && assigned == output->con; +} + /* * Returns a pointer to the workspace with the given number (starting at 0), * creating the workspace if necessary (by allocating the necessary amount of @@ -957,8 +976,9 @@ bool workspace_move_to_output(Con *ws, Output *output) { bool used_assignment = false; struct Workspace_Assignment *assignment; TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { - if (assignment->output == NULL || strcmp(assignment->output, output_primary_name(current_output)) != 0) + if (!output_triggers_assignment(current_output, assignment)) { continue; + } /* check if this workspace is already attached to the tree */ if (get_existing_workspace_by_name(assignment->name) != NULL) { continue; diff --git a/testcases/t/297-assign-workspace-to-output.t b/testcases/t/297-assign-workspace-to-output.t index 650efa12..a7b75be9 100644 --- a/testcases/t/297-assign-workspace-to-output.t +++ b/testcases/t/297-assign-workspace-to-output.t @@ -54,7 +54,49 @@ check_output('4', 'fake-3', 'First available number that is not assigned to exis check_output('dontusethisname', '', 'Named workspace that is assigned to output that does not exist is not used'); check_output('donotoverride', '', 'Named workspace assigned to already occupied output is not used'); +exit_gracefully($pid); +################################################################################ +# Test multiple assignments +################################################################################ + +sub do_test { + my ($workspace, $output, $msg) = @_; + cmd 'focus output ' . ($output eq 'fake-3' ? 'fake-0' : 'fake-3'); + + cmd "workspace $workspace"; + check_output($workspace, $output, $msg) +} + +$config = <