Prepare arrange code for type safe arguments #2511
Conversation
RyanDwyer
force-pushed
the
refactor-arrange
branch
3 times, most recently
from
0d5dc48
to
a1f34d9
Compare
I don't personally use gaps so I could be misunderstanding something, but this doesn't sound right to me. The gaps appear to only be copied from the parent to the child if the child does not have gaps and the parent does. Since it looks like it is possible to override gaps on a per container/view basis, wouldn't it be possible that the gaps at the workspace-level are not the correct gaps to apply? |
|
Hmm. That's true. Looking at the i3-gaps readme, it looks like |
Based on the changes to sway/commands/gaps.c in the open PR #2131, it looks like you are correct and this is actually a bug and not intentional. I'm fine with removing the ability to change gaps at the container/view level for now. I think that if it is re-implemented in the future, it should be done as an extension (rather than a deviation) and use an unused scope (ex |
|
We once had @Airblader explain how our gaps were supposed to be and confirmed that we got it right at some point. |
|
Yes, I can confirm again that |
|
Found a bug in this - please hold off on merging. |
RyanDwyer
changed the title
This commit changes the arrange code in a way that will support type safe arguments. The arrange_output et al functions are now public, however I opted not to use them directly yet. I've kept the generic arrange_windows there for convenience until type safety is fully implemented. This means this patch has much less risk of breaking things as it would otherwise. To be type safe, arrange_children_of cannot exist in its previous form because the thing passed to it could be either a workspace or a container. So it's now renamed to arrange_children and accepts a list_t, as well as the parent layout and parent's box. There was some code which checked the grandparent's layout to see if it was tabbed or stacked and adjusted the Y offset of the grandchild accordingly. Accessing the grandparent layout isn't easy when using type safe arguments, and it seemed odd to even need to do this. I determined that this was needed because a child of a tabbed container would have a swayc Y matching the top of the tab bar. I've changed this so a child of a tabbed container will have a swayc Y matching the bottom of the tab bar, which means we don't need to access the grandparent layout. Some tweaks to the rendering and autoconfigure code have been made to implement this, and the container_at code appears to work without needing any changes. arrange_children_of (now arrange_children) would check if the parent had gaps and would copy them to the child, effectively making the workspace's gaps recurse into all children. We can't do this any more without passing has_gaps, gaps_inner and gaps_outer as arguments to arrange_children, so I've changed the add_gaps function to retrieve it from the workspace directly. apply_tabbed_or_stacked_layout has been split into two functions, as it had different logic depending on the layout. Lastly, arrange.h had an unnecessary include of transaction.h. I've removed it, which means I've had to add it to several other files.
* In layout command, arrange parent of parent - not sure why this is needed but it is * Remove gap adjustment when rendering * Workspace should use outer gaps, not inner * Add exceptions for tabbed and stacked containers * Don't mess with gap state when splitting a container
RyanDwyer
changed the title
|
All good now. The bug was a rendering issue when using tabbed containers with inner gaps. The tab bar was misaligned. |
RyanDwyer
force-pushed
the
refactor-arrange
branch
from
a1f34d9
to
126a82f
Compare
|
Thanks! |
This commit changes the arrange code in a way that will support type safe arguments.
The
arrange_outputet al functions are now public, however I opted not to use them directly yet. I've kept the genericarrange_windowsthere for convenience until type safety is fully implemented. This means this patch has much less risk of breaking things as it would otherwise.To be type safe,
arrange_children_ofcannot exist in its previous form because the thing passed to it could be either a workspace or a container. So it's now renamed toarrange_childrenand accepts alist_t, as well as the parent layout and parent's box.There was some code which checked the grandparent's layout to see if it was tabbed or stacked and adjusted the Y offset of the grandchild accordingly. Accessing the grandparent layout isn't easy when using type safe arguments, and it seemed odd to even need to do this. I determined that this was needed because a child of a tabbed container would have a swayc Y matching the top of the tab bar. I've changed this so a child of a tabbed container will have a swayc Y matching the bottom of the tab bar, which means we don't need to access the grandparent layout. Some tweaks to the rendering and autoconfigure code have been made to implement this, and the
container_atcode appears to work without needing any changes.arrange_children_of(nowarrange_children) would check if the parent had gaps and would copy them to the child, effectively making the workspace's gaps recurse into all children. We can't do this any more without passinghas_gaps,gaps_innerandgaps_outeras arguments toarrange_children, so I've changed theadd_gapsfunction to retrieve it from the workspace directly.apply_tabbed_or_stacked_layouthas been split into two functions, as it had different logic depending on the layout.Lastly,
arrange.hhad an unnecessary include oftransaction.h. I've removed it, which means I've had to add it to several other files.