From 6a2728ba796aa88f7cae034b759169f032408d56 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 27 Mar 2018 22:18:17 +0300 Subject: [PATCH 1/2] Introduce get_existing_workspace_by_name --- include/workspace.h | 7 +++++++ src/commands.c | 11 +++-------- src/load_layout.c | 9 +-------- src/randr.c | 6 +----- src/workspace.c | 37 ++++++++++++++++++++----------------- 5 files changed, 32 insertions(+), 38 deletions(-) diff --git a/include/workspace.h b/include/workspace.h index 8d109e9e..3e7183b9 100644 --- a/include/workspace.h +++ b/include/workspace.h @@ -24,6 +24,13 @@ #define NET_WM_DESKTOP_NONE 0xFFFFFFF0 #define NET_WM_DESKTOP_ALL 0xFFFFFFFF +/** + * Returns the workspace with the given name or NULL if such a workspace does + * not exist. + * + */ +Con *get_existing_workspace_by_name(const char *name); + /** * 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/src/commands.c b/src/commands.c index 500a697e..e6f6174b 100644 --- a/src/commands.c +++ b/src/commands.c @@ -1955,11 +1955,9 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { LOG("Renaming current workspace to \"%s\"\n", new_name); } - Con *output, *workspace = NULL; + Con *workspace; if (old_name) { - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(output), - !strcasecmp(child->name, old_name)); + workspace = get_existing_workspace_by_name(old_name); } else { workspace = con_get_workspace(focused); old_name = workspace->name; @@ -1970,10 +1968,7 @@ void cmd_rename_workspace(I3_CMD, const char *old_name, const char *new_name) { return; } - Con *check_dest = NULL; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(check_dest, output_get_content(output), - !strcasecmp(child->name, new_name)); + Con *check_dest = get_existing_workspace_by_name(new_name); /* If check_dest == workspace, the user might be changing the case of the * workspace, or it might just be a no-op. */ diff --git a/src/load_layout.c b/src/load_layout.c index aa7ac03c..add78875 100644 --- a/src/load_layout.c +++ b/src/load_layout.c @@ -108,18 +108,11 @@ static int json_end_map(void *ctx) { /* Prevent name clashes when appending a workspace, e.g. when the * user tries to restore a workspace called “1” but already has a * workspace called “1”. */ - Con *output; - Con *workspace = NULL; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, json_node->name)); char *base = sstrdup(json_node->name); int cnt = 1; - while (workspace != NULL) { + while (get_existing_workspace_by_name(json_node->name) != NULL) { FREE(json_node->name); sasprintf(&(json_node->name), "%s_%d", base, cnt++); - workspace = NULL; - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, json_node->name)); } free(base); diff --git a/src/randr.c b/src/randr.c index c43c6455..d7e1acca 100644 --- a/src/randr.c +++ b/src/randr.c @@ -427,11 +427,7 @@ void init_ws_for_output(Output *output, Con *content) { if (strcmp(assignment->output, output_primary_name(output)) != 0) continue; - /* check if this workspace actually exists */ - Con *workspace = NULL, *out; - TAILQ_FOREACH(out, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(out), - !strcasecmp(child->name, assignment->name)); + Con *workspace = get_existing_workspace_by_name(assignment->name); if (workspace == NULL) continue; diff --git a/src/workspace.c b/src/workspace.c index edd3ee6f..bdd6f6f4 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -19,6 +19,20 @@ static char *previous_workspace_name = NULL; * keybindings. */ static char **binding_workspace_names = NULL; +/* + * Returns the workspace with the given name or NULL if such a workspace does + * not exist. + * + */ +Con *get_existing_workspace_by_name(const char *name) { + Con *output, *workspace = NULL; + TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { + GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, name)); + } + + return workspace; +} + /* * Sets ws->layout to splith/splitv if default_orientation was specified in the * configfile. Otherwise, it uses splith/splitv depending on whether the output @@ -46,15 +60,12 @@ static void _workspace_apply_default_orientation(Con *ws) { * */ Con *workspace_get(const char *num, bool *created) { - Con *output, *workspace = NULL; - - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(output), !strcasecmp(child->name, num)); + Con *workspace = get_existing_workspace_by_name(num); if (workspace == NULL) { LOG("Creating new workspace \"%s\"\n", num); /* unless an assignment is found, we will create this workspace on the current output */ - output = con_get_output(focused); + Con *output = con_get_output(focused); /* look for assignments */ struct Workspace_Assignment *assignment; @@ -172,7 +183,6 @@ void extract_workspace_names_from_bindings(void) { */ Con *create_workspace_on_output(Output *output, Con *content) { /* add a workspace to this output */ - Con *out, *current; char *name; bool exists = true; Con *ws = con_new(NULL, NULL); @@ -198,10 +208,7 @@ Con *create_workspace_on_output(Output *output, Con *content) { if (assigned) continue; - current = NULL; - TAILQ_FOREACH(out, &(croot->nodes_head), nodes) - GREP_FIRST(current, output_get_content(out), !strcasecmp(child->name, target_name)); - exists = (current != NULL); + exists = (get_existing_workspace_by_name(target_name) != NULL); if (!exists) { ws->name = sstrdup(target_name); /* Set ->num to the number of the workspace, if the name actually @@ -222,7 +229,7 @@ Con *create_workspace_on_output(Output *output, Con *content) { ws->num = c; - current = NULL; + Con *out, *current = NULL; TAILQ_FOREACH(out, &(croot->nodes_head), nodes) GREP_FIRST(current, output_get_content(out), child->num == ws->num); exists = (current != NULL); @@ -940,14 +947,10 @@ bool workspace_move_to_output(Con *ws, const char *name) { TAILQ_FOREACH(assignment, &ws_assignments, ws_assignments) { if (assignment->output == NULL || strcmp(assignment->output, output_primary_name(current_output)) != 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) + if (get_existing_workspace_by_name(assignment->name) != NULL) { continue; + } /* so create the workspace referenced to by this assignment */ LOG("Creating workspace from assignment %s.\n", assignment->name); From 0b5799412aa1fec5f989ecdea410acc46b10e9a4 Mon Sep 17 00:00:00 2001 From: Orestis Floros Date: Tue, 27 Mar 2018 22:36:51 +0300 Subject: [PATCH 2/2] Introduce get_existing_workspace_by_num --- include/workspace.h | 7 +++++++ src/commands.c | 15 ++------------- src/manage.c | 6 +----- src/workspace.c | 25 ++++++++++++++++--------- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/include/workspace.h b/include/workspace.h index 3e7183b9..1cb24939 100644 --- a/include/workspace.h +++ b/include/workspace.h @@ -31,6 +31,13 @@ */ Con *get_existing_workspace_by_name(const char *name); +/** + * Returns the workspace with the given number or NULL if such a workspace does + * not exist. + * + */ +Con *get_existing_workspace_by_num(int num); + /** * 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/src/commands.c b/src/commands.c index e6f6174b..d2d15618 100644 --- a/src/commands.c +++ b/src/commands.c @@ -394,21 +394,15 @@ void cmd_move_con_to_workspace_number(I3_CMD, const char *which, const char *_no } LOG("should move window to workspace %s\n", which); - /* get the workspace */ - Con *output, *ws = NULL; long parsed_num = ws_name_to_number(which); - if (parsed_num == -1) { LOG("Could not parse initial part of \"%s\" as a number.\n", which); yerror("Could not parse number \"%s\"", which); return; } - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(ws, output_get_content(output), - child->num == parsed_num); - + Con *ws = get_existing_workspace_by_num(parsed_num); if (!ws) { ws = workspace_get(which, NULL); } @@ -901,7 +895,6 @@ void cmd_workspace(I3_CMD, const char *which) { */ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_and_forth) { const bool no_auto_back_and_forth = (_no_auto_back_and_forth != NULL); - Con *output, *workspace = NULL; if (con_get_fullscreen_con(croot, CF_GLOBAL)) { LOG("Cannot switch workspace while in global fullscreen\n"); @@ -910,17 +903,13 @@ void cmd_workspace_number(I3_CMD, const char *which, const char *_no_auto_back_a } long parsed_num = ws_name_to_number(which); - if (parsed_num == -1) { LOG("Could not parse initial part of \"%s\" as a number.\n", which); yerror("Could not parse number \"%s\"", which); return; } - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) - GREP_FIRST(workspace, output_get_content(output), - child->num == parsed_num); - + Con *workspace = get_existing_workspace_by_num(parsed_num); if (!workspace) { LOG("There is no workspace with number %ld, creating a new one.\n", parsed_num); ysuccess(true); diff --git a/src/manage.c b/src/manage.c index 8b306052..9dcc93f5 100644 --- a/src/manage.c +++ b/src/manage.c @@ -265,13 +265,9 @@ void manage_window(xcb_window_t window, xcb_get_window_attributes_cookie_t cooki Con *assigned_ws = NULL; if (assignment->type == A_TO_WORKSPACE_NUMBER) { - Con *output = NULL; long parsed_num = ws_name_to_number(assignment->dest.workspace); - /* This will only work for workspaces that already exist. */ - TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { - GREP_FIRST(assigned_ws, output_get_content(output), child->num == parsed_num); - } + assigned_ws = get_existing_workspace_by_num(parsed_num); } /* A_TO_WORKSPACE type assignment or fallback from A_TO_WORKSPACE_NUMBER * when the target workspace number does not exist yet. */ diff --git a/src/workspace.c b/src/workspace.c index bdd6f6f4..643b78dd 100644 --- a/src/workspace.c +++ b/src/workspace.c @@ -33,6 +33,20 @@ Con *get_existing_workspace_by_name(const char *name) { return workspace; } +/* + * Returns the workspace with the given number or NULL if such a workspace does + * not exist. + * + */ +Con *get_existing_workspace_by_num(int num) { + Con *output, *workspace = NULL; + TAILQ_FOREACH(output, &(croot->nodes_head), nodes) { + GREP_FIRST(workspace, output_get_content(output), child->num == num); + } + + return workspace; +} + /* * Sets ws->layout to splith/splitv if default_orientation was specified in the * configfile. Otherwise, it uses splith/splitv depending on whether the output @@ -225,17 +239,10 @@ Con *create_workspace_on_output(Output *output, Con *content) { DLOG("Getting next unused workspace by number\n"); int c = 0; while (exists) { - c++; - - ws->num = c; - - Con *out, *current = NULL; - TAILQ_FOREACH(out, &(croot->nodes_head), nodes) - GREP_FIRST(current, output_get_content(out), child->num == ws->num); - exists = (current != NULL); - + exists = (get_existing_workspace_by_num(++c) != NULL); DLOG("result for ws %d: exists = %d\n", c, exists); } + ws->num = c; sasprintf(&(ws->name), "%d", c); } con_attach(ws, content, false);