Question: does this handle wrapping workspace layouts with a new container?

[infokiller]

According to i3-save-layout's docs, the base workspace layout is not saved, only that of its children (not sure why this is the behavior).

Does i3-resurrect handle it automatically?

Thanks!

[JonnyHaystack]

You are right, I had overlooked this because I never do that, I always start with one window then create a new split container so I wouldn't lose any layout information.

I've tested changing the workspace layout itself and yeah that information is lost. Not sure why they made it like that but it should be an easy fix as long as append_layout supports loading the workspace properties.

Out of interest, has this affected you when using i3-resurrect? I wonder how many people actually change the workspace layout instead of creating a new split container.

And thanks for the report :smile:

[infokiller]

I think I ran into an issue because of this where the layout wasn't restored correctly, but I'm not sure if this was actually the cause. I also recently started using workspace_layout stacking in my i3 config, but I'm not sure if this is the problematic setting.

[JonnyHaystack]

Ah, yeah that would do it. I'll try and get this sorted as soon as I have time.

[infokiller]

Awesome, thanks!

[JonnyHaystack]

Hm, I'm trying to implement this now and it's proving to be less trivial than I'd hoped. I can include the whole workspace node, but when you restore it, it creates a new workspace with the same name and leaves the old one in the tree. And even though the old workspace is empty it doesn't go away because an invisible container gets left behind.

The original workspace name in my test was "10", and the new ones that are created with the name "10" displace the existing workspace with the same name, so the old workspace 10 gets automatically renamed to "10_1", "10_2", etc. Each of these workspaces has "num": 10, but their names are different.

I also tried it without saving any of the workspace's attributes (apart from nodes), but the new containers it creates are a whole two levels deeper in the tree than the original windows, and placeholder containers are not created, but instead it creates invisible containers.

All in all it doesn't look like this is going to work. I can see why it's done this way now, because it's much simpler to just be able to insert a list of new containers into a workspace and have windows swallowed into those rather than trying to selectively overwrite and merge parts of the tree (if you could overwrite the whole workspace tree I'm not sure what would happen to existing windows).

[infokiller]

Thanks for working on this! I actually haven't run into this issue since reporting it, and I'm not sure how it happens.

[JonnyHaystack]

Well, I can reproduce the issue by:

1. going to an empty workspace and changing the layout type ($mod+e) without creating a new container
2. then creating some windows
3. saving the layout
4. closing the windows
5. changing the workspace layout type back to something else
6. restoring the layout

But this is not something I ever do in practice, personally. You can avoid the issue by just creating an empty container inside the workspace then changing the layout of that, rather than changing the layout of the workspace itself. This is what I would recommend everyone to do, seeing as append_layout doesn't seem to let you restore the workspace properties cleanly.

[JonnyHaystack]

Just looking at i3's code to make sure I understand how it works.

json_content_t content = json_determine_content(buf, len);
LOG("JSON content = %d\n", content);
if (content == JSON_CONTENT_UNKNOWN) {
    ELOG("Could not determine the contents of \"%s\", not loading.\n", path);
    yerror("Could not determine the contents of \"%s\".", path);
    goto out;
}

Con *parent = focused;
if (content == JSON_CONTENT_WORKSPACE) {
    parent = output_get_content(con_get_output(parent));
} else {
    /* We need to append the layout to a split container, since a leaf
     * container must not have any children (by definition).
     * Note that we explicitly check for workspaces, since they are okay for
     * this purpose, but con_accepts_window() returns false for workspaces. */
    while (parent->type != CT_WORKSPACE && !con_accepts_window(parent))
        parent = parent->parent;
}
DLOG("Appending to parent=%p instead of focused=%p\n", parent, focused);
char *errormsg = NULL;
tree_append_json(parent, buf, len, &errormsg);
if (errormsg != NULL) {
    yerror(errormsg);
    free(errormsg);
    /* Note that we continue executing since tree_append_json() has
     * side-effects — user-provided layouts can be partly valid, partly
     * invalid, leading to half of the placeholder containers being
     * created. */
} else {
    ysuccess(true);
}

So it checks for the type of the data it loads from the file whose path you pass to append_layout, and defaults to con if it doesn't find any "type" properties.

If the content is of type "workspace", it appends the layout to the currently focused output's workspace array. If it is a con, it searches upwards from the currently focused node until it finds a workspace, then it appends the layout to the workspace's child array.

This confirms what I thought. It cannot overwrite nodes, it's very simple and just appends nodes to an array, meaning it cannot modify an existing workspace.

At least I've learned something useful from this. I think it wouldn't be too hard to implement layout restoring in the same way for sway, and sway is something I've been wanting to support for a while.

[infokiller]

You can avoid the issue by just creating an empty container inside the workspace then changing the layout of that, rather than changing the layout of the workspace itself. This is what I would recommend everyone to do, seeing as append_layout doesn't seem to let you restore the workspace properties cleanly.

How do you create an empty container?

[infokiller]

Hm, I'm trying to implement this now and it's proving to be less trivial than I'd hoped. I can include the whole workspace node, but when you restore it, it creates a new workspace with the same name and leaves the old one in the tree. And even though the old workspace is empty it doesn't go away because an invisible container gets left behind.

Re-reading this, I think my specific use case actually won't be affected from the issue you see: I only restore workspaces after I reboot, so none of my previously saved workspaces exist at this point. Is my understanding correct?

[JonnyHaystack]

Hm, I'm trying to implement this now and it's proving to be less trivial than I'd hoped. I can include the whole workspace node, but when you restore it, it creates a new workspace with the same name and leaves the old one in the tree. And even though the old workspace is empty it doesn't go away because an invisible container gets left behind.

Re-reading this, I think my specific use case actually won't be affected from the issue you see: I only restore workspaces after I reboot, so none of my previously saved workspaces exist at this point. Is my understanding correct?

No because it's the layout restoring stage which comes after you restore programs, so you would've presumably already opened those workspaces to restore the programs on them. Either way if I change the way the layout is saved/restored so that it inserts workspace nodes this will break it in situations where it the workspace does exist. For most people it would introduce confusing inconsistencies or even straight up buggy behaviour.

[JonnyHaystack]

You can avoid the issue by just creating an empty container inside the workspace then changing the layout of that, rather than changing the layout of the workspace itself. This is what I would recommend everyone to do, seeing as append_layout doesn't seem to let you restore the workspace properties cleanly.

How do you create an empty container?

Hm okay I guess you can't do this, I thought it was possible..

[JonnyHaystack]

Ok what might work is if I get it to to use the command layout <stacking|tabbed|splith|splitv> to change only the layout of the workspace (that's all we care about changing anyway). The only annoying bit is that I first have to focus the root workspace node. I'm not sure what the best/easiest way to do that is but I guess I can try calling focus parent until the current focused node is the same as the last focused node.

EDIT: Apparently i3ipc has everything I need so actually I can just do:

ws_layout = layout.get('layout', 'default')
tree = i3.get_tree()
focused = tree.find_focused()
workspace = focused.workspace()
workspace.command(f'layout {ws_layout}')

And it seems to work perfectly so far with no significant performance impact :smiley:

[infokiller]

Hm, I'm trying to implement this now and it's proving to be less trivial than I'd hoped. I can include the whole workspace node, but when you restore it, it creates a new workspace with the same name and leaves the old one in the tree. And even though the old workspace is empty it doesn't go away because an invisible container gets left behind.

Re-reading this, I think my specific use case actually won't be affected from the issue you see: I only restore workspaces after I reboot, so none of my previously saved workspaces exist at this point. Is my understanding correct?

No because it's the layout restoring stage which comes after you restore programs, so you would've presumably already opened those workspaces to restore the programs on them.

I actually don't restore the programs/windows using i3-resurrect (because I don't think it's possible to do it correctly in my case), only the workspaces. Specifically after I reboot and start my user graphical session I do the following:

1. Call i3-resurrect in a loop to restore all my workspaces
2. Switch to a new "tmp" workspace
3. Open Chromium and restore all previous browser windows in the "tmp" workspace (there's usually 20-50 such windows). After this is done, the windows are swallowed by the placeholder windows.

Either way if I change the way the layout is saved/restored so that it inserts workspace nodes this will break it in situations where it the workspace does exist. For most people it would introduce confusing inconsistencies or even straight up buggy behaviour.

Yup.

[JonnyHaystack]

I actually don't restore the programs/windows using i3-resurrect (because I don't think it's possible to do it correctly in my case), only the workspaces. Specifically after I reboot and start my user graphical session I do the following:

1. Call i3-resurrect in a loop to restore all my workspaces
2. Switch to a new "tmp" workspace
3. Open Chromium and restore all previous browser windows in the "tmp" workspace (there's usually 20-50 such windows). After this is done, the windows are swallowed by the placeholder windows.

Nice, that's a very good way to use it :smiley: I do a similar thing for browser windows

Anyways, fix for this issue is done and will be available in the next release along with some other bug fixes, extra features, and very significant performance improvements!

By the way if you ever want to try out unreleased changes, the "next" branch is the main development branch and you can clone it and install it with pip. I only merge into the master branch for releases, because the AUR package is a source package but I only want it to be updated when I create a new release.