awesome icon indicating copy to clipboard operation
awesome copied to clipboard

constrain awful.mouse.client.move/resize if maximized

Open mvduin opened this issue 6 years ago • 4 comments

So, I finally to upgrade from awesome 4.0 to the latest version and just try to dig into the problems that kept me on 4.0 and try to fix them myself.

I originally reported this bug as #2061, which got merged into #2036 which was reworded to apply to resize requests by the client and by the user. The issue however was closed without ever fixing the latter. Hopefully this now finally fixes #2036 entirely.

This also reverts 51ddb5639e9c, which was another partial fix which is unnecessary now and which had the undesirable side-effect of causing the mouse to not be grabbed at all.

mvduin avatar May 12 '19 23:05 mvduin

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.21%. Comparing base (f03d547) to head (5662b63). Report is 1462 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2767      +/-   ##
==========================================
- Coverage   86.21%   86.21%   -0.01%     
==========================================
  Files         541      541              
  Lines       36953    36953              
==========================================
- Hits        31859    31858       -1     
- Misses       5094     5095       +1     
Flag Coverage Δ
gcov 74.98% <ø> (-0.02%) :arrow_down:
luacov 89.13% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
lib/awful/mouse/init.lua 59.23% <100.00%> (ø)

... and 1 file with indirect coverage changes

codecov[bot] avatar May 13 '19 00:05 codecov[bot]

Thanks for this. Do you think you can find some time and add regression tests for this? Changes to maximizations tend to always make a subset of the userbase unhappy, no matter what they are. At least with test it is possible to "prove it is the intended behavior". You can do it either in tests/test-maximization.lua or tests/test-resize.lua. The former is a better home for this test, but the later code is using fake_input to emulate mouse movements, which is what you need to do to test this.

I'm still not very familiar with awesome's codebase, and I have no idea why awful.placement was designed to work the way it does.

The original problem was that c:geometry() was called directly from about 20 different places. Implicit focus stealing was also done all over the place. In the case of c:geometry(), most code paths that did it also had their own hardcoded (and more often than not, duplicated) geometry algorithm. A couple of them also had their own memento to save older geometries. On top of that, some code like tiled layouts and fullscreen also listened to changes to the geometry just for the sake of reverting them because these code paths considered their changes "more important".

When I tried to implement dynamic layouts and tabs, this became unworkable. There was no way to integrate more [third party] geometry code without some other code getting in the way and undoing the changes. This is when the request::geometry API was born. It invert the geometry change architecture so that rather than directly changing the geometry, all code must request the geometry to be changed along with a context and some metadata hints. Then you have a set of handlers that listen for such request, filter what they are capable of handling and apply the changes. Those handlers can be disconnected and replaced by modules. However, the most important aspect is that there is a lot less handlers than requesters, which simplifies the pipeline.

The second step for this was to extract all the geometry code away from the place where the newly created request::geometry is introduced and place this code in a common repository where the handler can just select the algorithm and apply it. awful.placement already existed and was extended as a do-it-all class with some compositing and memento system bolted on because some existing code that was ported to the request::geometry API required it.

All corner cases required by all users required to add some knobs to configure each algorithm. Some users, such as the awful.rules placement properties, required to be able to concatenate different algorithms to express some intents in a more declarative way. Before that, rc.lua also hardcoded some geometry changes which had to be generalized to integrate themselves in the new geometry pipeline. awful.placement ended up the way it is with all the over engineering and complexity so that this complexity would not end up splattered across the entire codebase. While awful.placement itself is nearly unmaintainable, everything else became very easier to work with and extend. It's a tradeoff, but in my opinion one that's worth it in the long run.

continuing to use awesome 4.0 (which was my solution so far, since later versions were too broken for me to be usable),

As you are aware since you reported some of the bugs, this is not universally true. Some people considered v4.0 to be the broken one and v4.2 the one that worked. v4.1 was indeed rather bad at maximization. The different bugs reported against what was considered by some to be broken in v4.0 created new corner cases that were not detected before the release and ended up regressing the ability to client to maximize themselves. When a client maximized itself, Awesome maximization state became out of sync then the behavior was rather random. You can read test-maximization.lua and get an idea of the many corner cases that are now handled (yours currently isn't one, hence this pull request). The changes done in v4.1 and v4.2 also had the nasty side effect of breaking the original design of the memento implementation. Because the API is "stable" it could not be broken to replace the implementation and new changes were bolted on top to make some people happy. You have some "sigh..." caliber comments in the code where it is rather obvious I wasn't very happy with the changes being made. As an attempt to clean this up, me and @blueyed added the "immobilized" properties to simplify some of the algorithms. It is not yet used in all the place where it should be used, thanks for making progress toward that.

Before v4.0, it worked "better" because some code, as mentioned above, listened to geometry changes and would unconditionally revert them. But, also as mentioned, this design could work since it both caused things to flicker, added code duplication, added feedback loops and could not allow modules to cleanly plug themselves to add often requested features such as i3-style layouts and tabs.

Elv13 avatar May 13 '19 19:05 Elv13

At least all existing tests are passing.. ;) \o/

blueyed avatar May 14 '19 03:05 blueyed

Do you think you can find some time and add regression tests for this?

Ping? (I assume it means "no"?)

Elv13 avatar Jun 08 '19 20:06 Elv13