(WIP) Rework tree_flatten

[ilikeheaps]

Complete rework of tree_flatten. Also includes code for #3500 because they share common functions - I'll remove appropriate parts if we don't want it.

Principles

A container is redundant if at the same time:

1. It's not focused
2. None of its children is focused (optional?)
3. Its layout is split horizontal/vertical
4. Either:
   • its layout is the same as its parent
   • it has only one child

I'm currently having problems when the parent is a workspace because sometimes there is a container being implicitly created when you change the layout of a workspace and I would try to flatten it ending up in an endless loop. So I'm not flattening workspaces' children until I figure out how exactly that works.

Reasoning for the rules

1. If a container is focused the user might want to perform some action on it.
2. Similar to (1.) but I'm not sure what's being focused when we create a new container so it might be not needed or needed only in certain cases
3. Tabbed/stack containers always are possibly desired by the user to label and structurize things
4. Assuming (3.) it just seems right

State of the implementation

Seems to transform the tree correctly but messes up the order of containers and their sizes. I temporarily removed regular calls to tree_flatten on certain events (because they were in functions that I currently use in tree_flatten and ended up in endless loops) and made a binding to call it on-demand for testing.

TODO:

• scale container scales (uh, their percentage size) so that they don't actually change in absolute size
• keep the order of containers (is this the focus list?)
• decide when to call tree_flatten

Alternative approach

I'm considering whether it wouldn't be possible to keep the tree flattened at all times by running some smaller function checking invariants at every change instead of walking the whole tree every few operations. I think that approach could be complicated so I'm sticking with tree_flatten for now.

[orestisfl]

#3235 and related discussions / issues

[Airblader]

Haven't looked at the changes yet, but assumption #1 already sounds wrong to me — operations can also be done on non-focused containers.

Could you explain what exactly this rework fixes (other adding the feature from your feature request)?

[ilikeheaps]

I want to fix some issues mentioned in #3235 - currently it's very easy to make a tree with redundant containers that won't be flattened (like H[w1 H[w2 w3]] === H[w1 w2 w3]). I'm not sure which issues exactly I want to solve - I'm hoping to make a mechanism with fairly simple rules that make sense and see where that ends up. I think the ones I proposed are fairly simple but I'm not sure if they make sense.

On (1.): if one wanted to create a container away from the focused point of the tree, he would do that in order to change the perceived layout; that is if he made a layout that would be flattened it would have to be a temporary structure. I see few solutions: not running `tree_flatten' during the sequence of operations or providing commands that avoid flattenable temporary states. These two could (or would) break compatibility.

Another possible solution I see is introducing some kind of "spawn invulnerability" to containers so we wouldn't collapse a freshly created container that the user wanted to actually use for something. It could be configurable so that a container can't be flattened during first X calls of tree_flatten. Heck, it could have an option of infinite for compatibility (and keeping the old behaviour during invulnerability but that would make code twice as big).

Essentially most solutions seem to be breaking compatibility to some extent because this tree_flatten is fundamentally different from the old one. The question is: do people actually use things that would be broken by this change? If you have some specific example I think it would be a good basis for further consideration of the rules. If the compatibility is absolutely needed I think the invulnerability thing would be good (without old behaviour - there isn't someone relying on the containers collapsing in a script, is there?).

[Airblader]

it's very easy to make a tree with redundant containers that won't be flattened (like H[w1 H[w2 w3]] === H[w1 w2 w3]).

Those structures aren't and shouldn't be equal. I don't see a redundant container in there. This is a perfectly valid setup depending on how you want resizing to work and whether you want to, say, change the layout of that inner container.

I don't have time to go into detail on the rest at the moment, but neither of the other proposals sound like good ideas to me, mostly for the reasons you mentioned: they fundamental change — and break — how i3 containers work. That's a high price to pay, for a benefit I'm not yet seeing.

[ilikeheaps]

I guess I could make it so that by default only h/v splits with one child are flattened (with some restrictions so that it doesn't immediately close a container that the user wants to use) which I think would fix unintuitive behaviour like #3003. In that case making the behaviour that I desire optional would be trivial and wouldn't make the code any more complex. I think that making behaviour expected in #3001 optional would be quite simple as well. Anyways, I'll keep thinking about it. I wanted to consider two containers the same if one can always mimic the other by using appropriate parameters but resizing behaviour is important indeed (depending on how one uses i3).

[Airblader]

I think that if we want to rework how tree flattening works, we should first come up with the requirements, the issues of the current implementation actually justifying any change (such as #3003), discuss what the solution should be and, most importantly, define a comprehensive set of test cases through which we can run any new implementation for it (we do already have some tests, but I'm not sure how thorough they are).

I'd like to exercise a high level of caution here because this is really at the core of what defines i3, and container manipulation is probably one of the most important use cases of existing scripts and certainly of day to day user interaction. And I don't think tree flattening is fundamentally broken at the moment, so there's no need to rush things — let's take our time thinking about this one.

As a pretty simple first requirement: we shouldn't break any scenario for a user. That is, we do not remove containers that can in fact serve a purpose, but only those that are truly unnecessary in the sense that any layout derived from the unflattened structure can also be derived from the flattened one.
The reasoning for this is quite simple: I'll take a user having to perform an extra step to close a (for them in that moment unnecessary) container over a user who cannot achieve the layout they need. :-)

Two more points, though I haven't thought them through entirely:

1. Focused containers should never be treated differently.
2. I'd like to avoid having some kind of "pristine" flag on containers. Layout changes should behave the same regardless of whether commands are executed in sequence or whether there's another command in between that affects something entirely different.

I think as such I'll close this PR for now and we move any discussion to the issue tracker (perhaps we open a fresh issue where we also collect a list of links to related issues like #3003).

Note, please, that #3500 is completely orthogonal/independent of such a change as that discusses adding a new feature. We shouldn't mix these things.

[ilikeheaps]

Note, please, that #3500 is completely orthogonal/independent of such a change as that discusses adding a new feature. We shouldn't mix these things.

Sure, it only got mixed up in here because of how I organized my branches.

[CarloWood]

Making a special command for a (new) tree flatten would solve all mentioned concerns of Airblader: it would not break any script or change anything for existing users. It would be a NEW command that people can bind and use if they want. I see no reason not to add such a new command. I do agree that it shouldn't be like the function "tree_flatten" and work on the whole tree though. It should work on the focused window/container (and it's children) only.