feat: switched to using click for the CLI, fixed bugs

Author: ErikBjare

aw_qt/config.py

@@ -4,6 +4,7 @@
 from aw_core.config import load_config
 import json
 
+# NOTE: Updating this won't update the defaults for users, this is an issue with how aw_core.config works
 default_settings = {
     "autostart_modules": json.dumps(
         ["aw-server", "aw-watcher-afk", "aw-watcher-window",]

aw_qt/main.py

@@ -1,41 +1,40 @@
 import sys
 import logging
 import argparse
-from typing import List
+import click
+from typing import List, Optional
 from typing_extensions import TypedDict
 
 from aw_core.log import setup_logging
 
 from .manager import Manager
 from . import trayicon
+from .config import AwQtSettings
 
 logger = logging.getLogger(__name__)
 
 
-def main() -> None:
-    args = parse_args()
-    setup_logging("aw-qt", testing=args['testing'], verbose=args['testing'], log_file=True)
-
-    _manager = Manager(testing=args['testing'])
-    _manager.autostart(args['autostart_modules'])
-
-    error_code = trayicon.run(_manager, testing=args['testing'])
+@click.command("aw-qt", help="A trayicon and service manager for ActivityWatch")
+@click.option(
+    "--testing", is_flag=True, help="Run the trayicon and services in testing mode"
+)
+@click.option(
+    "--autostart-modules",
+    help="A comma-separated list of modules to autostart, or just `none` to not autostart anything.",
+)
+def main(testing: bool, autostart_modules: Optional[str]) -> None:
+    config = AwQtSettings(testing=testing)
+    _autostart_modules = (
+        [m.strip() for m in autostart_modules.split(",") if m and m.lower() != "none"]
+        if autostart_modules
+        else config.autostart_modules
+    )
+    setup_logging("aw-qt", testing=testing, verbose=testing, log_file=True)
+
+    _manager = Manager(testing=testing)
+    _manager.autostart(_autostart_modules)
+
+    error_code = trayicon.run(_manager, testing=testing)
     _manager.stop_all()
 
     sys.exit(error_code)
-
-
-CommandLineArgs = TypedDict('CommandLineArgs', {'testing': bool, 'autostart_modules': List[str]}, total=False)
-
-
-def parse_args() -> CommandLineArgs:
-    parser = argparse.ArgumentParser(prog="aw-qt", description='A trayicon and service manager for ActivityWatch')
-    parser.add_argument('--testing', action='store_true',
-                        help='Run the trayicon and services in testing mode')
-    parser.add_argument('--autostart-modules', dest='autostart_modules',
-                        type=lambda s: [m for m in s.split(',') if m and m.lower() != "none"],
-                        default=None,
-                        help='A comma-separated list of modules to autostart, or just `none` to not autostart anything')
-    parsed_args = parser.parse_args()
-    dict: CommandLineArgs = {'autostart_modules': parsed_args.autostart_modules, 'testing': parsed_args.testing}
-    return dict

aw_qt/manager.py

@@ -5,7 +5,7 @@
 import logging
 import subprocess
 import shutil
-from typing import Optional, List, Dict, Set
+from typing import Optional, List, Dict, Set, Tuple
 
 import aw_core
 
@@ -35,7 +35,7 @@ def _is_system_module(name: str) -> bool:
     return shutil.which(name) is not None
 
 
-def _locate_executable(name: str) -> Optional[str]:
+def _locate_executable(name: str) -> Tuple[Optional[str], Optional[str]]:
     """
     Will return the path to the executable if bundled,
     otherwise returns the name if it is available in PATH.
@@ -44,41 +44,38 @@ def _locate_executable(name: str) -> Optional[str]:
     """
     exec_path = _locate_bundled_executable(name)
     if exec_path is not None:  # Check if it exists in bundle
-        return exec_path
+        return exec_path, "bundle"
     elif _is_system_module(name):  # Check if it's in PATH
-        return name
+        return name, "system"
     else:
         logger.warning(
             "Could not find module '{}' in installation directory or PATH".format(name)
         )
-        return None
+        return None, None
 
 
-def _discover_modules_in_directory(modules: List[str], search_path: str) -> None:
+def _discover_modules_in_directory(search_path: str) -> List[str]:
     """Look for modules in given directory path and recursively in subdirs matching aw-*"""
+    modules = []
     matches = glob(os.path.join(search_path, "aw-*"))
     for match in matches:
         if os.path.isfile(match) and os.access(match, os.X_OK):
             name = os.path.basename(match)
             modules.append(name)
         elif os.path.isdir(match) and os.access(match, os.X_OK):
-            _discover_modules_in_directory(modules, match)
+            modules.extend(_discover_modules_in_directory(match))
         else:
             logger.warning(
                 "Found matching file but was not executable: {}".format(match)
             )
+    return modules
 
 
 def _discover_modules_bundled() -> List[str]:
     """Use ``_discover_modules_in_directory`` to find all bundled modules """
-    modules: List[str] = []
     cwd = os.getcwd()
-    _discover_modules_in_directory(modules, cwd)
-
-    if len(modules) > 0:
-        logger.info("Found bundled modules: {}".format(set(modules)))
-    else:
-        logger.info("Found no bundles modules")
+    modules = _discover_modules_in_directory(cwd)
+    logger.info("Found bundled modules: {}".format(set(modules)))
     return modules
 
 
@@ -90,7 +87,7 @@ def _discover_modules_system() -> List[str]:
         if os.path.isdir(path):
             files = os.listdir(path)
             for filename in files:
-                if "aw-" in filename:
+                if filename.startswith("aw-"):
                     modules.append(filename)
 
     logger.info("Found system modules: {}".format(set(modules)))
@@ -116,9 +113,10 @@ def start(self) -> None:
         if platform.system() != "Windows":
             os.setpgrp()
 
-        exec_path = _locate_executable(self.name)
+        exec_path, location = _locate_executable(self.name)
         if exec_path is None:
             logger.error("Tried to start nonexistent module {}".format(self.name))
+            return
         else:
             exec_cmd = [exec_path]
             if self.testing:
@@ -132,7 +130,7 @@ def start(self) -> None:
             startupinfo = subprocess.STARTUPINFO()  # type: ignore
             startupinfo.dwFlags |= subprocess.STARTF_USESHOWWINDOW  # type: ignore
         elif platform.system() == "Darwin":
-            logger.info("Macos: Disable dock icon")
+            logger.info("macOS: Disable dock icon")
             import AppKit
 
             AppKit.NSBundle.mainBundle().infoDictionary()["LSBackgroundOnly"] = "1"
@@ -200,9 +198,7 @@ def read_log(self) -> str:
 
 class Manager:
     def __init__(self, testing: bool = False) -> None:
-        self.settings: AwQtSettings = AwQtSettings(testing)
         self.modules: Dict[str, Module] = {}
-        self.autostart_modules: Set[str] = set(self.settings.autostart_modules)
         self.testing = testing
 
         self.discover_modules()
@@ -211,7 +207,7 @@ def discover_modules(self) -> None:
         # These should always be bundled with aw-qt
         found_modules = set(_discover_modules_bundled())
         found_modules |= set(_discover_modules_system())
-        found_modules ^= {"aw-qt"}  # Exclude self
+        found_modules ^= {"aw-qt", "aw-client"}  # Exclude self
 
         for m_name in found_modules:
             if m_name not in self.modules:
@@ -230,26 +226,26 @@ def start(self, module_name: str) -> None:
                 "Manager tried to start nonexistent module {}".format(module_name)
             )
 
-    def autostart(self, autostart_modules: Optional[List[str]]) -> None:
-        if autostart_modules is None:
-            autostart_modules = []
-        if len(autostart_modules) > 0:
-            logger.info(
-                "Modules to start weren't specified in CLI arguments. Falling back to configuration."
-            )
-            autostart_modules = self.settings.autostart_modules
+    def autostart(self, autostart_modules: List[str]) -> None:
         # We only want to autostart modules that are both in found modules and are asked to autostart.
-        modules_to_start = set(autostart_modules).intersection(set(self.modules.keys()))
+        not_found = []
+        for name in autostart_modules:
+            if name not in self.modules.keys():
+                logger.error(f"Module {name} not found")
+                not_found.append(name)
+        autostart_modules = list(set(autostart_modules) - set(not_found))
 
         # Start aw-server-rust first
-        if "aw-server-rust" in modules_to_start:
+        if "aw-server-rust" in autostart_modules:
             self.start("aw-server-rust")
-        elif "aw-server" in modules_to_start:
+        elif "aw-server" in autostart_modules:
             self.start("aw-server")
 
-        modules_to_start = set(autostart_modules) - {"aw-server", "aw-server-rust"}
-        for module_name in modules_to_start:
-            self.start(module_name)
+        autostart_modules = list(
+            set(autostart_modules) - {"aw-server", "aw-server-rust"}
+        )
+        for name in autostart_modules:
+            self.start(name)
 
     def stop_all(self) -> None:
         for module in filter(lambda m: m.is_alive(), self.modules.values()):

poetry.lock

@@ -18,6 +18,7 @@ PyQt5 = "5.10.1"
 sip = "4.19.8"
 pyobjc = { version = "^6.1", platform = "darwin" }
 aw-core = {git = "https://github.com/ActivityWatch/aw-core.git"}
+click = "^7.1.2"
 
 [tool.poetry.dev-dependencies]
 mypy = "^0.761"