Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Myrdden opened this issue Feb 6, 2022 · 8 comments · Fixed by #8356
Closed

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

Myrdden opened this issue Feb 6, 2022 · 8 comments · Fixed by #8356
Labels
bug Not working as intended

Comments

@Myrdden
Copy link
Contributor

Myrdden commented Feb 6, 2022

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 Myrdden added the bug Not working as intended label Feb 6, 2022
@Myrdden
Copy link
Contributor Author

Myrdden commented Feb 10, 2022

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
Copy link
Contributor Author

Myrdden commented Feb 15, 2022

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:

sway/sway/commands/move.c

Lines 232 to 238 in f707f58

static void container_move_to_container(struct sway_container *container,
struct sway_container *destination) {
if (container == destination
|| container_has_ancestor(container, destination)
|| container_has_ancestor(destination, container)) {
return;
}

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 Myrdden changed the title move window to mark <mark> is usually not doing anything, and when it does, it does the wrong thing container_move_to_container doesn't behave like it's i3 equivalent Mar 15, 2022
@Myrdden
Copy link
Contributor Author

Myrdden commented Mar 15, 2022

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.

  1. 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
Copy link

korreman commented Jul 3, 2022

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
Copy link

i have same problem on sway v1.8

@justyn
Copy link

justyn commented Aug 14, 2023

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
Copy link

ccdunder commented Oct 26, 2024

#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.

@ccdunder
Copy link

ccdunder commented Oct 30, 2024

I whipped up a solution in #8418. I would greatly appreciate feedback on the general approach.

The one remaining issue is that this won't work for moving a container up to the workspace, because a workspace can't be marked and treated like a container in sway like in i3. My guess is that changing that would be a more involved change?

Thank you in advance for guidance, maintainers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
5 participants