Converting outer gaps into side-specific gaps, also allowing outer/horizontal/vertical #211
Conversation
Outer gaps split into top, right, bottom, and left Outer, horizontal, and vertical options added to control all/some sides README updates for new configuration options
# Conflicts: # src/workspace.c
|
@cameronleger Hey, thanks for the PR and sorry for the drastically long delay. At quick glance this looks all really good. We'll be releasing i3 4.16 in the next few days and afterwards I think might be a good time to give this PR a chance. If you're still interested, could you update the PR to resolve the merge conflicts? I'd appreciate it! |
|
On it :) FWIW, I've been running this without issues ever since I made it. |
|
Oh and if you do, please also make sure to properly rebase the PR onto gaps-next (instead of merging it in) and squashing the commits down so we can have a clean (and potentially revertable) commit history. Thank you! |
|
Awesome, I'm happy to hear that we already have (quite some) time with someone running it. |
|
@SirCmpwn FYI, if we merge this I don't know if you'd want a change to add compat for it in sway. |
|
Thanks, I'll add a ticket. |
|
@Airblader, one more thing to consider: the https://github.com/acrisci/i3ipc-python module completely breaks when this branch is used. I assume the same is true of https://github.com/acrisci/i3ipc-python. I made a Python app to manage my workspaces, so I have already made the required changes to that module in a local repository for it to continue working with this change. |
|
Can you specify "completely breaks"? The old syntax still works, the only breaking change I could think of is when using the new feature, the inner/outer properties on the IPC responses will no longer be correct, right? That'd mean that users not using these new features shouldn't have issues. Or am I mistaken? |
|
OK please ignore that, I must have been mistaken as I just pulled the latest code there, installed it, and it still works. I definitely remember modifying it, but perhaps it was with the other branch that we're not going with. |
|
4.16 is released now, so we can go ahead with this once the changes are done. Just a minor note, I will be travelling for the next few days, so I'm not 100% sure I have time to merge it during that time, but I'll be back by the end of the week. |
|
Superseded by #249 Note: You can force-push onto an existing PR to update it, no need to open new ones. ;-) |
|
nicely done! would be awesome to have another release with this feature so pkg managers can pick it up |
|
The releases of i3-gaps are aligned with i3. I specifically waited for the 4.16 release so that this change will have a period of testing before being included in a release. On Arch-based systems you can find i3-gaps-next-git in the AUR. |
|
@Airblader got it, that makes sense. I've reinstalled i3-gaps-next-git and now have the new feature. works like a charm! thanks |
Commit Notes:
This is an alternative to #210 based on user feedback. It has a number of important differences that I want to layout for discussion.
The previous PR added horizontal and vertical gaps. This PR replaces outer gaps with top, right, bottom, and left gaps while providing syntatical sugar for modifying outer, horizontal, and vertical gaps. While the commands and config settings are extremely similar to the current version, the backend is different and I bet that will break some backwards compatibility. I do not know enough to imagine the possibilities, but later I will touch on i3ipc's impacts. Similar to how one should apply workspace-specific gaps after applying global gaps, now one must apply higher-level gaps before side-specific gaps. Otherwise, 'setting' an outer gap would replace the side-specific values.
For the example README config, I slightly tweaked it and made the return key go back into the top gaps mode because I found that when modifying anything other than inner/outer, you're probably adjusting a few things.
Most changes are similar to previous commits and are probably easy to compare and understand. However, I refactored config_directives.c in a different way than before. Instead of passing a single value, it passes a gaps_t struct with only the relevant members modified, and it only applies updates to the relevant members based on the scope. Also, it is overall more verbose, however there is less computations on strings and it exits earlier when possible.
Back to i3ipc.c; originally I was swapping the outer node with the new top/right/bottom/left ints. However, I noted that this broke compatibility with the Python i3ipc package since it has minimal support for gaps, but it expects inner and outer in the JSON. I don't know how many programs this would affect, but I have created my own extensive workspace manager based on this package and it couldn't start because of the i3ipc module failing. So, I've left an outer definition in the JSON, but it only contains the 'top' value. At least i3ipc will continue to work, but anything relying on modifying gaps will probably not function correctly. I'm up for suggestions on managing this; perhaps making 'outer' a map that contains all side values will work, but that might also break outdated programs.
Finally, re: #22, I applied and updated the potential fix located here, however I don't think it was working correctly so I have not included it here. It definitely solved the fact that only edge containers are modified by outer gaps, but it introduced more issues for the right and bottom sides. When I 'set' them to 0, they extended much further past the screen than the edge, and I had to 'minus' them back to normal. I'm not too versed on the relevant math in render.c to know what to tweak, but I can look into this more. I believe this issue will be more pronounce with this PR; for example add three windows on a workspace, only adjust the left or right gaps, and notice that only the left-most or right-most containers will adjust.