Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aerosnap limping on deprecated syntax #16

Closed
junkmechanic opened this issue Jul 6, 2015 · 7 comments
Closed

aerosnap limping on deprecated syntax #16

junkmechanic opened this issue Jul 6, 2015 · 7 comments
Labels

Comments

@junkmechanic
Copy link

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
Copy link
Member

Hjdskes commented Aug 29, 2015

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.

@Hjdskes Hjdskes closed this as completed Aug 29, 2015
@junkmechanic
Copy link
Author

@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
Copy link
Member

Hjdskes commented Aug 30, 2015

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

@johnraff johnraff reopened this Oct 14, 2016
@johnraff
Copy link
Member

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

@Hjdskes
Copy link
Member

Hjdskes commented Oct 14, 2016

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

@johnraff
Copy link
Member

OK no problem.

@johnraff
Copy link
Member

johnraff commented Oct 4, 2019

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

@johnraff johnraff closed this as completed Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants