From a7a7bd9f08704dfe56f581ef2fe04091018bf5f9 Mon Sep 17 00:00:00 2001 From: Zach White Date: Wed, 29 Apr 2020 10:38:56 -0700 Subject: [PATCH] improve the qmk setup flow (#20) * improve the qmk setup flow * Apply suggestions from code review Co-Authored-By: Erovia * Update qmk_cli/subcommands/setup.py * explicity set encoding to utf-8 * polishing Co-authored-by: Erovia --- milc.py | 119 +++++++++++++++++++++-------------- qmk_cli/git.py | 10 +-- qmk_cli/helpers.py | 5 +- qmk_cli/subcommands/clone.py | 5 +- qmk_cli/subcommands/setup.py | 53 ++++++++-------- 5 files changed, 111 insertions(+), 81 deletions(-) diff --git a/milc.py b/milc.py index eb18984..59432f7 100644 --- a/milc.py +++ b/milc.py @@ -295,7 +295,7 @@ class MILC(object): self.prog_name = self.prog_name.split('/')[-1] # Initialize all the things - self.read_config_file() + self.initialize_config() self.initialize_argparse() self.initialize_logging() @@ -482,22 +482,20 @@ class MILC(object): self.release_lock() - def read_config_file(self): - """Read in the configuration file and store it in self.config. + def read_config_file(self, config_file): + """Read in the configuration file and return Configuration objects for it and the config_source. """ - self.acquire_lock() - self.config = Configuration() - self.config_source = Configuration() - self.config_file = self.find_config_file() + config = Configuration() + config_source = Configuration() - if self.config_file and self.config_file.exists(): - config = RawConfigParser(self.config) - config.read(str(self.config_file)) + if config_file.exists(): + raw_config = RawConfigParser() + raw_config.read(str(config_file)) - # Iterate over the config file options and write them into self.config - for section in config.sections(): - for option in config.options(section): - value = config.get(section, option) + # Iterate over the config file options and write them into config + for section in raw_config.sections(): + for option in raw_config.options(section): + value = raw_config.get(section, option) # Coerce values into useful datatypes if value.lower() in ['1', 'yes', 'true', 'on']: @@ -512,9 +510,17 @@ class MILC(object): else: value = int(value) - self.config[section][option] = value - self.config_source[section][option] = 'config_file' + config[section][option] = value + config_source[section][option] = 'config_file' + return config, config_source + + def initialize_config(self): + """Read in the configuration file and store it in self.config. + """ + self.acquire_lock() + self.config_file = self.find_config_file() + self.config, self.config_source = self.read_config_file(self.config_file) self.release_lock() def merge_args_into_config(self): @@ -559,6 +565,54 @@ class MILC(object): self.release_lock() + def _save_config_file(self, config): + """Write config to disk. + """ + # Generate a sanitized version of our running configuration + sane_config = RawConfigParser() + for section_name, section in config._config.items(): + sane_config.add_section(section_name) + for option_name, value in section.items(): + if section_name == 'general': + if option_name in ['config_file']: + continue + if value is not None: + sane_config.set(section_name, option_name, str(value)) + + config_dir = self.config_file.parent + if not config_dir.exists(): + config_dir.mkdir(parents=True, exist_ok=True) + + # Write the config file atomically. + self.acquire_lock() + with NamedTemporaryFile(mode='w', dir=str(config_dir), delete=False) as tmpfile: + sane_config.write(tmpfile) + + if os.path.getsize(tmpfile.name) > 0: + os.replace(tmpfile.name, str(self.config_file)) + else: + self.log.warning('Config file saving failed, not replacing %s with %s.', str(self.config_file), tmpfile.name) + self.release_lock() + + def write_config_option(self, section, option): + """Save a single config option to the config file. + """ + if not self.config_file: + self.log.warning('%s.config_file not set, not saving config!', self.__class__.__name__) + return + + config, config_source = self.read_config_file(self.config_file) + + if section in config and option in config[section] and config[section][option] is None: + del(config[section][option]) + else: + config[section][option] = str(self.config[section][option]) + + self._save_config_file(config) + + # Housekeeping + self.log.info('Wrote configuration to %s', shlex.quote(str(self.config_file))) + def save_config(self): """Save the current configuration to the config file. """ @@ -568,36 +622,9 @@ class MILC(object): self.log.warning('%s.config_file file not set, not saving config!', self.__class__.__name__) return - self.acquire_lock() - - # Generate a sanitized version of our running configuration - config = RawConfigParser() - for section_name, section in self.config._config.items(): - config.add_section(section_name) - for option_name, value in section.items(): - if section_name == 'general': - if option_name in ['config_file']: - continue - if value is not None: - config.set(section_name, option_name, str(value)) - - # Write out the config file - config_dir = self.config_file.parent - if not config_dir.exists(): - config_dir.mkdir(parents=True, exist_ok=True) - - with NamedTemporaryFile(mode='w', dir=str(config_dir), delete=False) as tmpfile: - config.write(tmpfile) - - # Move the new config file into place atomically - if os.path.getsize(tmpfile.name) > 0: - os.replace(tmpfile.name, str(self.config_file)) - else: - self.log.warning('Config file saving failed, not replacing %s with %s.', str(self.config_file), tmpfile.name) - - # Housekeeping - self.release_lock() - cli.log.info('Wrote configuration to %s', shlex.quote(str(self.config_file))) + # Write config to disk + self._save_config_file(self.config) + self.log.info('Wrote configuration to %s', shlex.quote(str(self.config_file))) def __call__(self): """Execute the entrypoint function. diff --git a/qmk_cli/git.py b/qmk_cli/git.py index 269241b..77d8e78 100644 --- a/qmk_cli/git.py +++ b/qmk_cli/git.py @@ -9,7 +9,7 @@ default_fork = 'qmk/' + default_repo default_branch = 'master' -def clone(url, destination, branch): +def git_clone(url, destination, branch): git_clone = [ 'git', 'clone', @@ -20,13 +20,13 @@ def clone(url, destination, branch): ] cli.log.debug('Git clone command: %s', git_clone) - with subprocess.Popen(git_clone, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, bufsize=1, universal_newlines=True) as p: + with subprocess.Popen(git_clone, stderr=subprocess.STDOUT, stdout=subprocess.PIPE, bufsize=1, universal_newlines=True, encoding='utf-8') as p: for line in p.stdout: print(line, end='') if p.returncode != 0: cli.log.error('git clone exited %d', p.returncode) - return False else: - cli.log.info('Successfully cloned %s to %s!', cli.args.fork, cli.args.destination) - return True + cli.log.info('Successfully cloned %s to %s!', url, destination) + + return p.returncode diff --git a/qmk_cli/helpers.py b/qmk_cli/helpers.py index e9f3b8c..502c01a 100644 --- a/qmk_cli/helpers.py +++ b/qmk_cli/helpers.py @@ -1,6 +1,6 @@ """Useful helper functions. """ -from milc import cli +from milc import cli, format_ansi def question(question, boolean=True, default=''): @@ -18,10 +18,11 @@ def question(question, boolean=True, default=''): else: answer_key = 'y/n' - prompt = '*** %s [%s] ' % (question, answer_key) + prompt = format_ansi('\n*** %s [%s] ' % (question, answer_key)) while True: answer = input(prompt) + print() if answer == '' and default.lower() == 'y': answer = 'y' elif answer == '' and default.lower() == 'n': diff --git a/qmk_cli/subcommands/clone.py b/qmk_cli/subcommands/clone.py index 69233c5..4a922d1 100644 --- a/qmk_cli/subcommands/clone.py +++ b/qmk_cli/subcommands/clone.py @@ -4,7 +4,7 @@ import os from pathlib import Path from milc import cli -from qmk_cli.git import clone as git_clone +from qmk_cli.git import git_clone default_repo = 'qmk_firmware' default_fork = 'qmk/' + default_repo @@ -27,5 +27,4 @@ def clone(cli): cli.log.error('Destination already exists: %s', cli.args.destination) exit(1) - success = git_clone(git_url, cli.args.destination, cli.config.clone.branch) - exit(0 if success else 1) + return git_clone(git_url, cli.args.destination, cli.config.clone.branch) diff --git a/qmk_cli/subcommands/setup.py b/qmk_cli/subcommands/setup.py index 4d8a063..7433783 100644 --- a/qmk_cli/subcommands/setup.py +++ b/qmk_cli/subcommands/setup.py @@ -1,12 +1,13 @@ """Setup qmk_firmware on your computer. """ import os +import shlex import subprocess import sys from pathlib import Path from milc import cli -from qmk_cli.git import clone +from qmk_cli.git import git_clone from qmk_cli.helpers import question default_repo = 'qmk_firmware' @@ -18,11 +19,14 @@ default_branch = 'master' @cli.argument('-y', '--yes', arg_only=True, action='store_true', help='Answer yes to all questions') @cli.argument('--baseurl', default='https://github.com', help='The URL all git operations start from') @cli.argument('-b', '--branch', default=default_branch, help='The branch to clone') -@cli.argument('destination', default=os.environ['QMK_HOME'], nargs='?', help='The directory to clone to') +@cli.argument('-H', '--home', default=Path(os.environ['QMK_HOME']), type=Path, help='The location for QMK Firmware. Defaults to %s' % (os.environ['QMK_HOME'],)) @cli.argument('fork', default=default_fork, nargs='?', help='The qmk_firmware fork to clone') @cli.subcommand('Setup your computer for qmk_firmware.') def setup(cli): - qmk_firmware = Path(cli.args.destination) + """Guide the user through setting up their QMK environment. + """ + clone_prompt = 'Would you like to clone {fg_cyan}%s{fg_reset} to {fg_cyan}%s{fg_reset}?' % (cli.args.fork, shlex.quote(str(cli.args.home))) + home_prompt = 'Would you like to set {fg_cyan}%s{fg_reset} as your QMK home?' % (shlex.quote(str(cli.args.home)),) # Sanity checks if cli.args.yes and cli.args.no: @@ -30,30 +34,29 @@ def setup(cli): exit(1) # Check on qmk_firmware, and if it doesn't exist offer to check it out. - if (qmk_firmware / 'Makefile').exists(): - cli.log.info('Found qmk_firmware at %s.', str(qmk_firmware)) + if (cli.args.home / 'Makefile').exists(): + cli.log.info('Found qmk_firmware at %s.', str(cli.args.home)) else: - cli.log.error('qmk_firmware not found!') - if question('Would you like to clone %s?' % cli.args.fork): + cli.log.error('Could not find qmk_firmware!') + if question(clone_prompt): git_url = '/'.join((cli.config.setup.baseurl, cli.args.fork)) - clone(git_url, cli.args.destination, cli.config.setup.branch) + result = git_clone(git_url, cli.args.home, cli.config.setup.branch) + if result != 0: + exit(1) + + # Offer to set `user.qmk_home` for them. + if cli.args.home != os.environ['QMK_HOME'] and question(home_prompt): + cli.config['user']['qmk_home'] = str(cli.args.home.absolute()) + cli.write_config_option('user', 'qmk_home') # Run `qmk_firmware/bin/qmk doctor` to check the rest of the environment out - if qmk_firmware.exists(): - qmk_bin = qmk_firmware / 'bin' / 'qmk' - doctor_cmd = [sys.executable, str(qmk_bin), 'doctor'] - if cli.args.yes: - doctor_cmd.append('--yes') - if cli.args.no: - doctor_cmd.append('--no') - doctor = subprocess.run(doctor_cmd) - if doctor.returncode != 0: - cli.log.error('Your build environment is not setup completely.') + qmk_bin = cli.args.home / 'bin/qmk' + doctor_cmd = [sys.executable, qmk_bin, 'doctor'] - if question('Would you like to run util/qmk_install?'): - curdir = os.getcwd() - os.chdir(str(qmk_firmware)) - process = subprocess.run(['util/qmk_install.sh']) - os.chdir(curdir) - if process.returncode == 0: - cli.log.info('QMK setup complete!') + if cli.args.yes: + doctor_cmd.append('--yes') + + if cli.args.no: + doctor_cmd.append('--no') + + subprocess.run(doctor_cmd)