module/i3: fix regression in handling of persistent workspaces

bbd2394601 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
This commit is contained in:
Daniel Eklöf 2022-12-28 15:09:25 +01:00
parent 02d281df58
commit 73ccafdade
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
2 changed files with 59 additions and 6 deletions

View file

@ -33,10 +33,13 @@
`poll-interval` ([#241][241]). `poll-interval` ([#241][241]).
* battery: module was not thread safe. * battery: module was not thread safe.
* dwl module reporting only the last part of the title ([#251][251]) * 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 [239]: https://codeberg.org/dnkl/yambar/issues/239
[241]: https://codeberg.org/dnkl/yambar/issues/241 [241]: https://codeberg.org/dnkl/yambar/issues/241
[251]: https://codeberg.org/dnkl/yambar/pulls/251 [251]: https://codeberg.org/dnkl/yambar/pulls/251
[253]: https://codeberg.org/dnkl/yambar/issues/253
### Security ### Security

View file

@ -150,12 +150,19 @@ workspace_from_json(const struct json_object *json, struct workspace *ws)
} }
static void 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->output); ws->output = NULL;
free(ws->window.title); ws->window.title = NULL; free(ws->window.title); ws->window.title = NULL;
free(ws->window.application); ws->window.application = 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 static void
@ -250,6 +257,17 @@ workspace_lookup(struct private *m, int id)
return NULL; 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 static bool
handle_get_version_reply(int sock, int type, const struct json_object *json, void *_m) 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); const int id = json_object_get_int(_id);
struct workspace *already_exists = workspace_lookup(m, 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 its an empty, persistent
* workspace, and if so, use it.
*
* This is necessary, since empty, persistent workspaces dont
* exist in the i3/Sway server, and thus we dont _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) { if (already_exists != NULL) {
bool persistent = already_exists->persistent; bool persistent = already_exists->persistent;
assert(persistent); assert(persistent);
@ -381,9 +428,8 @@ handle_workspace_event(int sock, int type, const struct json_object *json, void
if (!ws->persistent) if (!ws->persistent)
workspace_del(m, current_id); workspace_del(m, current_id);
else { else {
workspace_free(ws); workspace_free_persistent(ws);
ws->empty = true; ws->empty = true;
assert(ws->persistent);
} }
} }
@ -697,6 +743,7 @@ run(struct module *mod)
} }
struct workspace ws = { struct workspace ws = {
.id = -1,
.name = strdup(name_as_string), .name = strdup(name_as_string),
.name_as_int = name_as_int, .name_as_int = name_as_int,
.persistent = true, .persistent = true,
@ -783,6 +830,9 @@ content(struct module *mod)
/* Lookup content template for workspace. Fall back to default /* Lookup content template for workspace. Fall back to default
* template if this workspace doesn't have a specific * template if this workspace doesn't have a specific
* template */ * template */
if (ws->name == NULL) {
LOG_ERR("%d %d", ws->name_as_int, ws->id);
}
template = ws_content_for_name(m, ws->name); template = ws_content_for_name(m, ws->name);
if (template == NULL) { if (template == NULL) {
LOG_DBG("no ws template for %s, using default template", ws->name); 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" : ws->visible ? ws->focused ? "focused" : "unfocused" :
"invisible"; "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", "application=%s, title=%s, mode=%s",
ws->name, ws->name, ws->name_as_int,
ws->visible ? "yes" : "no", ws->visible ? "yes" : "no",
ws->focused ? "yes" : "no", ws->focused ? "yes" : "no",
ws->urgent ? "yes" : "no", ws->urgent ? "yes" : "no",