swaywm / sway

i3-compatible Wayland compositor
https://swaywm.org
MIT License
14.57k stars 1.11k forks source link

`container_move_to_container` doesn't behave like it's i3 equivalent #6818

Closed Myrdden closed 1 month ago

Myrdden commented 2 years ago

Sway Version: 1.7

Logs: here

I'm migrating here from i3 because X just slowly descended into graphical madness, I have a few keybindings which call a script. The relevant one to this is:

parent=$(nodePath | jq -rcM '.[-3] | if type == "null" then empty else .id end')
if [ "$parent" != '' ]; then
  swaymsg "[con_id=$parent] mark _parent; move window to mark _parent; unmark _parent"
fi

Where nodePath makes an array of each container down to the focused node, so [-3] of that is the "grandparent" (parent's parent). So it takes a window, and then makes it a child of it's parent's parent, thus moving it up the tree one level. This works fine on i3, and does absolutely nothing when used in sway. There's no error, the log says it's running the command, but nothing happens. When I go into a console and try to dispatch that set of commands manually with swaymsg, it also doesn't do anything. The issue doesn't appear to lie further down in the tree fetching or anything, because when inspecting the command as it runs the value of $parent is consistent and correct, the subsequent commands just aren't doing anything. (Well, sometimes it will actually place the mark, which can be seen by removing the unmark bit, but even then this extremely is inconsistent)

And, while we're here, when I try to move containers to marks in other ways, it's seemingly random and sporadic whether it will work or not. I think it might be failing to do anything whenever the marked container is a direct ancestor of the focused one, but I'm not entirely certain on that. But also, when it does work, the container being moved always loses focus, which is not how the same command behaves in i3.

Myrdden commented 2 years ago

Okay actually looks like the above isn't entirely accurate, the focus does remain on the correct container as it's moved, but the actual screen itself doesn't move with it, so for instance in a tabbed situation if I move a focused container into a child container, I'm left looking at a different tab without focus, and the focus goes into the non-visible container.

Myrdden commented 2 years ago

Right, so I'm not going to pretend to grasp this fully but I think I've tracked down at least the issue regarding the "macro" in the script. So in the move command here: https://github.com/swaywm/sway/blob/f707f583e17cb5e8323ceb4bfd951ad0465b7d10/sway/commands/move.c#L232-L238 It's checking both that the destination is not a descendant of the container and that the container is not a descendant of the destination. Meanwhile, what I'm like 90% sure is the equivalent bit of code in i3 here (Apparently I can't embed lines from other repos)

    /* We compare the focus order of the children of the lowest common ancestor. If con or
     * its ancestor is before target's ancestor then con should be placed before the target
     * in the focus stack. */
    Con *lca = lowest_common_ancestor(con, parent);
    if (lca == con) {
        ELOG("Container is being inserted into one of its descendants.\n");
        return;
    }

Only checks that the container being moved is not an ancestor of it's destination. So I pulled source and just commented out move.c:235, and now my "macro" script, along with the other manual move-to-mark commands, appear to be functioning as expected now. However, the issues regarding loss of focus still remain and I've not had much luck tracking those down as I barely know how any of this works.

Myrdden commented 2 years ago

Right, so I've not had a lot of time to work on this, but some other things I've noticed and have been trying to address in the previously mentioned PR,

  1. When a focused window is moved into a non-focused container, it keeps the focus, which is fine, but the display or window or screen or what have you doesn't update, so you just end up losing the focus, for instance, say you have:

    T[a b c H[d e] *f* g]

    where *f* is focused, and as such the T[] is also "focused". Now you move f into the H[], thus you get:

    T[a b c H[d e *f*] g]

    and f still has the focus, but in spite of this the T[] is still "focused", or in view, meaning you'll be looking at g, but g won't have the focus. If you type anything it goes to f, and if you focus directionally it focuses from f, within the H[], none of which you can actually see unless you pull the focus all the way back out to the T[] container (on a, b, c, or g) and then focus back in to the H[]. With my limited understanding of the framework I've not made any headway on finding out why this is.

  2. i3's equivalent of this command has an algorithm in place that's completely absent from sway's version, that says that when you move a window into a parent container, it's placed immediately after whatever in that parent container has focus, such that if you have:

    H[a b V[c *d*] e f]

    where d has focus, and you move to H[], via a mark or some other means, you'd end up with:

    H[a b V[c] *d* e f]

    with d immediately after the V[], because the V[] was in "focus" when it contained d. Obviously as sway does not currently allow you to move children into their parent containers, this algorithm I guess had no need to be implemented. But, in order to actually be par with i3, it probably should. I've been trying to take a stab at it with #6835, but have not really had any time to devote to it as work and life get in the way.

korreman commented 2 years ago

Getting back to this, and I've realized that you can circumvent the ancestor check by first moving the container to a temporary workspace, then moving it to your desired location. As long as it's happening in a single sequenced command, the workspace never shows up on screen. For example:

[con_mark=_source] move container to workspace _temp; [con_mark=_source] move container to mark _dest

There's still the issue that _source is placed at the end of _dest, not next to its inactive focus. As far as I can tell, there is currently no reliable way to move a node from one position in the tree to another.

rickyrsyah commented 1 year ago

i have same problem on sway v1.8

justyn commented 1 year ago

I'm not sure that I've fully followed the issue preventing sway following the same move-to-mark logic as i3, but @Myrdden regarding your desire to add a move [container|window] [to] con_id <con_id> command, have you seen: https://github.com/WillPower3309/swayfx ?

ccdunder commented 1 day ago

8356 seems a partial fix. It makes it possible to move a child up in the tree, but it always makes it the last sibling/uncle, rather than the nearest sibling/uncle as in i3.

Example:

  1. Start with the layout:
    • 3 tabs
    • 1st tab has a split (2 children)
  2. Move one of the 2 children to its grandparent (making it a sibling of its parent).

Expected behavior (matches i3): there are 4 tabs, the child has become the 2nd tab.

Actual behavior: there are 4 tabs, the child has become the 4th tab.

Happy to take a crack at this if anyone has a suggestion as to how at a high-level.