Configuration system for CLI (#6708)

* Rework how bin/qmk handles subcommands

* qmk config wip

* Code to show all configs

* Fully working `qmk config` command

* Mark some CLI arguments so they don't pollute the config file

* Fleshed out config support, nicer subcommand support

* sync with installable cli

* pyformat

* Add a test for subcommand_modules

* Documentation for the `qmk config` command

* split config_token on space so qmk config is more predictable

* Rework how subcommands are imported

* Document `arg_only`

* Document deleting from CLI

* Document how multiple operations work

* Add cli config to the doc index

* Add tests for the cli commands

* Make running the tests more reliable

* Be more selective about building all default keymaps

* Update new-keymap to fit the new subcommand style

* Add documentation about writing CLI scripts

* Document new-keyboard

* Update docs/cli_configuration.md

Co-Authored-By: noroadsleft <18669334+noroadsleft@users.noreply.github.com>

* Update docs/cli_development.md

Co-Authored-By: noroadsleft <18669334+noroadsleft@users.noreply.github.com>

* Update docs/cli_development.md

Co-Authored-By: noroadsleft <18669334+noroadsleft@users.noreply.github.com>

* Update docs/cli_development.md

Co-Authored-By: noroadsleft <18669334+noroadsleft@users.noreply.github.com>

* Address yan's comments.

* Apply suggestions from code review

suggestions from @noahfrederick

Co-Authored-By: Noah Frederick <code@noahfrederick.com>

* Apply suggestions from code review

Co-Authored-By: Noah Frederick <code@noahfrederick.com>

* Remove pip3 from the test runner

bin/qmk

@@ -4,10 +4,8 @@
 import os
 import subprocess
 import sys
-from glob import glob
-from time import strftime
-from importlib import import_module
 from importlib.util import find_spec
+from time import strftime
 
 # Add the QMK python libs to our path
 script_dir = os.path.dirname(os.path.realpath(__file__))
@@ -15,12 +13,8 @@ qmk_dir = os.path.abspath(os.path.join(script_dir, '..'))
 python_lib_dir = os.path.abspath(os.path.join(qmk_dir, 'lib', 'python'))
 sys.path.append(python_lib_dir)
 
-# Change to the root of our checkout
-os.environ['ORIG_CWD'] = os.getcwd()
-os.chdir(qmk_dir)
-
 # Make sure our modules have been setup
-with open('requirements.txt', 'r') as fd:
+with open(os.path.join(qmk_dir, 'requirements.txt'), 'r') as fd:
     for line in fd.readlines():
         line = line.strip().replace('<', '=').replace('>', '=')
 
@@ -32,72 +26,58 @@ with open('requirements.txt', 'r') as fd:
 
         module = line.split('=')[0] if '=' in line else line
         if not find_spec(module):
-            print('Your QMK build environment is not fully setup!\n')
+            print('Could not find module %s!', module)
-            print('Please run `./util/qmk_install.sh` to setup QMK.')
+            print('Please run `pip3 install -r requirements.txt` to install the python dependencies.')
             exit(255)
 
 # Figure out our version
+# TODO(skullydazed/anyone): Find a method that doesn't involve git. This is slow in docker and on windows.
 command = ['git', 'describe', '--abbrev=6', '--dirty', '--always', '--tags']
-result = subprocess.run(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+result = subprocess.run(command, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
 if result.returncode == 0:
-    os.environ['QMK_VERSION'] = 'QMK ' + result.stdout.strip()
+    os.environ['QMK_VERSION'] = result.stdout.strip()
 else:
-    os.environ['QMK_VERSION'] = 'QMK ' + strftime('%Y-%m-%d-%H:%M:%S')
+    os.environ['QMK_VERSION'] = 'nogit-' + strftime('%Y-%m-%d-%H:%M:%S') + '-dirty'
 
 # Setup the CLI
 import milc
-milc.EMOJI_LOGLEVELS['INFO'] = '{fg_blue}ψ{style_reset_all}'
 
-# If we were invoked as `qmk <cmd>` massage sys.argv into `qmk-<cmd>`.
+milc.EMOJI_LOGLEVELS['INFO'] = '{fg_blue}Ψ{style_reset_all}'
-# This means we can't accept arguments to the qmk script itself.
-script_name = os.path.basename(sys.argv[0])
-if script_name == 'qmk':
-    if len(sys.argv) == 1:
-        milc.cli.log.error('No subcommand specified!\n')
 
-    if len(sys.argv) == 1 or sys.argv[1] in ['-h', '--help']:
-        milc.cli.echo('usage: qmk <subcommand> [...]')
-        milc.cli.echo('\nsubcommands:')
-        subcommands = glob(os.path.join(qmk_dir, 'bin', 'qmk-*'))
-        for subcommand in sorted(subcommands):
-            subcommand = os.path.basename(subcommand).split('-', 1)[1]
-            milc.cli.echo('\t%s', subcommand)
-        milc.cli.echo('\nqmk <subcommand> --help for more information')
-        exit(1)
 
-    if sys.argv[1] in ['-V', '--version']:
+@milc.cli.entrypoint('QMK Helper Script')
-        milc.cli.echo(os.environ['QMK_VERSION'])
+def qmk_main(cli):
-        exit(0)
+    """The function that gets run when no subcommand is provided.
+    """
+    cli.print_help()
 
-    sys.argv[0] = script_name = '-'.join((script_name, sys.argv[1]))
-    del sys.argv[1]
 
-# Look for which module to import
+def main():
-if script_name == 'qmk':
+    """Setup our environment and then call the CLI entrypoint.
-    milc.cli.print_help()
+    """
-    exit(0)
+    # Change to the root of our checkout
-elif not script_name.startswith('qmk-'):
+    os.environ['ORIG_CWD'] = os.getcwd()
-    milc.cli.log.error('Invalid symlink, must start with "qmk-": %s', script_name)
+    os.chdir(qmk_dir)
-else:
-    subcommand = script_name.replace('-', '.').replace('_', '.').split('.')
-    subcommand.insert(1, 'cli')
-    subcommand = '.'.join(subcommand)
 
-    try:
+    # Import the subcommands
-        import_module(subcommand)
+    import qmk.cli
-    except ModuleNotFoundError as e:
-        if e.__class__.__name__ != subcommand:
-            raise
 
-        milc.cli.log.error('Invalid subcommand! Could not import %s.', subcommand)
+    # Execute
-        exit(1)
-
-if __name__ == '__main__':
     return_code = milc.cli()
+
     if return_code is False:
         exit(1)
-    elif return_code is not True and isinstance(return_code, int) and return_code < 256:
+
+    elif return_code is not True and isinstance(return_code, int):
+        if return_code < 0 or return_code > 255:
+            milc.cli.log.error('Invalid return_code: %d', return_code)
+            exit(255)
+
         exit(return_code)
-    else:
+
-        exit(0)
+    exit(0)
+
+
+if __name__ == '__main__':
+    main()

bin/qmk-compile-json

@@ -1 +0,0 @@
-qmk

bin/qmk-doctor

@@ -1 +0,0 @@
-qmk

bin/qmk-hello

@@ -1 +0,0 @@
-qmk

bin/qmk-json-keymap

@@ -1 +0,0 @@
-qmk

build_json.mk

@@ -23,5 +23,5 @@ endif
 
 # Generate the keymap.c
 ifneq ("$(KEYMAP_JSON)","")
-    _ = $(shell test -e $(KEYMAP_C) || bin/qmk-json-keymap $(KEYMAP_JSON) -o $(KEYMAP_C))
+    _ = $(shell test -e $(KEYMAP_C) || bin/qmk json-keymap $(KEYMAP_JSON) -o $(KEYMAP_C))
 endif

lib/python/milc.py

@@ -17,6 +17,7 @@ import argparse
 import logging
 import os
 import re
+import shlex
 import sys
 from decimal import Decimal
 from tempfile import NamedTemporaryFile
@@ -35,6 +36,10 @@ except ImportError:
 
 import argcomplete
 import colorama
+from appdirs import user_config_dir
+
+# Disable logging until we can configure it how the user wants
+logging.basicConfig(filename='/dev/null')
 
 # Log Level Representations
 EMOJI_LOGLEVELS = {
@@ -47,6 +52,7 @@ EMOJI_LOGLEVELS = {
 }
 EMOJI_LOGLEVELS['FATAL'] = EMOJI_LOGLEVELS['CRITICAL']
 EMOJI_LOGLEVELS['WARN'] = EMOJI_LOGLEVELS['WARNING']
+UNICODE_SUPPORT = sys.stdout.encoding.lower().startswith('utf')
 
 # ANSI Color setup
 # Regex was gratefully borrowed from kfir on stackoverflow:
@@ -97,11 +103,12 @@ class ANSIFormatter(logging.Formatter):
 
 
 class ANSIEmojiLoglevelFormatter(ANSIFormatter):
-    """A log formatter that makes the loglevel an emoji.
+    """A log formatter that makes the loglevel an emoji on UTF capable terminals.
     """
 
     def format(self, record):
-        record.levelname = EMOJI_LOGLEVELS[record.levelname].format(**ansi_colors)
+        if UNICODE_SUPPORT:
+            record.levelname = EMOJI_LOGLEVELS[record.levelname].format(**ansi_colors)
         return super(ANSIEmojiLoglevelFormatter, self).format(record)
 
 
@@ -144,13 +151,15 @@ class Configuration(object):
 
     def __init__(self, *args, **kwargs):
         self._config = {}
-        self.default_container = ConfigurationOption
+
+    def __getattr__(self, key):
+        return self.__getitem__(key)
 
     def __getitem__(self, key):
         """Returns a config section, creating it if it doesn't exist yet.
         """
         if key not in self._config:
-            self.__dict__[key] = self._config[key] = ConfigurationOption()
+            self.__dict__[key] = self._config[key] = ConfigurationSection(self)
 
         return self._config[key]
 
@@ -161,30 +170,34 @@ class Configuration(object):
     def __delitem__(self, key):
         if key in self.__dict__ and key[0] != '_':
             del self.__dict__[key]
-        del self._config[key]
+        if key in self._config:
+            del self._config[key]
 
 
-class ConfigurationOption(Configuration):
+class ConfigurationSection(Configuration):
-    def __init__(self, *args, **kwargs):
+    def __init__(self, parent, *args, **kwargs):
-        super(ConfigurationOption, self).__init__(*args, **kwargs)
+        super(ConfigurationSection, self).__init__(*args, **kwargs)
-        self.default_container = dict
+        self.parent = parent
 
     def __getitem__(self, key):
-        """Returns a config section, creating it if it doesn't exist yet.
+        """Returns a config value, pulling from the `user` section as a fallback.
         """
-        if key not in self._config:
+        if key in self._config:
-            self.__dict__[key] = self._config[key] = None
+            return self._config[key]
 
-        return self._config[key]
+        elif key in self.parent.user:
+            return self.parent.user[key]
+
+        return None
 
 
 def handle_store_boolean(self, *args, **kwargs):
     """Does the add_argument for action='store_boolean'.
     """
-    kwargs['add_dest'] = False
     disabled_args = None
     disabled_kwargs = kwargs.copy()
     disabled_kwargs['action'] = 'store_false'
+    disabled_kwargs['dest'] = self.get_argument_name(*args, **kwargs)
     disabled_kwargs['help'] = 'Disable ' + kwargs['help']
     kwargs['action'] = 'store_true'
     kwargs['help'] = 'Enable ' + kwargs['help']
@@ -219,11 +232,6 @@ class SubparserWrapper(object):
         self.subparser.completer = completer
 
     def add_argument(self, *args, **kwargs):
-        if kwargs.get('add_dest', True):
-            kwargs['dest'] = self.submodule + '_' + self.cli.get_argument_name(*args, **kwargs)
-        if 'add_dest' in kwargs:
-            del kwargs['add_dest']
-
         if 'action' in kwargs and kwargs['action'] == 'store_boolean':
             return handle_store_boolean(self, *args, **kwargs)
 
@@ -254,12 +262,16 @@ class MILC(object):
         self._entrypoint = None
         self._inside_context_manager = False
         self.ansi = ansi_colors
+        self.arg_only = []
         self.config = Configuration()
         self.config_file = None
-        self.prog_name = sys.argv[0][:-3] if sys.argv[0].endswith('.py') else sys.argv[0]
         self.version = os.environ.get('QMK_VERSION', 'unknown')
         self.release_lock()
 
+        # Figure out our program name
+        self.prog_name = sys.argv[0][:-3] if sys.argv[0].endswith('.py') else sys.argv[0]
+        self.prog_name = self.prog_name.split('/')[-1]
+
         # Initialize all the things
         self.initialize_argparse()
         self.initialize_logging()
@@ -273,7 +285,7 @@ class MILC(object):
         self._description = self._arg_parser.description = self._arg_defaults.description = value
 
     def echo(self, text, *args, **kwargs):
-        """Print colorized text to stdout, as long as stdout is a tty.
+        """Print colorized text to stdout.
 
         ANSI color strings (such as {fg-blue}) will be converted into ANSI
         escape sequences, and the ANSI reset sequence will be added to all
@@ -284,11 +296,10 @@ class MILC(object):
         if args and kwargs:
             raise RuntimeError('You can only specify *args or **kwargs, not both!')
 
-        if sys.stdout.isatty():
+        args = args or kwargs
-            args = args or kwargs
+        text = format_ansi(text)
-            text = format_ansi(text)
 
-            print(text % args)
+        print(text % args)
 
     def initialize_argparse(self):
         """Prepare to process arguments from sys.argv.
@@ -313,21 +324,21 @@ class MILC(object):
         self.release_lock()
 
     def completer(self, completer):
-        """Add an arpcomplete completer to this subcommand.
+        """Add an argcomplete completer to this subcommand.
         """
         self._arg_parser.completer = completer
 
     def add_argument(self, *args, **kwargs):
         """Wrapper to add arguments to both the main and the shadow argparser.
         """
+        if 'action' in kwargs and kwargs['action'] == 'store_boolean':
+            return handle_store_boolean(self, *args, **kwargs)
+
         if kwargs.get('add_dest', True) and args[0][0] == '-':
             kwargs['dest'] = 'general_' + self.get_argument_name(*args, **kwargs)
         if 'add_dest' in kwargs:
             del kwargs['add_dest']
 
-        if 'action' in kwargs and kwargs['action'] == 'store_boolean':
-            return handle_store_boolean(self, *args, **kwargs)
-
         self.acquire_lock()
         self._arg_parser.add_argument(*args, **kwargs)
 
@@ -396,7 +407,7 @@ class MILC(object):
         if self.args and self.args.general_config_file:
             return self.args.general_config_file
 
-        return os.path.abspath(os.path.expanduser('~/.%s.ini' % self.prog_name))
+        return os.path.join(user_config_dir(appname='qmk', appauthor='QMK'), '%s.ini' % self.prog_name)
 
     def get_argument_name(self, *args, **kwargs):
         """Takes argparse arguments and returns the dest name.
@@ -413,6 +424,11 @@ class MILC(object):
             raise RuntimeError('You must run this before the with statement!')
 
         def argument_function(handler):
+            if 'arg_only' in kwargs and kwargs['arg_only']:
+                arg_name = self.get_argument_name(*args, **kwargs)
+                self.arg_only.append(arg_name)
+                del kwargs['arg_only']
+
             if handler is self._entrypoint:
                 self.add_argument(*args, **kwargs)
 
@@ -485,15 +501,20 @@ class MILC(object):
             if argument in ('subparsers', 'entrypoint'):
                 continue
 
-            if '_' not in argument:
+            if '_' in argument:
-                continue
+                section, option = argument.split('_', 1)
-
-            section, option = argument.split('_', 1)
-            if hasattr(self.args_passed, argument):
-                self.config[section][option] = getattr(self.args, argument)
             else:
-                if option not in self.config[section]:
+                section = self._entrypoint.__name__
-                    self.config[section][option] = getattr(self.args, argument)
+                option = argument
+
+            if option not in self.arg_only:
+                if hasattr(self.args_passed, argument):
+                    arg_value = getattr(self.args, argument)
+                    if arg_value:
+                        self.config[section][option] = arg_value
+                else:
+                    if option not in self.config[section]:
+                        self.config[section][option] = getattr(self.args, argument)
 
         self.release_lock()
 
@@ -509,6 +530,8 @@ class MILC(object):
         self.acquire_lock()
 
         config = RawConfigParser()
+        config_dir = os.path.dirname(self.config_file)
+
         for section_name, section in self.config._config.items():
             config.add_section(section_name)
             for option_name, value in section.items():
@@ -517,7 +540,10 @@ class MILC(object):
                         continue
                 config.set(section_name, option_name, str(value))
 
-        with NamedTemporaryFile(mode='w', dir=os.path.dirname(self.config_file), delete=False) as tmpfile:
+        if not os.path.exists(config_dir):
+            os.makedirs(config_dir)
+
+        with NamedTemporaryFile(mode='w', dir=config_dir, delete=False) as tmpfile:
             config.write(tmpfile)
 
         # Move the new config file into place atomically
@@ -527,6 +553,7 @@ class MILC(object):
             self.log.warning('Config file saving failed, not replacing %s with %s.', self.config_file, tmpfile.name)
 
         self.release_lock()
+        cli.log.info('Wrote configuration to %s', shlex.quote(self.config_file))
 
     def __call__(self):
         """Execute the entrypoint function.
@@ -602,8 +629,8 @@ class MILC(object):
         """Called by __enter__() to setup the logging configuration.
         """
         if len(logging.root.handlers) != 0:
-            # This is not a design decision. This is what I'm doing for now until I can examine and think about this situation in more detail.
+            # MILC is the only thing that should have root log handlers
-            raise RuntimeError('MILC should be the only system installing root log handlers!')
+            logging.root.handlers = []
 
         self.acquire_lock()
 
@@ -648,8 +675,9 @@ class MILC(object):
         self.read_config()
         self.setup_logging()
 
-        if self.config.general.save_config:
+        if 'save_config' in self.config.general and self.config.general.save_config:
             self.save_config()
+            exit(0)
 
         return self
 
@@ -712,4 +740,3 @@ if __name__ == '__main__':
     cli.goodbye.add_argument('-n', '--name', help='Name to bid farewell to', default='World')
 
     cli()  # Automatically picks between main(), hello() and goodbye()
-    print(sorted(ansi_colors.keys()))

lib/python/qmk/cli/__init__.py

@@ -0,0 +1,13 @@
+"""QMK CLI Subcommands
+
+We list each subcommand here explicitly because all the reliable ways of searching for modules are slow and delay startup.
+"""
+from . import cformat
+from . import compile
+from . import config
+from . import doctor
+from . import hello
+from . import json
+from . import new
+from . import pyformat
+from . import pytest

lib/python/qmk/cli/cformat.py

@@ -6,9 +6,9 @@ import subprocess
 from milc import cli
 
 
-@cli.argument('files', nargs='*', help='Filename(s) to format.')
+@cli.argument('files', nargs='*', arg_only=True, help='Filename(s) to format.')
-@cli.entrypoint("Format C code according to QMK's style.")
+@cli.subcommand("Format C code according to QMK's style.")
-def main(cli):
+def cformat(cli):
     """Format C code according to QMK's style.
     """
     clang_format = ['clang-format', '-i']

lib/python/qmk/cli/compile.py

@@ -14,11 +14,11 @@ import qmk.keymap
 import qmk.path
 
 
-@cli.argument('filename', nargs='?', type=FileType('r'), help='The configurator export to compile')
+@cli.argument('filename', nargs='?', arg_only=True, type=FileType('r'), help='The configurator export to compile')
 @cli.argument('-kb', '--keyboard', help='The keyboard to build a firmware for. Ignored when a configurator export is supplied.')
 @cli.argument('-km', '--keymap', help='The keymap to build a firmware for. Ignored when a configurator export is supplied.')
-@cli.entrypoint('Compile a QMK Firmware.')
+@cli.subcommand('Compile a QMK Firmware.')
-def main(cli):
+def compile(cli):
     """Compile a QMK Firmware.
 
     If a Configurator export is supplied this command will create a new keymap, overwriting an existing keymap if one exists.
@@ -41,9 +41,9 @@ def main(cli):
         # Compile the keymap
         command = ['make', ':'.join((user_keymap['keyboard'], user_keymap['keymap']))]
 
-    elif cli.config.general.keyboard and cli.config.general.keymap:
+    elif cli.config.compile.keyboard and cli.config.compile.keymap:
         # Generate the make command for a specific keyboard/keymap.
-        command = ['make', ':'.join((cli.config.general.keyboard, cli.config.general.keymap))]
+        command = ['make', ':'.join((cli.config.compile.keyboard, cli.config.compile.keymap))]
 
     else:
         cli.log.error('You must supply a configurator export or both `--keyboard` and `--keymap`.')

lib/python/qmk/cli/config.py

@@ -0,0 +1,96 @@
+"""Read and write configuration settings
+"""
+import os
+import subprocess
+
+from milc import cli
+
+
+def print_config(section, key):
+    """Print a single config setting to stdout.
+    """
+    cli.echo('%s.%s{fg_cyan}={fg_reset}%s', section, key, cli.config[section][key])
+
+
+@cli.argument('-ro', '--read-only', action='store_true', help='Operate in read-only mode.')
+@cli.argument('configs', nargs='*', arg_only=True, help='Configuration options to read or write.')
+@cli.subcommand("Read and write configuration settings.")
+def config(cli):
+    """Read and write config settings.
+
+    This script iterates over the config_tokens supplied as argument. Each config_token has the following form:
+
+        section[.key][=value]
+
+    If only a section (EG 'compile') is supplied all keys for that section will be displayed.
+
+    If section.key is supplied the value for that single key will be displayed.
+
+    If section.key=value is supplied the value for that single key will be set.
+
+    If section.key=None is supplied the key will be deleted.
+
+    No validation is done to ensure that the supplied section.key is actually used by qmk scripts.
+    """
+    if not cli.args.configs:
+        # Walk the config tree
+        for section in cli.config:
+            for key in cli.config[section]:
+                print_config(section, key)
+
+        return True
+
+    # Process config_tokens
+    save_config = False
+
+    for argument in cli.args.configs:
+        # Split on space in case they quoted multiple config tokens
+        for config_token in argument.split(' '):
+            # Extract the section, config_key, and value to write from the supplied config_token.
+            if '=' in config_token:
+                key, value = config_token.split('=')
+            else:
+                key = config_token
+                value = None
+
+            if '.' in key:
+                section, config_key = key.split('.', 1)
+            else:
+                section = key
+                config_key = None
+
+            # Validation
+            if config_key and '.' in config_key:
+                cli.log.error('Config keys may not have more than one period! "%s" is not valid.', key)
+                return False
+
+            # Do what the user wants
+            if section and config_key and value:
+                # Write a config key
+                log_string = '%s.%s{fg_cyan}:{fg_reset} %s {fg_cyan}->{fg_reset} %s'
+                if cli.args.read_only:
+                    log_string += ' {fg_red}(change not written)'
+
+                cli.echo(log_string, section, config_key, cli.config[section][config_key], value)
+
+                if not cli.args.read_only:
+                    if value == 'None':
+                        del cli.config[section][config_key]
+                    else:
+                        cli.config[section][config_key] = value
+                    save_config = True
+
+            elif section and config_key:
+                # Display a single key
+                print_config(section, config_key)
+
+            elif section:
+                # Display an entire section
+                for key in cli.config[section]:
+                    print_config(section, key)
+
+    # Ending actions
+    if save_config:
+        cli.save_config()
+
+    return True

lib/python/qmk/cli/doctor.py

@@ -11,8 +11,8 @@ from glob import glob
 from milc import cli
 
 
-@cli.entrypoint('Basic QMK environment checks')
+@cli.subcommand('Basic QMK environment checks')
-def main(cli):
+def doctor(cli):
     """Basic QMK environment checks.
 
     This is currently very simple, it just checks that all the expected binaries are on your system.
@@ -36,6 +36,7 @@ def main(cli):
         else:
             try:
                 subprocess.run([binary, '--version'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=5, check=True)
+                cli.log.info('Found {fg_cyan}%s', binary)
             except subprocess.CalledProcessError:
                 cli.log.error("{fg_red}Can't run `%s --version`", binary)
                 ok = False

lib/python/qmk/cli/hello.py

@@ -6,8 +6,8 @@ from milc import cli
 
 
 @cli.argument('-n', '--name', default='World', help='Name to greet.')
-@cli.entrypoint('QMK Hello World.')
+@cli.subcommand('QMK Hello World.')
-def main(cli):
+def hello(cli):
     """Log a friendly greeting.
     """
-    cli.log.info('Hello, %s!', cli.config.general.name)
+    cli.log.info('Hello, %s!', cli.config.hello.name)

lib/python/qmk/cli/json/__init__.py

@@ -0,0 +1,5 @@
+"""QMK CLI JSON Subcommands
+
+We list each subcommand here explicitly because all the reliable ways of searching for modules are slow and delay startup.
+"""
+from . import keymap

lib/python/qmk/cli/json/keymap.py

@@ -9,10 +9,10 @@ from milc import cli
 import qmk.keymap
 
 
-@cli.argument('-o', '--output', help='File to write to')
+@cli.argument('-o', '--output', arg_only=True, help='File to write to')
-@cli.argument('filename', help='Configurator JSON file')
+@cli.argument('filename', arg_only=True, help='Configurator JSON file')
-@cli.entrypoint('Create a keymap.c from a QMK Configurator export.')
+@cli.subcommand('Create a keymap.c from a QMK Configurator export.')
-def main(cli):
+def json_keymap(cli):
     """Generate a keymap.c from a configurator export.
 
     This command uses the `qmk.keymap` module to generate a keymap.c from a configurator export. The generated keymap is written to stdout, or to a file if -o is provided.
@@ -28,8 +28,8 @@ def main(cli):
         exit(1)
 
     # Environment processing
-    if cli.config.general.output == ('-'):
+    if cli.args.output == ('-'):
-        cli.config.general.output = None
+        cli.args.output = None
 
     # Parse the configurator json
     with open(qmk.path.normpath(cli.args.filename), 'r') as fd:
@@ -38,17 +38,17 @@ def main(cli):
     # Generate the keymap
     keymap_c = qmk.keymap.generate(user_keymap['keyboard'], user_keymap['layout'], user_keymap['layers'])
 
-    if cli.config.general.output:
+    if cli.args.output:
-        output_dir = os.path.dirname(cli.config.general.output)
+        output_dir = os.path.dirname(cli.args.output)
 
         if not os.path.exists(output_dir):
             os.makedirs(output_dir)
 
-        output_file = qmk.path.normpath(cli.config.general.output)
+        output_file = qmk.path.normpath(cli.args.output)
         with open(output_file, 'w') as keymap_fd:
             keymap_fd.write(keymap_c)
 
-        cli.log.info('Wrote keymap to %s.', cli.config.general.output)
+        cli.log.info('Wrote keymap to %s.', cli.args.output)
 
     else:
         print(keymap_c)

lib/python/qmk/cli/new/__init__.py

@@ -0,0 +1 @@
+from . import keymap

lib/python/qmk/cli/new/keymap.py

@@ -6,15 +6,15 @@ import shutil
 from milc import cli
 
 
-@cli.argument('-k', '--keyboard', help='Specify keyboard name. Example: 1upkeyboards/1up60hse')
+@cli.argument('-kb', '--keyboard', help='Specify keyboard name. Example: 1upkeyboards/1up60hse')
-@cli.argument('-u', '--username', help='Specify any name for the new keymap directory')
+@cli.argument('-km', '--keymap', help='Specify the name for the new keymap directory')
-@cli.entrypoint('Creates a new keymap for the keyboard of your choosing')
+@cli.subcommand('Creates a new keymap for the keyboard of your choosing')
-def main(cli):
+def new_keymap(cli):
     """Creates a new keymap for the keyboard of your choosing.
     """
     # ask for user input if keyboard or username was not provided in the command line
-    keyboard = cli.config.general.keyboard if cli.config.general.keyboard else input("Keyboard Name: ")
+    keyboard = cli.config.new_keymap.keyboard if cli.config.new_keymap.keyboard else input("Keyboard Name: ")
-    username = cli.config.general.username if cli.config.general.username else input("Username: ")
+    keymap = cli.config.new_keymap.keymap if cli.config.new_keymap.keymap else input("Keymap Name: ")
 
     # generate keymap paths
     kb_path = os.path.join(os.getcwd(), "keyboards", keyboard)
@@ -36,6 +36,5 @@ def main(cli):
     shutil.copytree(keymap_path_default, keymap_path, symlinks=True)
 
     # end message to user
-    cli.log.info("%s keymap directory created in: %s\n" +
+    cli.log.info("%s keymap directory created in: %s", username, keymap_path)
-                 "Compile a firmware file with your new keymap by typing: \n" +
+    cli.log.info("Compile a firmware with your new keymap by typing: \n" + "qmk compile -kb %s -km %s", keyboard, username)
-                 "qmk compile -kb %s -km %s", username, keymap_path, keyboard, username)

lib/python/qmk/cli/pyformat.py

@@ -5,12 +5,13 @@ from milc import cli
 import subprocess
 
 
-@cli.entrypoint("Format python code according to QMK's style.")
+@cli.subcommand("Format python code according to QMK's style.")
-def main(cli):
+def pyformat(cli):
     """Format python code according to QMK's style.
     """
     try:
         subprocess.run(['yapf', '-vv', '-ri', 'bin/qmk', 'lib/python'], check=True)
         cli.log.info('Successfully formatted the python code in `bin/qmk` and `lib/python`.')
+
     except subprocess.CalledProcessError:
         cli.log.error('Error formatting python code!')

lib/python/qmk/cli/pytest.py

@@ -2,17 +2,19 @@
 
 QMK script to run unit and integration tests against our python code.
 """
+import sys
 from milc import cli
 
 
-@cli.entrypoint('QMK Python Unit Tests')
+@cli.subcommand('QMK Python Unit Tests')
-def main(cli):
+def pytest(cli):
     """Use nose2 to run unittests
     """
     try:
         import nose2
+
     except ImportError:
         cli.log.error('Could not import nose2! Please install it with {fg_cyan}pip3 install nose2')
         return False
 
-    nose2.discover()
+    nose2.discover(argv=['nose2', '-v'])

lib/python/qmk/path.py

@@ -2,6 +2,7 @@
 """
 import logging
 import os
+from pkgutil import walk_packages
 
 from qmk.errors import NoSuchKeyboardError
 

lib/python/qmk/tests/test_cli_commands.py

@@ -0,0 +1,39 @@
+import subprocess
+
+
+def check_subcommand(command, *args):
+    cmd = ['bin/qmk', command] + list(args)
+    return subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+
+
+def test_cformat():
+    assert check_subcommand('cformat', 'tmk_core/common/backlight.c').returncode == 0
+
+
+def test_compile():
+    assert check_subcommand('compile', '-kb', 'handwired/onekey/pytest', '-km', 'default').returncode == 0
+
+
+def test_config():
+    result = check_subcommand('config')
+    assert result.returncode == 0
+    assert 'general.color' in result.stdout
+
+
+def test_doctor():
+    result = check_subcommand('doctor')
+    assert result.returncode == 0
+    assert 'QMK Doctor is checking your environment.' in result.stderr
+    assert 'QMK is ready to go' in result.stderr
+
+
+def test_hello():
+    result = check_subcommand('hello')
+    assert result.returncode == 0
+    assert 'Hello,' in result.stderr
+
+
+def test_pyformat():
+    result = check_subcommand('pyformat')
+    assert result.returncode == 0
+    assert 'Successfully formatted the python code' in result.stderr

requirements.txt

@@ -1,5 +1,5 @@
 # Python requirements
 # milc FIXME(skullydazed): Included in the repo for now.
+appdirs
 argcomplete
 colorama
-#halo