Skip to content

Commit

Permalink
refactor: address pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
antazoey committed Oct 31, 2023
1 parent 740f871 commit fac6ef5
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 73 deletions.
28 changes: 18 additions & 10 deletions docs/userguides/clis.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,22 +159,30 @@ def create_account(alias):
click.echo(alias)
```

You can control additional filtering of the alias account options by using one or both of the `account_type` or `account_list` kwargs.
Use `account_type` to filter the choices by specific types of [AccountAPI](../methoddocs/api.html#ape.api.accounts.AccountAPI).
Use the `account_list` kwarg to filter the choices by a given sequence of accounts.
You can control additional filtering of the accounts by using the `account_type` kwarg.
Use `account_type` to filter the choices by specific types of [AccountAPI](../methoddocs/api.html#ape.api.accounts.AccountAPI), or you can give it a list of already known accounts, or you can provide a callable-filter that takes an account and returns a boolean.

```python
import click
from ape import accounts
from ape.cli import existing_alias_argument
from ape.cli import existing_alias_argument, get_user_selected_account
from ape_accounts.accounts import KeyfileAccount

def _valid_accounts():
return [a for a in accounts if a.alias.startswith("root_signer_")]
# NOTE: This is just an example and not anything specific or recommended.
APPLICATION_PREFIX = "<FOO_BAR>"

@click.command()
@existing_alias_argument(account_type=KeyfileAccount, account_list=_valid_accounts())
def cli(alias):
# alias matches a KeyfileAccount type with an alias beginning with `root_signer_`.
print(alias)
@existing_alias_argument(account_type=KeyfileAccount)
def cli_0(alias):
pass

@click.command()
@existing_alias_argument(account_type=lambda a: a.alias.startswith(APPLICATION_PREFIX))
def cli_1(alias):
pass


# Select from the given accounts directly.
my_accounts = [accounts.load("me"), accounts.load("me2")]
selected_account = get_user_selected_account(account_type=my_accounts)
```
11 changes: 3 additions & 8 deletions src/ape/cli/arguments.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
from itertools import chain
from typing import Optional, Sequence, Type

import click
from eth_utils import is_hex

from ape import accounts, project
from ape.api import AccountAPI
from ape.cli.choices import Alias
from ape.cli.choices import _ACCOUNT_TYPE_FILTER, Alias
from ape.cli.paramtype import AllFilePaths
from ape.exceptions import AccountsError, AliasAlreadyInUseError

Expand All @@ -26,10 +24,7 @@ def _alias_callback(ctx, param, value):
return value


def existing_alias_argument(
account_type: Optional[Type[AccountAPI]] = None,
account_list: Optional[Sequence[AccountAPI]] = None,
):
def existing_alias_argument(account_type: _ACCOUNT_TYPE_FILTER = None):
"""
A ``click.argument`` for an existing account alias.
Expand All @@ -38,7 +33,7 @@ def existing_alias_argument(
If given, limits the type of account the user may choose from.
"""

return click.argument("alias", type=Alias(account_type=account_type, account_list=account_list))
return click.argument("alias", type=Alias(account_type=account_type))


def non_existing_alias_argument():
Expand Down
91 changes: 43 additions & 48 deletions src/ape/cli/choices.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re
from enum import Enum
from functools import lru_cache
from typing import Any, Iterator, List, Optional, Sequence, Type, Union
from typing import Any, Callable, Iterator, List, Optional, Sequence, Type, Union

import click
from click import BadParameter, Choice, Context, Parameter
Expand All @@ -12,27 +12,30 @@
from ape.types import _LazySequence

ADHOC_NETWORK_PATTERN = re.compile(r"\w*:\w*:https?://\w*.*")
_ACCOUNT_TYPE_FILTER = Union[
None, Sequence[AccountAPI], Type[AccountAPI], Callable[[AccountAPI], bool]
]


def _get_accounts(
account_type: Optional[Type[AccountAPI]] = None,
account_list: Optional[Sequence[AccountAPI]] = None,
) -> List[AccountAPI]:
def _get_accounts(account_type: _ACCOUNT_TYPE_FILTER) -> List[AccountAPI]:
add_test_accounts = False
if account_list is None and account_type is not None:
# Given type and not a list.
account_list = accounts.get_accounts_by_type(account_type)

elif account_list is None:
# Given an account list and not a type.
if account_type is None:
account_list = list(accounts)

# Include test accounts at end.
add_test_accounts = True

elif account_type:
# Given both type and list.
account_list = [a for a in account_list if isinstance(a, account_type)]
elif isinstance(account_type, type):
# Filtering by type.
account_list = accounts.get_accounts_by_type(account_type)

elif isinstance(account_type, (list, tuple, set)):
# Given an account list.
account_list = account_type # type: ignore

else:
# Filtering by callable.
account_list = [a for a in accounts if account_type(a)] # type: ignore

sorted_accounts = sorted(account_list, key=lambda a: a.alias or "")
if add_test_accounts:
Expand All @@ -51,20 +54,15 @@ class Alias(click.Choice):

name = "alias"

def __init__(
self,
account_type: Optional[Type[AccountAPI]] = None,
account_list: Optional[Sequence[AccountAPI]] = None,
):
def __init__(self, account_type: _ACCOUNT_TYPE_FILTER = None):
# NOTE: we purposely skip the constructor of `Choice`
self.case_sensitive = False
self._account_type = account_type
self._account_list = account_list
self.choices = _LazySequence(self._choices_iterator)

@property
def _choices_iterator(self) -> Iterator[str]:
for acct in _get_accounts(account_type=self._account_type, account_list=self._account_list):
for acct in _get_accounts(account_type=self._account_type):
if acct.alias is None:
continue

Expand Down Expand Up @@ -141,9 +139,7 @@ def get_user_selected_choice(self) -> str:


def get_user_selected_account(
prompt_message: Optional[str] = None,
account_type: Optional[Type[AccountAPI]] = None,
account_list: Optional[Sequence[AccountAPI]] = None,
prompt_message: Optional[str] = None, account_type: _ACCOUNT_TYPE_FILTER = None
) -> AccountAPI:
"""
Prompt the user to pick from their accounts and return that account.
Expand All @@ -153,24 +149,19 @@ def get_user_selected_account(
Args:
prompt_message (Optional[str]): Customize the prompt message.
account_type (Optional[Type[:class:`~ape.api.accounts.AccountAPI`]]]):
If given, the user may only select an account of this type.
account_list (Optional[Sequence[:class:`~ape.api.accounts.AccountAPI`]]):
Optionally provide a list of accounts to use as choices. Defaults to other
accounts. Works independently of ``account_type`` kwarg.
account_type (Union[None, Type[AccountAPI], Callable[[AccountAPI], bool]]):
If given, the user may only select a matching account. You can provide
a list of accounts, an account class type, or a callable for filtering
the accounts.
Returns:
:class:`~ape.api.accounts.AccountAPI`
"""

if account_type and not issubclass(account_type, AccountAPI):
if account_type and isinstance(account_type, type) and not issubclass(account_type, AccountAPI):
raise AccountsError(f"Cannot return accounts with type '{account_type}'.")

prompt = AccountAliasPromptChoice(
prompt_message=prompt_message,
account_type=account_type,
account_list=account_list,
)
prompt = AccountAliasPromptChoice(prompt_message=prompt_message, account_type=account_type)
return prompt.get_user_selected_account()


Expand All @@ -182,22 +173,30 @@ class AccountAliasPromptChoice(PromptChoice):

def __init__(
self,
account_type: Optional[Type[AccountAPI]] = None,
account_type: _ACCOUNT_TYPE_FILTER = None,
prompt_message: Optional[str] = None,
name: str = "account",
account_list: Optional[Sequence[AccountAPI]] = None,
):
# NOTE: we purposely skip the constructor of `PromptChoice`
self._account_type = account_type
self._account_list = account_list
self._prompt_message = prompt_message or "Select an account"
self.name = name
self.choices = _LazySequence(self._choices_iterator)

def convert(
self, value: Any, param: Optional[Parameter], ctx: Optional[Context]
) -> Optional[AccountAPI]:
if isinstance(value, str) and value.startswith("TEST::"):
# Prompt the user if they didn't provide a value.

if value is None:
return None

if isinstance(value, str) and value.isnumeric():
alias = super().convert(value, param, ctx)
else:
alias = value

if isinstance(alias, str) and alias.startswith("TEST::"):
idx_str = value.replace("TEST::", "")
if not idx_str.isnumeric():
self.fail(f"Cannot reference test account by '{value}'.", param=param)
Expand All @@ -208,12 +207,10 @@ def convert(

self.fail(f"Index '{idx_str}' is not valid.", param=param)

if value and value in accounts.aliases:
return accounts.load(value)
elif alias and alias in accounts.aliases:
return accounts.load(alias)

# Prompt the user if they didn't provide a value.
alias = super().convert(value, param, ctx)
return accounts.load(alias) if alias else None
return None

def print_choices(self):
choices = dict(enumerate(self.choices, 0))
Expand All @@ -237,14 +234,12 @@ def print_choices(self):
@property
def _choices_iterator(self) -> Iterator[str]:
# Yield real accounts.
for account in _get_accounts(
account_type=self._account_type, account_list=self._account_list
):
for account in _get_accounts(account_type=self._account_type):
if account and (alias := account.alias):
yield alias

# Yield test accounts.
if self._account_type is not None and self._account_list is not None:
if self._account_type is None:
for idx, _ in enumerate(accounts.test_accounts):
yield f"TEST::{idx}"

Expand Down
5 changes: 3 additions & 2 deletions src/ape/cli/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from ape import networks, project
from ape.cli.choices import (
_ACCOUNT_TYPE_FILTER,
AccountAliasPromptChoice,
NetworkChoice,
OutputFormat,
Expand Down Expand Up @@ -182,15 +183,15 @@ def _account_callback(ctx, param, value):
return value


def account_option():
def account_option(account_type: _ACCOUNT_TYPE_FILTER = None):
"""
A CLI option that accepts either the account alias or the account number.
If not given anything, it will prompt the user to select an account.
"""

return click.option(
"--account",
type=AccountAliasPromptChoice(),
type=AccountAliasPromptChoice(account_type=account_type),
callback=_account_callback,
)

Expand Down
10 changes: 5 additions & 5 deletions tests/functional/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def test_get_user_selected_account_no_accounts_found(no_accounts):

def test_get_user_selected_account_one_account(runner, one_account):
# No input needed when only one account
with runner.isolation():
with runner.isolation("0\n"):
account = get_user_selected_account()

assert account == one_account
Expand Down Expand Up @@ -147,14 +147,14 @@ def test_get_user_selected_account_unknown_type(runner, keyfile_account):
def test_get_user_selected_account_with_account_list(
runner, keyfile_account, second_keyfile_account
):
account = get_user_selected_account(account_list=[keyfile_account])
account = get_user_selected_account(account_type=[keyfile_account])
assert account == keyfile_account

account = get_user_selected_account(account_list=[second_keyfile_account])
account = get_user_selected_account(account_type=[second_keyfile_account])
assert account == second_keyfile_account

with runner.isolation(input="1\n"):
account = get_user_selected_account(account_list=[keyfile_account, second_keyfile_account])
account = get_user_selected_account(account_type=[keyfile_account, second_keyfile_account])
assert account == second_keyfile_account


Expand Down Expand Up @@ -245,7 +245,7 @@ def test_account_option_uses_single_account_as_default(runner, one_account):
"""

@click.command()
@account_option()
@account_option(account_type=[one_account])
def cmd(account):
_expected = get_expected_account_str(account)
click.echo(_expected)
Expand Down

0 comments on commit fac6ef5

Please sign in to comment.