python: plugin cleanup and refactor

3 files changed, 143 insertions, 73 deletions

diff --git a/plugins/python/_hexchat.py b/plugins/python/_hexchat.py
index 50ccfb83..ebee5657 100644
--- a/plugins/python/_hexchat.py
+++ b/plugins/python/_hexchat.py
@@ -1,6 +1,7 @@
-from contextlib import contextmanager
 import inspect
 import sys
+from contextlib import contextmanager
+
 from _hexchat_embedded import ffi, lib
 
 __all__ = [
@@ -40,13 +41,15 @@ def __get_current_plugin():
     while '__plugin' not in frame.f_globals:
         frame = frame.f_back
         assert frame is not None
+
     return frame.f_globals['__plugin']
 
 
 # Keeping API compat
-if sys.version_info[0] is 2:
+if sys.version_info[0] == 2:
     def __decode(string):
         return string
+
 else:
     def __decode(string):
         return string.decode()
@@ -64,16 +67,18 @@ def emit_print(event_name, *args, **kwargs):
         arg = args[i].encode() if len(args) > i else b''
         cstring = ffi.new('char[]', arg)
         cargs.append(cstring)
-    if time is 0:
+
+    if time == 0:
         return lib.hexchat_emit_print(lib.ph, event_name.encode(), *cargs)
-    else:
-        attrs = lib.hexchat_event_attrs_create(lib.ph)
-        attrs.server_time_utc = time
-        ret = lib.hexchat_emit_print(lib.ph, attrs, event_name.encode(), *cargs)
-        lib.hexchat_event_attrs_free(lib.ph, attrs)
-        return ret
+
+    attrs = lib.hexchat_event_attrs_create(lib.ph)
+    attrs.server_time_utc = time
+    ret = lib.hexchat_emit_print(lib.ph, attrs, event_name.encode(), *cargs)
+    lib.hexchat_event_attrs_free(lib.ph, attrs)
+    return ret
 
 
+# TODO: this shadows itself. command should be changed to cmd
 def command(command):
     lib.hexchat_command(lib.ph, command.encode())
 
@@ -97,21 +102,24 @@ def get_info(name):
         # Surely there is a less dumb way?
         ptr = repr(ret).rsplit(' ', 1)[1][:-1]
         return ptr
+
     return __decode(ffi.string(ret))
 
 
 def get_prefs(name):
     string_out = ffi.new('char**')
     int_out = ffi.new('int*')
-    type = lib.hexchat_get_prefs(lib.ph, name.encode(), string_out, int_out)
-    if type is 0:
+    _type = lib.hexchat_get_prefs(lib.ph, name.encode(), string_out, int_out)
+    if _type == 0:
         return None
-    elif type is 1:
+
+    if _type == 1:
         return __decode(ffi.string(string_out[0]))
-    elif type is 2 or type is 3:  # XXX: 3 should be a bool, but keeps API
+
+    if _type in (2, 3):  # XXX: 3 should be a bool, but keeps API
         return int_out[0]
-    else:
-        assert False
+
+    raise AssertionError('Out of bounds pref storage')
 
 
 def __cstrarray_to_list(arr):
@@ -120,6 +128,7 @@ def __cstrarray_to_list(arr):
     while arr[i] != ffi.NULL:
         ret.append(ffi.string(arr[i]))
         i += 1
+
     return ret
 
 
@@ -127,8 +136,7 @@ __FIELD_CACHE = {}
 
 
 def __get_fields(name):
-    return __FIELD_CACHE.setdefault(name,
-                                    __cstrarray_to_list(lib.hexchat_list_fields(lib.ph, name)))
+    return __FIELD_CACHE.setdefault(name, __cstrarray_to_list(lib.hexchat_list_fields(lib.ph, name)))
 
 
 __FIELD_PROPERTY_CACHE = {}
@@ -154,6 +162,7 @@ class ListItem:
 if sys.version_info[0] == 2:
     def get_getter(name):
         return ord(name[0])
+
 else:
     def get_getter(name):
         return name[0]
@@ -179,6 +188,7 @@ def get_list(name):
         string = lib.hexchat_list_str(lib.ph, list_, field)
         if string != ffi.NULL:
             return __decode(ffi.string(string))
+
         return ''
 
     def ptr_getter(field):
@@ -186,6 +196,7 @@ def get_list(name):
             ptr = lib.hexchat_list_str(lib.ph, list_, field)
             ctx = ffi.cast('hexchat_context*', ptr)
             return Context(ctx)
+
         return None
 
     getters = {
@@ -195,25 +206,27 @@ def get_list(name):
         ord('p'): ptr_getter,
     }
 
-    while lib.hexchat_list_next(lib.ph, list_) is 1:
+    while lib.hexchat_list_next(lib.ph, list_) == 1:
         item = ListItem(orig_name)
-        for field in fields:
-            getter = getters.get(get_getter(field))
+        for _field in fields:
+            getter = getters.get(get_getter(_field))
             if getter is not None:
-                field_name = field[1:]
+                field_name = _field[1:]
                 setattr(item, __cached_decoded_str(field_name), getter(field_name))
+
         ret.append(item)
 
     lib.hexchat_list_free(lib.ph, list_)
     return ret
 
 
+# TODO: 'command' here shadows command above, and should be renamed to cmd
 def hook_command(command, callback, userdata=None, priority=PRI_NORM, help=None):
     plugin = __get_current_plugin()
     hook = plugin.add_hook(callback, userdata)
     handle = lib.hexchat_hook_command(lib.ph, command.encode(), priority, lib._on_command_hook,
-                                      help.encode() if help is not None else ffi.NULL,
-                                      hook.handle)
+                                      help.encode() if help is not None else ffi.NULL, hook.handle)
+
     hook.hexchat_hook = handle
     return id(hook)
 
@@ -221,8 +234,7 @@ def hook_command(command, callback, userdata=None, priority=PRI_NORM, help=None)
 def hook_print(name, callback, userdata=None, priority=PRI_NORM):
     plugin = __get_current_plugin()
     hook = plugin.add_hook(callback, userdata)
-    handle = lib.hexchat_hook_print(lib.ph, name.encode(), priority, lib._on_print_hook,
-                                    hook.handle)
+    handle = lib.hexchat_hook_print(lib.ph, name.encode(), priority, lib._on_print_hook, hook.handle)
     hook.hexchat_hook = handle
     return id(hook)
 
@@ -230,8 +242,7 @@ def hook_print(name, callback, userdata=None, priority=PRI_NORM):
 def hook_print_attrs(name, callback, userdata=None, priority=PRI_NORM):
     plugin = __get_current_plugin()
     hook = plugin.add_hook(callback, userdata)
-    handle = lib.hexchat_hook_print_attrs(lib.ph, name.encode(), priority, lib._on_print_attrs_hook,
-                                          hook.handle)
+    handle = lib.hexchat_hook_print_attrs(lib.ph, name.encode(), priority, lib._on_print_attrs_hook, hook.handle)
     hook.hexchat_hook = handle
     return id(hook)
 
@@ -239,8 +250,7 @@ def hook_print_attrs(name, callback, userdata=None, priority=PRI_NORM):
 def hook_server(name, callback, userdata=None, priority=PRI_NORM):
     plugin = __get_current_plugin()
     hook = plugin.add_hook(callback, userdata)
-    handle = lib.hexchat_hook_server(lib.ph, name.encode(), priority, lib._on_server_hook,
-                                     hook.handle)
+    handle = lib.hexchat_hook_server(lib.ph, name.encode(), priority, lib._on_server_hook, hook.handle)
     hook.hexchat_hook = handle
     return id(hook)
 
@@ -248,8 +258,7 @@ def hook_server(name, callback, userdata=None, priority=PRI_NORM):
 def hook_server_attrs(name, callback, userdata=None, priority=PRI_NORM):
     plugin = __get_current_plugin()
     hook = plugin.add_hook(callback, userdata)
-    handle = lib.hexchat_hook_server_attrs(lib.ph, name.encode(), priority, lib._on_server_attrs_hook,
-                                           hook.handle)
+    handle = lib.hexchat_hook_server_attrs(lib.ph, name.encode(), priority, lib._on_server_attrs_hook, hook.handle)
     hook.hexchat_hook = handle
     return id(hook)
 
@@ -276,17 +285,18 @@ def unhook(handle):
 def set_pluginpref(name, value):
     if isinstance(value, str):
         return bool(lib.hexchat_pluginpref_set_str(lib.ph, name.encode(), value.encode()))
-    elif isinstance(value, int):
+
+    if isinstance(value, int):
         return bool(lib.hexchat_pluginpref_set_int(lib.ph, name.encode(), value))
-    else:
-        # XXX: This should probably raise but this keeps API
-        return False
+
+    # XXX: This should probably raise but this keeps API
+    return False
 
 
 def get_pluginpref(name):
     name = name.encode()
     string_out = ffi.new('char[512]')
-    if lib.hexchat_pluginpref_get_str(lib.ph, name, string_out) is not 1:
+    if lib.hexchat_pluginpref_get_str(lib.ph, name, string_out) != 1:
         return None
 
     string = ffi.string(string_out)
@@ -308,8 +318,9 @@ def del_pluginpref(name):
 
 def list_pluginpref():
     prefs_str = ffi.new('char[4096]')
-    if lib.hexchat_pluginpref_list(lib.ph, prefs_str) is 1:
+    if lib.hexchat_pluginpref_list(lib.ph, prefs_str) == 1:
         return __decode(prefs_str).split(',')
+
     return []
 
 
@@ -320,6 +331,7 @@ class Context:
     def __eq__(self, value):
         if not isinstance(value, Context):
             return False
+
         return self._ctx == value._ctx
 
     @contextmanager
@@ -327,9 +339,9 @@ class Context:
         old_ctx = lib.hexchat_get_context(lib.ph)
         if not self.set():
             # XXX: Behavior change, previously used wrong context
-            lib.hexchat_print(lib.ph,
-                              b'Context object refers to closed context, ignoring call')
+            lib.hexchat_print(lib.ph, b'Context object refers to closed context, ignoring call')
             return
+
         yield
         lib.hexchat_set_context(lib.ph, old_ctx)
 
@@ -370,4 +382,5 @@ def find_context(server=None, channel=None):
     ctx = lib.hexchat_find_context(lib.ph, server, channel)
     if ctx == ffi.NULL:
         return None
+
     return Context(ctx)
diff --git a/plugins/python/python.py b/plugins/python/python.py
index 1afb36c4..30694802 100644
--- a/plugins/python/python.py
+++ b/plugins/python/python.py
@@ -1,30 +1,30 @@
 from __future__ import print_function
 
+import importlib
 import os
 import pydoc
-import sys
-from contextlib import contextmanager
-import importlib
 import signal
-import site
+import sys
 import traceback
 import weakref
+from contextlib import contextmanager
+
+from _hexchat_embedded import ffi, lib
 
 if sys.version_info < (3, 0):
     from io import BytesIO as HelpEater
 else:
     from io import StringIO as HelpEater
 
-from _hexchat_embedded import ffi, lib
-
 if not hasattr(sys, 'argv'):
     sys.argv = ['<hexchat>']
 
 VERSION = b'2.0'  # Sync with hexchat.__version__
 PLUGIN_NAME = ffi.new('char[]', b'Python')
-PLUGIN_DESC = ffi.new('char[]', b'Python %d.%d scripting interface'
-                      % (sys.version_info[0], sys.version_info[1]))
+PLUGIN_DESC = ffi.new('char[]', b'Python %d.%d scripting interface' % (sys.version_info[0], sys.version_info[1]))
 PLUGIN_VERSION = ffi.new('char[]', VERSION)
+
+# TODO: Constants should be screaming snake case
 hexchat = None
 local_interp = None
 hexchat_stdout = None
@@ -40,10 +40,11 @@ def redirected_stdout():
     sys.stderr = hexchat_stdout
 
 
-if os.environ.get('HEXCHAT_LOG_PYTHON'):
+if os.getenv('HEXCHAT_LOG_PYTHON'):
     def log(*args):
         with redirected_stdout():
             print(*args)
+
 else:
     def log(*args):
         pass
@@ -56,7 +57,7 @@ class Stdout:
     def write(self, string):
         string = string.encode()
         idx = string.rfind(b'\n')
-        if idx is not -1:
+        if idx != -1:
             self.buffer += string[:idx]
             lib.hexchat_print(lib.ph, bytes(self.buffer))
             self.buffer = bytearray(string[idx + 1:])
@@ -91,13 +92,15 @@ class Hook:
             lib.hexchat_unhook(lib.ph, self.hexchat_hook)
 
 
-if sys.version_info[0] is 2:
+if sys.version_info[0] == 2:
     def compile_file(data, filename):
         return compile(data, filename, 'exec', dont_inherit=True)
 
+
     def compile_line(string):
         try:
             return compile(string, '<string>', 'eval', dont_inherit=True)
+
         except SyntaxError:
             # For some reason `print` is invalid for eval
             # This will hide any return value though
@@ -106,6 +109,7 @@ else:
     def compile_file(data, filename):
         return compile(data, filename, 'exec', optimize=2, dont_inherit=True)
 
+
     def compile_line(string):
         # newline appended to solve unexpected EOF issues
         return compile(string + '\n', '<string>', 'single', optimize=2, dont_inherit=True)
@@ -135,8 +139,9 @@ class Plugin:
                 ud = h.userdata
                 self.hooks.remove(h)
                 return ud
-        else:
-            log('Hook not found')
+
+        log('Hook not found')
+        return None
 
     def loadfile(self, filename):
         try:
@@ -148,21 +153,22 @@ class Plugin:
 
             try:
                 self.name = self.globals['__module_name__']
+
             except KeyError:
                 lib.hexchat_print(lib.ph, b'Failed to load module: __module_name__ must be set')
+
                 return False
 
             self.version = self.globals.get('__module_version__', '')
             self.description = self.globals.get('__module_description__', '')
-            self.ph = lib.hexchat_plugingui_add(lib.ph, filename.encode(),
-                                                self.name.encode(),
-                                                self.description.encode(),
-                                                self.version.encode(),
-                                                ffi.NULL)
+            self.ph = lib.hexchat_plugingui_add(lib.ph, filename.encode(), self.name.encode(),
+                                                self.description.encode(), self.version.encode(), ffi.NULL)
+
         except Exception as e:
             lib.hexchat_print(lib.ph, 'Failed to load module: {}'.format(e).encode())
             traceback.print_exc()
             return False
+
         return True
 
     def __del__(self):
@@ -171,17 +177,20 @@ class Plugin:
             if hook.is_unload is True:
                 try:
                     hook.callback(hook.userdata)
+
                 except Exception as e:
                     log('Failed to run hook:', e)
                     traceback.print_exc()
+
         del self.hooks
         if self.ph is not None:
             lib.hexchat_plugingui_remove(lib.ph, self.ph)
 
 
-if sys.version_info[0] is 2:
+if sys.version_info[0] == 2:
     def __decode(string):
         return string
+
 else:
     def __decode(string):
         return string.decode()
@@ -192,6 +201,7 @@ def wordlist_len(words):
     for i in range(31, 1, -1):
         if ffi.string(words[i]):
             return i
+
     return 0
 
 
@@ -205,24 +215,26 @@ def create_wordlist(words):
 # This makes no sense to do...
 def create_wordeollist(words):
     words = reversed(words)
-    last = None
     accum = None
     ret = []
     for word in words:
         if accum is None:
             accum = word
+
         elif word:
             last = accum
             accum = ' '.join((word, last))
+
         ret.insert(0, accum)
+
     return ret
 
 
 def to_cb_ret(value):
     if value is None:
         return 0
-    else:
-        return int(value)
+
+    return int(value)
 
 
 @ffi.def_extern()
@@ -274,13 +286,14 @@ def _on_timer_hook(userdata):
     hook = ffi.from_handle(userdata)
     if hook.callback(hook.userdata) is True:
         return 1
-    else:
-        hook.is_unload = True  # Don't unhook
-        for h in hook.plugin.hooks:
-            if h == hook:
-                hook.plugin.hooks.remove(h)
-                break
-        return 0
+
+    hook.is_unload = True  # Don't unhook
+    for h in hook.plugin.hooks:
+        if h == hook:
+            hook.plugin.hooks.remove(h)
+            break
+
+    return 0
 
 
 @ffi.def_extern(error=3)
@@ -291,6 +304,7 @@ def _on_say_command(word, word_eol, userdata):
         lib.hexchat_print(lib.ph, b'>>> ' + python)
         exec_in_interp(__decode(python))
         return 1
+
     return 0
 
 
@@ -298,33 +312,36 @@ def load_filename(filename):
     filename = os.path.expanduser(filename)
     if not os.path.isabs(filename):
         configdir = __decode(ffi.string(lib.hexchat_get_info(lib.ph, b'configdir')))
+
         filename = os.path.join(configdir, 'addons', filename)
+
     if filename and not any(plugin.filename == filename for plugin in plugins):
         plugin = Plugin()
         if plugin.loadfile(filename):
             plugins.add(plugin)
             return True
+
     return False
 
 
 def unload_name(name):
     if name:
         for plugin in plugins:
-            if name in (plugin.name, plugin.filename,
-                        os.path.basename(plugin.filename)):
+            if name in (plugin.name, plugin.filename, os.path.basename(plugin.filename)):
                 plugins.remove(plugin)
                 return True
+
     return False
 
 
 def reload_name(name):
     if name:
         for plugin in plugins:
-            if name in (plugin.name, plugin.filename,
-                        os.path.basename(plugin.filename)):
+            if name in (plugin.name, plugin.filename, os.path.basename(plugin.filename)):
                 filename = plugin.filename
                 plugins.remove(plugin)
                 return load_filename(filename)
+
     return False
 
 
@@ -346,6 +363,7 @@ def autoload():
                     log('Autoloading', f)
                     # TODO: Set cwd
                     load_filename(os.path.join(addondir, f))
+
     except FileNotFoundError as e:
         log('Autoload failed', e)
 
@@ -376,7 +394,6 @@ def list_plugins():
     for row in tbl:
         lib.hexchat_print(lib.ph, b' '.join(item.ljust(column_sizes[i])
                                             for i, item in enumerate(row)))
-
     lib.hexchat_print(lib.ph, b'')
 
 
@@ -396,6 +413,7 @@ def exec_in_interp(python):
         ret = eval(code, local_interp.globals, local_interp.locals)
         if ret is not None:
             lib.hexchat_print(lib.ph, '{}'.format(ret).encode())
+
     except Exception as e:
         traceback.print_exc(file=hexchat_stdout)
 
@@ -406,6 +424,7 @@ def _on_load_command(word, word_eol, userdata):
     if filename.endswith(b'.py'):
         load_filename(__decode(filename))
         return 3
+
     return 0
 
 
@@ -415,6 +434,7 @@ def _on_unload_command(word, word_eol, userdata):
     if filename.endswith(b'.py'):
         unload_name(__decode(filename))
         return 3
+
     return 0
 
 
@@ -424,6 +444,7 @@ def _on_reload_command(word, word_eol, userdata):
     if filename.endswith(b'.py'):
         reload_name(__decode(filename))
         return 3
+
     return 0
 
 
@@ -434,23 +455,30 @@ def _on_py_command(word, word_eol, userdata):
     if subcmd == 'exec':
         python = __decode(ffi.string(word_eol[3]))
         exec_in_interp(python)
+
     elif subcmd == 'load':
         filename = __decode(ffi.string(word[3]))
         load_filename(filename)
+
     elif subcmd == 'unload':
         name = __decode(ffi.string(word[3]))
         if not unload_name(name):
             lib.hexchat_print(lib.ph, b'Can\'t find a python plugin with that name')
+
     elif subcmd == 'reload':
         name = __decode(ffi.string(word[3]))
         if not reload_name(name):
             lib.hexchat_print(lib.ph, b'Can\'t find a python plugin with that name')
+
     elif subcmd == 'console':
         lib.hexchat_command(lib.ph, b'QUERY >>python<<')
+
     elif subcmd == 'list':
         list_plugins()
+
     elif subcmd == 'about':
         lib.hexchat_print(lib.ph, b'HexChat Python interface version ' + VERSION)
+
     else:
         lib.hexchat_command(lib.ph, b'HELP PY')
 
@@ -473,8 +501,10 @@ def _on_plugin_init(plugin_name, plugin_desc, plugin_version, arg, libdir):
         modpath = os.path.join(libdir, '..', 'python')
         sys.path.append(os.path.abspath(modpath))
         hexchat = importlib.import_module('hexchat')
+
     except (UnicodeDecodeError, ImportError) as e:
         lib.hexchat_print(lib.ph, b'Failed to import module: ' + repr(e).encode())
+
         return 0
 
     hexchat_stdout = Stdout()
@@ -517,6 +547,7 @@ def _on_plugin_deinit():
     for mod in ('_hexchat', 'hexchat', 'xchat', '_hexchat_embedded'):
         try:
             del sys.modules[mod]
+
         except KeyError:
             pass
 
diff --git a/plugins/python/python_style_guide.md b/plugins/python/python_style_guide.md
new file mode 100644
index 00000000..41db2474
--- /dev/null
+++ b/plugins/python/python_style_guide.md
@@ -0,0 +1,26 @@
+# HexChat Python Module Style Guide
+
+(This is a work in progress).
+
+## General rules
+
+- PEP8 as general fallback recommendations
+- Max line length: 120
+- Avoid overcomplex compound statements. i.e. dont do this: `somevar = x if x == y else z if a == b and c == b else x`
+
+## Indentation style
+
+### Multi-line functions
+
+```python
+foo(really_long_arg_1,
+    really_long_arg_2)
+```
+
+### Mutli-line lists/dicts
+
+```python
+foo = {
+    'bar': 'baz',
+}
+```