diff --git a/python/.changelog.d/2123.changed b/python/.changelog.d/2123.changed new file mode 100644 index 000000000..49c572431 --- /dev/null +++ b/python/.changelog.d/2123.changed @@ -0,0 +1 @@ +Unify boolean arguments/options in trezorlib commands to on/off diff --git a/python/src/trezorlib/cli/__init__.py b/python/src/trezorlib/cli/__init__.py index be6beb1fd..5d471c92c 100644 --- a/python/src/trezorlib/cli/__init__.py +++ b/python/src/trezorlib/cli/__init__.py @@ -150,3 +150,53 @@ def with_client(func: "Callable[Concatenate[TrezorClient, P], R]") -> "Callable[ # the return type of @click.pass_obj is improperly specified and pyright doesn't # understand that it converts f(obj, *args, **kwargs) to f(*args, **kwargs) return trezorctl_command_with_client # type: ignore [cannot be assigned to return type] + + +class AliasedGroup(click.Group): + """Command group that handles aliases and Click 6.x compatibility. + + Click 7.0 silently switched all underscore_commands to dash-commands. + This implementation of `click.Group` responds to underscore_commands by invoking + the respective dash-command. + + Supply an `aliases` dict at construction time to provide an alternative list of + command names: + + >>> @click.command(cls=AliasedGroup, aliases={"do_bar", do_foo}) + >>> def cli(): + >>> ... + + If these commands are not known at the construction time, they can be set later: + + >>> @click.command(cls=AliasedGroup) + >>> def cli(): + >>> ... + >>> + >>> @cli.command() + >>> def do_foo(): + >>> ... + >>> + >>> cli.aliases={"do_bar", do_foo} + """ + + def __init__( + self, + aliases: Optional[Dict[str, click.Command]] = None, + *args: Any, + **kwargs: Any, + ) -> None: + super().__init__(*args, **kwargs) + self.aliases = aliases or {} + + def get_command(self, ctx: click.Context, cmd_name: str) -> Optional[click.Command]: + cmd_name = cmd_name.replace("_", "-") + # try to look up the real name + cmd = super().get_command(ctx, cmd_name) + if cmd: + return cmd + + # look for a backwards compatibility alias + if cmd_name in self.aliases: + return self.aliases[cmd_name] + + return None diff --git a/python/src/trezorlib/cli/device.py b/python/src/trezorlib/cli/device.py index ed9b6bf1d..402fb784c 100644 --- a/python/src/trezorlib/cli/device.py +++ b/python/src/trezorlib/cli/device.py @@ -39,8 +39,8 @@ BACKUP_TYPE = { } SD_PROTECT_OPERATIONS = { - "enable": messages.SdProtectOperationType.ENABLE, - "disable": messages.SdProtectOperationType.DISABLE, + "on": messages.SdProtectOperationType.ENABLE, + "off": messages.SdProtectOperationType.DISABLE, "refresh": messages.SdProtectOperationType.REFRESH, } @@ -259,8 +259,8 @@ def sd_protect( device. The options are: \b - enable - Generate SD card secret and use it to protect the PIN and storage. - disable - Remove SD card secret protection. + on - Generate SD card secret and use it to protect the PIN and storage. + off - Remove SD card secret protection. refresh - Replace the current SD card secret with a new one. """ if client.features.model == "1": diff --git a/python/src/trezorlib/cli/settings.py b/python/src/trezorlib/cli/settings.py index b3a51d791..906c92257 100644 --- a/python/src/trezorlib/cli/settings.py +++ b/python/src/trezorlib/cli/settings.py @@ -14,12 +14,12 @@ # You should have received a copy of the License along with this library. # If not, see . -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Optional, cast import click from .. import device, firmware, messages, toif -from . import ChoiceType, with_client +from . import AliasedGroup, ChoiceType, with_client if TYPE_CHECKING: from ..client import TrezorClient @@ -89,30 +89,49 @@ def image_to_tt(filename: str) -> bytes: return toif_image.to_bytes() +def _should_remove(enable: Optional[bool], remove: bool) -> bool: + """Helper to decide whether to remove something or not. + + Needed for backwards compatibility purposes, so we can support + both positive (enable) and negative (remove) args. + """ + if remove and enable: + raise click.ClickException("Argument and option contradict each other") + + if remove or enable is False: + return True + + return False + + @click.group(name="set") def cli() -> None: """Device settings.""" @cli.command() -@click.option("-r", "--remove", is_flag=True) +@click.option("-r", "--remove", is_flag=True, hidden=True) +@click.argument("enable", type=ChoiceType({"on": True, "off": False}), required=False) @with_client -def pin(client: "TrezorClient", remove: bool) -> str: +def pin(client: "TrezorClient", enable: Optional[bool], remove: bool) -> str: """Set, change or remove PIN.""" - return device.change_pin(client, remove) + # Remove argument is there for backwards compatibility + return device.change_pin(client, remove=_should_remove(enable, remove)) @cli.command() -@click.option("-r", "--remove", is_flag=True) +@click.option("-r", "--remove", is_flag=True, hidden=True) +@click.argument("enable", type=ChoiceType({"on": True, "off": False}), required=False) @with_client -def wipe_code(client: "TrezorClient", remove: bool) -> str: +def wipe_code(client: "TrezorClient", enable: Optional[bool], remove: bool) -> str: """Set or remove the wipe code. The wipe code functions as a "self-destruct PIN". If the wipe code is ever entered into any PIN entry dialog, then all private data will be immediately removed and the device will be reset to factory defaults. """ - return device.change_wipe_code(client, remove) + # Remove argument is there for backwards compatibility + return device.change_wipe_code(client, remove=_should_remove(enable, remove)) @cli.command() @@ -233,25 +252,39 @@ def experimental_features(client: "TrezorClient", enable: bool) -> str: # -@cli.group() -def passphrase() -> None: +# Using special class AliasedGroup, so we can support multiple commands +# to invoke the same function to keep backwards compatibility +@cli.command(cls=AliasedGroup, name="passphrase") +def passphrase_main() -> None: """Enable, disable or configure passphrase protection.""" # this exists in order to support command aliases for "enable-passphrase" # and "disable-passphrase". Otherwise `passphrase` would just take an argument. -@passphrase.command(name="enabled") +# Cast for type-checking purposes +passphrase = cast(AliasedGroup, passphrase_main) + + +@passphrase.command(name="on") @click.option("-f/-F", "--force-on-device/--no-force-on-device", default=None) @with_client -def passphrase_enable(client: "TrezorClient", force_on_device: Optional[bool]) -> str: +def passphrase_on(client: "TrezorClient", force_on_device: Optional[bool]) -> str: """Enable passphrase.""" return device.apply_settings( client, use_passphrase=True, passphrase_always_on_device=force_on_device ) -@passphrase.command(name="disabled") +@passphrase.command(name="off") @with_client -def passphrase_disable(client: "TrezorClient") -> str: +def passphrase_off(client: "TrezorClient") -> str: """Disable passphrase.""" return device.apply_settings(client, use_passphrase=False) + + +# Registering the aliases for backwards compatibility +# (these are not shown in --help docs) +passphrase.aliases = { + "enabled": passphrase_on, + "disabled": passphrase_off, +} diff --git a/python/src/trezorlib/cli/trezorctl.py b/python/src/trezorlib/cli/trezorctl.py index b4d43f242..8e133d85c 100755 --- a/python/src/trezorlib/cli/trezorctl.py +++ b/python/src/trezorlib/cli/trezorctl.py @@ -29,6 +29,7 @@ from ..client import TrezorClient from ..transport import enumerate_devices from ..transport.udp import UdpTransport from . import ( + AliasedGroup, TrezorConnection, binance, btc, @@ -57,8 +58,8 @@ LOG = logging.getLogger(__name__) COMMAND_ALIASES = { "change-pin": settings.pin, - "enable-passphrase": settings.passphrase_enable, - "disable-passphrase": settings.passphrase_disable, + "enable-passphrase": settings.passphrase_on, + "disable-passphrase": settings.passphrase_off, "wipe-device": device.wipe, "reset-device": device.setup, "recovery-device": device.recover, @@ -86,16 +87,9 @@ COMMAND_ALIASES = { } -class TrezorctlGroup(click.Group): +class TrezorctlGroup(AliasedGroup): """Command group that handles compatibility for trezorctl. - The purpose is twofold: convert underscores to dashes, and ensure old-style commands - still work with new-style groups. - - Click 7.0 silently switched all underscore_commands to dash-commands. - This implementation of `click.Group` responds to underscore_commands by invoking - the respective dash-command. - With trezorctl 0.11.5, we started to convert old-style long commands (such as "binance-sign-tx") to command groups ("binance") with subcommands ("sign-tx"). The `TrezorctlGroup` can perform subcommand lookup: if a command @@ -104,16 +98,12 @@ class TrezorctlGroup(click.Group): """ def get_command(self, ctx: click.Context, cmd_name: str) -> Optional[click.Command]: - cmd_name = cmd_name.replace("_", "-") - # try to look up the real name cmd = super().get_command(ctx, cmd_name) if cmd: return cmd - # look for a backwards compatibility alias - if cmd_name in COMMAND_ALIASES: - return COMMAND_ALIASES[cmd_name] - + # the subsequent lookups rely on dash-separated command names + cmd_name = cmd_name.replace("_", "-") # look for subcommand in btc - "sign-tx" is now "btc sign-tx" cmd = btc.cli.get_command(ctx, cmd_name) if cmd: @@ -138,7 +128,11 @@ def configure_logging(verbose: int) -> None: log.OMITTED_MESSAGES.add(messages.Features) -@click.command(cls=TrezorctlGroup, context_settings={"max_content_width": 400}) +@click.command( + cls=TrezorctlGroup, + context_settings={"max_content_width": 400}, + aliases=COMMAND_ALIASES, +) @click.option( "-p", "--path",