bl-aerosnap: rewrite it to use XLib and EWMH

[Hjdskes]

Using XLib and EWMH directly is a cleaner solution than spawning subprocesses all the time. Whilst working on this, I made several other changes to improve the performance and maintainability of bl-aerosnap:

• Remove as much global state as possible;
• Read and write history only once;
• Use sys.exit as opposed to just exit (https://docs.python.org/2/library/sys.html#sys.exit);
• Use tempfile to get the system's temp directory, as opposed to hardcoding /tmp;
• Add --top and --bottom options.

Since I changed the code quite significantly, I wrote a small test script that compares the debug statements (which I'd have to remove prior to this being merged) to see if the behavior is what we want it to be. Note that you should probably change the coordinate strings for your own setup (mine is a 1366x768 laptop monitor with a single panel in the bottom of 30 pixels). I could probably make it setup independent by working with /tmp/bl-aerosnap-$(id -u) instead of debug statements, but we're talking about a test script here :stuck_out_tongue_winking_eye:.

#!/usr/bin/env sh

# Remove old history, if any
rm "/tmp/bl-aerosnap-$(id -u)"

# Test regular snap/restore, by snapping and restoring the active window 10
# times per snapping direction. If the debug prints are not equal, the test is
# stopped and exists with an exit status of 1.
for i in "left" "right" "top" "bottom"; do
    for j in $(seq 1 10); do
        FROM="$(python bin/bl-aerosnap --$i | grep "from" | cut -c23- -)"
        TO="$(python bin/bl-aerosnap --$i | grep "Restoring" | cut -c15- -)"
        if [ "$FROM" != "$TO" ]; then
            printf "Geometries do not match:\nFrom: %s\nTo: %s\n" "$FROM" "$TO" 1>&2
            exit 1
        fi
    done
done

# Imagine the following scenario: user snaps terminal to the left, but now wants
# it snapped to the right. In this case, several things should happen:
#1. The user should be able to directly snap to the right, without having to
#    restore the window first.
#2. The original position (the one prior to snapping to the left) should be
#    remembered to restore to.
# This piece of code tests whether this happens, by first snapping to the left
# and then to the right. If it works for these two directions and this
# combination, it works for all directions and combinations.
ORIG="$(python bin/bl-aerosnap --left | grep "from" | cut -c23- -)"
RIGHT="$(python bin/bl-aerosnap --right | grep "from" | cut -c23- -)"
if [ "$RIGHT" != "0 0 681 713" ]; then
    printf "Not snapping from the left" 1>&2
    exit 1
fi
TO="$(python bin/bl-aerosnap --right | grep "Restoring" | cut -c15- -)"
if [ "$ORIG" != "$TO" ]; then
    printf "Geometries do not match:\nFrom: %s\nTo: %s\n" "$ORIG" "$TO" 1>&2
    exit 1
fi

There is one issue remaining, namely that windows that originally have a negative position will not be able to be restored. Help with this issue would be appreciated, as it looks like it is quite a deep issue. A traceback of this issue is the following:

 > ./bin/bl-aerosnap --left
Restoring to: -195 192 790 422
Traceback (most recent call last):
  File "./bin/bl-aerosnap", line 279, in <module>
    main()
  File "./bin/bl-aerosnap", line 262, in main
    window_restore(ewmh, active_win)
  File "./bin/bl-aerosnap", line 183, in window_restore
    ewmh.set_move_resize_window(win, x=geom[0], y=geom[1], width=geom[2], height=geom[3])
  File "./bin/bl-aerosnap", line 128, in set_move_resize_window
    [gravity_flags, int(x), int(y), int(width), int(height)])
  File "./bin/bl-aerosnap", line 78, in _set_property
    data=(datasize, data))
  File "/usr/lib/python3.5/site-packages/Xlib/protocol/rq.py", line 1413, in __init__
    self._binary = self._fields.to_binary(**keys)
  File "/usr/lib/python3.5/site-packages/Xlib/protocol/rq.py", line 1012, in to_binary
    v, l, fm = f.pack_value(field_args[f.name])
  File "/usr/lib/python3.5/site-packages/Xlib/protocol/rq.py", line 707, in pack_value
    data, dlen, fmt = PropertyData.pack_value(self, value)
  File "/usr/lib/python3.5/site-packages/Xlib/protocol/rq.py", line 688, in pack_value
    data = array(array_unsigned_codes[size], val).tostring()
OverflowError: can't convert negative value to unsigned int

Another potential issue that I want to point out, is that currently I copied in the relevant bits of pyewmh because this module is not available as a Debian package. (Don't worry, the licenses are compatible). If preferred, we could probably package this ourselves and just import the upstream code, which is probably cleaner.

Please test this thoroughly, with your funkiest setups (especially multi monitor, as I currently do not have access to my second monitor!)

Looking forward to your feedback!

[johnraff]

@Unia Many thanks for your work on this!

If you could post some very specific instructions on how you'd like this tested, maybe you could start a "RFT" thread in the BunsenLabs forum to request testers, according to this recent post: https://forums.bunsenlabs.org/viewtopic.php?pid=35655#p35655

Before merging this into deuterium we need to be reasonably sure the remaining issues will be fixed in time for the deuterium merge into master - probably in another two or three weeks, I'm guessing.

[Hjdskes]

If you could post some very specific instructions on how you'd like this tested, maybe you could start a "RFT" thread in the BunsenLabs forum to request testers, according to this recent post: https://forums.bunsenlabs.org/viewtopic.php?pid=35655#p35655

Done: https://forums.bunsenlabs.org/viewtopic.php?pid=36119#p36119

Before merging this into deuterium we need to be reasonably sure the remaining issues will be fixed in time for the deuterium merge into master - probably in another two or three weeks, I'm guessing.

Of course! The two issues I pointed out above are just to draw extra eyes on them; I don't think they are inherently unsolvable. My thoughts on them, are:

1. Regarding negative window coordinates: set negative coordinates to 0 prior to adding them to the WINDOWS dictionary. This means that the window will not be restored to its original position, but it's better than crashing. It's a workaround and not a solution, but the alternative is to debug python-xlib, fix the issue (which also means making sure that our fix does not break everyone else's applications -- tough) and get it upstream, after which it will take a while before it is available in Debian Jessie.
2. Regarding copying in code from pyewmh: the alternative would be for @johnraff or @2ion to package it and add it to the BunsenLabs repositories. I'll leave this decision up to you.

---

In two days, I'll have access again to my second monitor to test multi monitor setups myself. That will probably improve time it takes for me to fix any issues that pop up.