aerosnap limping on deprecated syntax

[junkmechanic]

So got around to incorporating aerosnap in my setup and noticed this problem. The lint indicated that the syntax in lines 115 and 123 is deprecated. The funny thing is that the conditional works because of the deprecated syntax. Consider,

elif sys.argv[1] == "--left":
    if is_root_window != True:       <===
        if window_lookup():
            window_restore(width)

is_root_window is actually a function and should have been called. Rather the value of the reference (which is the function object) is being tested against True which would always result in the conditional being true (and the program would enter the subsequent loop) even when the window is the root window. Not that it would bring about any drastic change (like changing the geometry of the root window), but I thought I would mention it for the sake of correctness.

I have prepared the following patch. The rest of the edits are just lint-based. No changes in the logic.

diff --git a/bl-aerosnap b/bl-aerosnap
index c77859d..b05fa3f 100755
--- a/bl-aerosnap
+++ b/bl-aerosnap
@@ -7,13 +7,15 @@
 # ----------------------------------------------------------------------

 from subprocess import Popen, PIPE, STDOUT
-import sys, time, os, re
+import sys
+import os
+import re

 history = '/tmp/bl-aerosnap-'+str(os.getuid())
 windows = {}
 check_intervall = 0.2

-p = Popen(['xdotool','getdisplaygeometry'], stdout=PIPE, stderr=STDOUT)
+p = Popen(['xdotool', 'getdisplaygeometry'], stdout=PIPE, stderr=STDOUT)
 Dimensions = p.communicate()
 Dimensions = Dimensions[0].replace('\n', '')
 Dimensions = Dimensions.split(' ')
@@ -22,12 +24,12 @@ height = int(Dimensions[1])
 hw = width / 2
 rt = width - 1
 bt = height - 1
-aeroLcommand="wmctrl -r :ACTIVE: -b add,maximized_vert && wmctrl -r :ACTIVE: -b remove,maximized_horz && wmctrl -r :ACTIVE: -e 0,0,0,"+str(hw)+",-1"
-aeroRcommand="wmctrl -r :ACTIVE: -b add,maximized_vert && wmctrl -r :ACTIVE: -b remove,maximized_horz && wmctrl -r :ACTIVE: -e 0,"+str(hw)+",0,"+str(hw)+",-1"
+aeroLcommand = "wmctrl -r :ACTIVE: -b add,maximized_vert && wmctrl -r :ACTIVE: -b remove,maximized_horz && wmctrl -r :ACTIVE: -e 0,0,0," + str(hw) + ",-1"
+aeroRcommand = "wmctrl -r :ACTIVE: -b add,maximized_vert && wmctrl -r :ACTIVE: -b remove,maximized_horz && wmctrl -r :ACTIVE: -e 0," + str(hw) + ",0," + str(hw) + ",-1"

-if os.path.exists(history) == False:
-    f = open(history,'w')
+if not os.path.exists(history):
+    f = open(history, 'w')
     f.close()

 def print_usage():
@@ -56,7 +58,7 @@ def window_id():
 def window_lookup():
     ID = window_id()
     windows = history_load()
-    if windows.has_key(ID):
+    if ID in windows:
         return True

 def window_geometry(ID):
@@ -73,8 +75,8 @@ def window_geometry(ID):
 def window_store():
     ID = window_id()
     windows[ID] = window_geometry(ID)
-    s = ID +'|'+window_geometry(ID)+'\n'
-    f = open(history,'a')
+    s = ID + '|' + window_geometry(ID) + '\n'
+    f = open(history, 'a')
     f.write(s)
     f.close()

@@ -92,14 +94,13 @@ def window_restore(width):
         for key in windows:
             h = windows[key].split('|')
             o = key+'|'+h[0]+'|'+h[1]+'|'+h[2]+'|'+h[3]+'\n'
-    f = open(history,'w')
+    f = open(history, 'w')
     f.write(o)
     f.close()
     os.system(command)

 def history_load():
-    f = open(history,'r')
-    i = 0
+    f = open(history, 'r')
     for line in f:
         h = line.split('|')
         h[4] = h[4].replace('\n', '')
@@ -111,7 +112,7 @@ if len(sys.argv) < 2 or sys.argv[1] == "--help":
     print_usage()

 elif sys.argv[1] == "--left":
-    if is_root_window != True:
+    if not is_root_window():
         if window_lookup():
             window_restore(width)
         else:
@@ -119,7 +120,7 @@ elif sys.argv[1] == "--left":
             os.system(aeroLcommand)

 elif sys.argv[1] == "--right":
-    if is_root_window != True:
+    if not is_root_window():
         if window_lookup():
             window_restore(width)
         else:

[Hjdskes]

Thanks for your contribution and my apologies for the delay in a response! This is now fixed in my fork. Pylint now reports:

W: 14, 0: TODO: (fixme)
W: 25, 0: FIXME: replace with an actual enum in python3 (fixme)
W:155, 0: TODO: might have to unmaximize_vert window first (fixme)
W:156, 0: FIXME from corenominal: adjust horizontal placement, not sure where this discrepancy comes from? (fixme)
W:181, 0: FIXME: replace with switch in python 3 (fixme)
W:194, 0: FIXME: geom.x and geom.y are always 10 and 34 (fixme)
C: 17, 0: Line too long (103/100) (line-too-long)
C:166, 0: Unnecessary parens after 'print' keyword (superfluous-parens)
C:236, 0: Unnecessary parens after 'print' keyword (superfluous-parens)
C:  1, 0: Invalid module name "bl-aerosnap" (invalid-name)
R: 26, 0: Too few public methods (0/2) (too-few-public-methods)
C: 83, 4: Invalid argument name "x" (invalid-name)
C: 83, 4: Invalid argument name "y" (invalid-name)
R: 83, 4: Too many arguments (7/5) (too-many-arguments)
W:144, 8: Using the global statement (global-statement)
W:145,18: Use of eval (eval-used)
C:210, 0: Missing function docstring (missing-docstring)
C:235, 0: Missing function docstring (missing-docstring)

...

Global evaluation
-----------------
Your code has been rated at 8.39/10 (previous run: 8.78/10, -0.39)

The -0.39 comes from a big TODO comment I just added to remind myself what needs to be done still.

[junkmechanic]

@Unia Looks great, your fork. Do you intend on merging your commit with this repo at some point or should i follow yours separately?

[Hjdskes]

I intend to make a PR when all issues are resolved :)

[johnraff]

@Hjdskes The code in the deuterium branch looks to me basically unchanged. Is your fork ready to merge?

[Hjdskes]

No, but I do not have time to continue development right now.

[johnraff]

OK no problem.

[johnraff]

For lack of interest, bl-aerosnap has been removed from bunsen-utilities.