diff --git a/docs/config.md b/docs/config.md index a6a9a3e..4335c55 100644 --- a/docs/config.md +++ b/docs/config.md @@ -4,7 +4,7 @@ revup config - Edit revup configuration files. # SYNOPSIS -`revup config [--help] [--repo] ` +`revup config [--help] [--repo] [--delete] ` # DESCRIPTION @@ -40,6 +40,9 @@ revup will warn if attempting to specify them directly. : If specified, configuration value will be written to the file in the current repo. Otherwise, value will apply globally. +**--delete, -d** +: Delete the value with the given flag key. + # EXAMPLES The default value for `revup upload --skip-confirm` is `false`. The user diff --git a/revup/config.py b/revup/config.py index e7b14c4..1d4e939 100644 --- a/revup/config.py +++ b/revup/config.py @@ -4,10 +4,87 @@ import logging import os import re +from argparse import _StoreAction, _StoreFalseAction, _StoreTrueAction +from typing import Any, Dict, List, Optional from revup.types import RevupUsageException +class RevupArgParser(argparse.ArgumentParser): + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + + def add_argument(self, *args: Any, **kwargs: Any) -> argparse.Action: + """ + For each boolean store_true action, add a corresponding "no" store_false action + with the same target. + """ + action = super().add_argument(*args, **kwargs) + + if isinstance(action, _StoreTrueAction): + no_options = [] + for option_string in action.option_strings: + if option_string.startswith("--"): + no_options.append("--no-" + option_string[2:]) + elif option_string.startswith("-"): + no_options.append("-n" + option_string[1:]) + + if no_options: + action = _StoreFalseAction( + no_options, action.dest, action.default, False, "autogenerated negation" + ) + for op in no_options: + self._option_string_actions[op] = action + self._actions.append(action) + elif isinstance(action, _StoreFalseAction): + # We assume all store false actions are the autogenerated one + raise RuntimeError("Direct store_false actions are not allowed") + + return action + + def set_defaults_from_config(self, conf: configparser.ConfigParser) -> None: + cmd = self.get_command() + for option, action in self.get_actions().items(): + if conf.has_option(cmd, option): + self.set_option_default(option, action, conf.get(cmd, option)) + + def get_command(self) -> str: + return self.prog.split()[-1].replace("-", "_") + + def get_actions(self) -> Dict[str, argparse.Action]: + ret: Dict[str, argparse.Action] = {} + for action in self._actions: + if not isinstance(action, (_StoreTrueAction, _StoreAction)): + # Ignore nonconfigurable actions (help, auto-generated negation) + continue + + if len(action.option_strings) > 0 and action.option_strings[0].startswith("--"): + option = action.option_strings[0][2:].replace("-", "_") + ret[option] = action + return ret + + def set_option_default(self, option: str, action: argparse.Action, value: str) -> None: + if isinstance(action, _StoreTrueAction): + override = value.lower() + if override in ("true", "false"): + action.default = override == "true" + else: + raise ValueError( + f'"{override}" not a valid override for boolean flag {option}, must' + ' be "true" or "false"' + ) + elif isinstance(action, _StoreAction): + if action.choices and value not in action.choices: + raise ValueError( + f"Value {value} for {self.get_command()}.{option} is not" + f" one of the choices {action.choices}" + ) + + action.default = value + else: + raise RuntimeError("Unknown option type!: ", option, action) + + class Config: # Object containing configuration values. Populated by read(), and can then # be modified by set_value() @@ -44,7 +121,13 @@ def write(self) -> None: self.config.write(f) self.dirty = False - def set_value(self, section: str, key: str, value: str) -> None: + def set_value(self, section: str, key: str, value: Optional[str]) -> None: + if value is None: + if self.config.has_option(section, key): + self.config.remove_option(section, key) + self.dirty = True + return + if not self.config.has_section(section): self.config.add_section(section) self.dirty = True @@ -57,7 +140,7 @@ def get_config(self) -> configparser.ConfigParser: return self.config -def config_main(conf: Config, args: argparse.Namespace) -> int: +def config_main(conf: Config, args: argparse.Namespace, all_parsers: List[RevupArgParser]) -> int: split_key = args.flag[0].replace("-", "_").split(".") if len(split_key) == 1: command = "revup" @@ -68,34 +151,55 @@ def config_main(conf: Config, args: argparse.Namespace) -> int: else: raise RevupUsageException("Invalid flag argument (must be or .)") - if not args.value: - value = getpass.getpass(f"Input value for {command}.{key}: ").strip() - else: - value = args.value + all_commands = {p.get_command(): p for p in all_parsers} + if command not in all_commands: + raise RevupUsageException( + f"Invalid command section {command}, choose from {list(all_commands.keys())}" + ) + parser = all_commands[command] + actions = parser.get_actions() + + if not args.delete: + if key not in actions: + raise RevupUsageException( + f"Invalid option key {key}, choose from {list(actions.keys())}" + ) + + if args.delete: + value = None + if args.value: + raise RevupUsageException("Can't provide a value when using --delete") + elif args.value: + value = args.value if command == "revup" and key == "github_oauth": logging.warning( "Prefer to omit the value on command line when entering sensitive info. " "You may want to clear your shell history." ) + else: + value = getpass.getpass(f"Input value for {command}.{key}: ").strip() config_path = conf.repo_config_path if args.repo else conf.config_path this_config = Config(config_path) this_config.read() - if command == "revup" and key == "github_username": - # From https://www.npmjs.com/package/github-username-regex : - # Github username may only contain alphanumeric characters or hyphens. - # Github username cannot have multiple consecutive hyphens. - # Github username cannot begin or end with a hyphen. - # Maximum is 39 characters. - if not re.match(r"^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$", value, re.I): - raise ValueError(f"{args.value} is not a valid GitHub username") - elif command == "revup" and key == "github_oauth": - if not re.match(r"^[a-z\d_]+$", value, re.I): - raise ValueError("Input string is not a valid oauth") - elif command == "config": - raise RevupUsageException("Can't set defaults for the config command itself") + if value is not None: + # Check whether this value is allowed by the parser by attempting to set it + # (this may throw if the value is not allowed) + parser.set_option_default(key, actions[key], value) + + if command == "revup" and key == "github_username": + # From https://www.npmjs.com/package/github-username-regex : + # Github username may only contain alphanumeric characters or hyphens. + # Github username cannot have multiple consecutive hyphens. + # Github username cannot begin or end with a hyphen. + # Maximum is 39 characters. + if not re.match(r"^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$", value, re.I): + raise ValueError(f"{value} is not a valid GitHub username") + elif command == "revup" and key == "github_oauth": + if not re.match(r"^[a-z\d_]+$", value, re.I): + raise ValueError("Input string is not a valid oauth") this_config.set_value(command, key, value) this_config.write() diff --git a/revup/revup.py b/revup/revup.py index 1f8be33..6f7a95c 100755 --- a/revup/revup.py +++ b/revup/revup.py @@ -1,19 +1,18 @@ from __future__ import annotations import argparse -import configparser import logging import os import stat import subprocess import sys -from argparse import _StoreAction, _StoreFalseAction, _StoreTrueAction from builtins import FileNotFoundError from contextlib import asynccontextmanager -from typing import Any, AsyncGenerator, Tuple +from typing import Any, AsyncGenerator, List, Tuple import revup from revup import config, git, logs, shell +from revup.config import RevupArgParser from revup.types import RevupUsageException REVUP_CONFIG_ENV_VAR = "REVUP_CONFIG_PATH" @@ -38,55 +37,6 @@ def __call__(self, parser: Any, namespace: Any, values: Any, option_string: Any sys.exit(0) -class RevupArgParser(argparse.ArgumentParser): - def __init__(self, *args: Any, **kwargs: Any) -> None: - super().__init__(*args, **kwargs) - - def add_argument(self, *args: Any, **kwargs: Any) -> argparse.Action: - """ - For each boolean store_true action, add a corresponding "no" store_false action - with the same target. - """ - action = super().add_argument(*args, **kwargs) - - if isinstance(action, _StoreTrueAction): - no_options = [] - for option_string in action.option_strings: - if option_string.startswith("--"): - no_options.append("--no-" + option_string[2:]) - elif option_string.startswith("-"): - no_options.append("-n" + option_string[1:]) - - if no_options: - action = _StoreFalseAction( - no_options, action.dest, action.default, False, "autogenerated negation" - ) - for op in no_options: - self._option_string_actions[op] = action - self._actions.append(action) - return action - - def set_defaults_from_config(self, conf: configparser.ConfigParser) -> None: - cmds = self.prog.split() - cmd = cmds[0] if len(cmds) == 1 else cmds[1] - for action in self._actions: - if len(action.option_strings) > 0 and action.option_strings[0].startswith("--"): - option = action.option_strings[0][2:].replace("-", "_") - if isinstance(action, _StoreTrueAction): - if conf.has_option(cmd, option): - override = conf.get(cmd, option).lower() - if override in ("true", "false"): - action.default = override == "true" - else: - raise ValueError( - f'"{override}" not a valid override for boolean flag {option}, must' - ' be "true" or "false"' - ) - elif isinstance(action, _StoreAction): - if conf.has_option(cmd, option): - action.default = conf.get(cmd, option) - - def make_toplevel_parser() -> RevupArgParser: revup_parser = RevupArgParser(add_help=False, prog="revup") revup_parser.add_argument("--help", "-h", action=HelpAction, nargs=0) @@ -247,6 +197,18 @@ async def main() -> int: "config", add_help=False, ) + toolkit_parser = subparsers.add_parser( + "toolkit", description="Test various subfunctionalities." + ) + + # Intentionally does not contain config or toolkit parsers since the those are not configurable + all_parsers: List[RevupArgParser] = [ + revup_parser, + amend_parser, + cherry_pick_parser, + restack_parser, + upload_parser, + ] for p in [upload_parser, restack_parser, amend_parser]: # Some args are used by both upload and restack @@ -301,10 +263,8 @@ async def main() -> int: config_parser.add_argument("flag", nargs=1) config_parser.add_argument("value", nargs="?") config_parser.add_argument("--repo", "-r", action="store_true") + config_parser.add_argument("--delete", "-d", action="store_true") - toolkit_parser = subparsers.add_parser( - "toolkit", description="Test various subfunctionalities." - ) toolkit_subparsers = toolkit_parser.add_subparsers(dest="toolkit_cmd", required=True) detect_branch = toolkit_subparsers.add_parser( "detect-branch", description="Detect the base branch of the current branch." @@ -368,10 +328,15 @@ async def main() -> int: # Do an initial parsing pass, which handles HelpAction args = revup_parser.parse_args() - conf = await get_config() - for p in [revup_parser, amend_parser, cherry_pick_parser, restack_parser, upload_parser]: + # Run config before setting the config, in order to avoid the situation where a broken + # config prevents you from running config at all. + if args.cmd == "config": + logs.configure_logger(False, {}) + return config.config_main(conf, args, all_parsers) + + for p in all_parsers: assert isinstance(p, RevupArgParser) p.set_defaults_from_config(conf.get_config()) args = revup_parser.parse_args() @@ -383,9 +348,6 @@ async def main() -> int: ) dump_args(args) - if args.cmd == "config": - return config.config_main(conf, args) - git_ctx = await get_git(args) if args.cmd == "toolkit":