From 73ccafdadee1116d65993d2f07fdeb718be6f371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 28 Dec 2022 15:09:25 +0100 Subject: [PATCH] module/i3: fix regression in handling of persistent workspaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bbd2394601918653f8f21cad57f5694f9b3da26a added support for ‘rename’ and ‘move’ events. In order to do that, it changed how workspace events are matched against our internal workspace list: from string comparing the workspace name, to checking i3/sway’s workspace ID parameter. This introduced a regression in our handling of persistent workspaces. A persistent workspace is one that isn’t removed from the bar when it’s deleted (empty, and switched away from) by i3/sway. This concept doesn’t exist in i3/sway, but is something we’ve added. Put simple, the way we do this is be keeping the workspace in our list, even when i3/sway tells us it has been deleted. However, at this point the workspace doesn’t have an ID anymore. And the same is true at startup; when we initialize the persistent workspaces, we only have their names. Not their IDs (since the workspaces don’t actually exist yet). To this the regression, we need to: a) fallback to looking up workspaces by name. That is, if we fail to find one with a matching ID, try again using the workspace name. For _this_ to match, we also required the matched workspace to be a persistent workspace, with an ID < 0 (which essentially means the workspace doesn’t exist yet). b) reset the ID to -1 when a persistent workspace is "deleted". Closes #253 --- CHANGELOG.md | 3 +++ modules/i3.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4420f7b..6f88a09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,10 +33,13 @@ `poll-interval` ([#241][241]). * battery: module was not thread safe. * dwl module reporting only the last part of the title ([#251][251]) +* i3/sway: regression; persistent workspaces shown twice + ([#253][253]). [239]: https://codeberg.org/dnkl/yambar/issues/239 [241]: https://codeberg.org/dnkl/yambar/issues/241 [251]: https://codeberg.org/dnkl/yambar/pulls/251 +[253]: https://codeberg.org/dnkl/yambar/issues/253 ### Security diff --git a/modules/i3.c b/modules/i3.c index f0aa137..96e215a 100644 --- a/modules/i3.c +++ b/modules/i3.c @@ -150,12 +150,19 @@ workspace_from_json(const struct json_object *json, struct workspace *ws) } static void -workspace_free(struct workspace *ws) +workspace_free_persistent(struct workspace *ws) { - free(ws->name); ws->name = NULL; free(ws->output); ws->output = NULL; free(ws->window.title); ws->window.title = NULL; free(ws->window.application); ws->window.application = NULL; + ws->id = -1; +} + +static void +workspace_free(struct workspace *ws) +{ + workspace_free_persistent(ws); + free(ws->name); ws->name = NULL; } static void @@ -250,6 +257,17 @@ workspace_lookup(struct private *m, int id) return NULL; } +static struct workspace * +workspace_lookup_by_name(struct private *m, const char *name) +{ + tll_foreach(m->workspaces, it) { + struct workspace *ws = &it->item; + if (strcmp(ws->name, name) == 0) + return ws; + } + return NULL; +} + static bool handle_get_version_reply(int sock, int type, const struct json_object *json, void *_m) { @@ -290,6 +308,35 @@ workspace_update_or_add(struct private *m, const struct json_object *ws_json) const int id = json_object_get_int(_id); struct workspace *already_exists = workspace_lookup(m, id); + if (already_exists == NULL) { + /* + * No workspace with this ID. + * + * Try looking it up again, but this time using the name. If + * we get a match, check if it’s an empty, persistent + * workspace, and if so, use it. + * + * This is necessary, since empty, persistent workspaces don’t + * exist in the i3/Sway server, and thus we don’t _have_ an + * ID. + */ + struct json_object *_name; + if (json_object_object_get_ex(ws_json, "name", &_name)) { + const char *name = json_object_get_string(_name); + if (name != NULL) { + struct workspace *maybe_persistent = + workspace_lookup_by_name(m, name); + + if (maybe_persistent != NULL && + maybe_persistent->persistent && + maybe_persistent->id < 0) + { + already_exists = maybe_persistent; + } + } + } + } + if (already_exists != NULL) { bool persistent = already_exists->persistent; assert(persistent); @@ -381,9 +428,8 @@ handle_workspace_event(int sock, int type, const struct json_object *json, void if (!ws->persistent) workspace_del(m, current_id); else { - workspace_free(ws); + workspace_free_persistent(ws); ws->empty = true; - assert(ws->persistent); } } @@ -697,6 +743,7 @@ run(struct module *mod) } struct workspace ws = { + .id = -1, .name = strdup(name_as_string), .name_as_int = name_as_int, .persistent = true, @@ -783,6 +830,9 @@ content(struct module *mod) /* Lookup content template for workspace. Fall back to default * template if this workspace doesn't have a specific * template */ + if (ws->name == NULL) { + LOG_ERR("%d %d", ws->name_as_int, ws->id); + } template = ws_content_for_name(m, ws->name); if (template == NULL) { LOG_DBG("no ws template for %s, using default template", ws->name); @@ -794,9 +844,9 @@ content(struct module *mod) ws->visible ? ws->focused ? "focused" : "unfocused" : "invisible"; - LOG_DBG("%s: visible=%s, focused=%s, urgent=%s, empty=%s, state=%s, " + LOG_DBG("name=%s (name-as-int=%d): visible=%s, focused=%s, urgent=%s, empty=%s, state=%s, " "application=%s, title=%s, mode=%s", - ws->name, + ws->name, ws->name_as_int, ws->visible ? "yes" : "no", ws->focused ? "yes" : "no", ws->urgent ? "yes" : "no",