From 164016481d83da7ffae6da4002f389c94bfb1132 Mon Sep 17 00:00:00 2001 From: Ivan Habunek Date: Wed, 13 Dec 2023 16:14:46 +0100 Subject: [PATCH] Replace lists commands with subcommands --- CHANGELOG.md | 7 +- changelog.yaml | 3 +- docs/changelog.md | 7 +- tests/integration/test_lists.py | 36 ++++---- toot/api.py | 8 -- toot/cli/lists.py | 142 ++++++++++++++++++++++++++++---- 6 files changed, 156 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb94777..6894551 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,11 @@ noted below please report any issues. * Add `--json` option to tag commands * Add `tags info`, `tags featured`, `tags feature`, and `tags unfeature` commands -* Add `tags followed`, `tags follow`, and `tags unfollow` commands, deprecate - `tags_followed`, `tags_follow`, and `tags tags_unfollow` +* Add `tags followed`, `tags follow`, and `tags unfollow` sub-commands, + deprecate `tags_followed`, `tags_follow`, and `tags tags_unfollow` +* Add `lists accounts`, `lists add`, `lists create`, `lists delete`, `lists + list`, `lists remove` subcommands, deprecate `lists`, `lists_accounts`, + `lists_add`, `lists_create`, `lists_delete`, `lists_remove` commands. * Add `toot --width` option for setting your prefered terminal width * Add `--media-viewer` and `--colors` options to `toot tui`. These were previously accessible only via settings. diff --git a/changelog.yaml b/changelog.yaml index 6d2539d..cf25955 100644 --- a/changelog.yaml +++ b/changelog.yaml @@ -13,7 +13,8 @@ - "Add shell completion, see: https://toot.bezdomni.net/shell_completion.html" - "Add `--json` option to tag commands" - "Add `tags info`, `tags featured`, `tags feature`, and `tags unfeature` commands" - - "Add `tags followed`, `tags follow`, and `tags unfollow` commands, deprecate `tags_followed`, `tags_follow`, and `tags tags_unfollow`" + - "Add `tags followed`, `tags follow`, and `tags unfollow` sub-commands, deprecate `tags_followed`, `tags_follow`, and `tags tags_unfollow`" + - "Add `lists accounts`, `lists add`, `lists create`, `lists delete`, `lists list`, `lists remove` subcommands, deprecate `lists`, `lists_accounts`, `lists_add`, `lists_create`, `lists_delete`, `lists_remove` commands." - "Add `toot --width` option for setting your prefered terminal width" - "Add `--media-viewer` and `--colors` options to `toot tui`. These were previously accessible only via settings." diff --git a/docs/changelog.md b/docs/changelog.md index eb94777..6894551 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -22,8 +22,11 @@ noted below please report any issues. * Add `--json` option to tag commands * Add `tags info`, `tags featured`, `tags feature`, and `tags unfeature` commands -* Add `tags followed`, `tags follow`, and `tags unfollow` commands, deprecate - `tags_followed`, `tags_follow`, and `tags tags_unfollow` +* Add `tags followed`, `tags follow`, and `tags unfollow` sub-commands, + deprecate `tags_followed`, `tags_follow`, and `tags tags_unfollow` +* Add `lists accounts`, `lists add`, `lists create`, `lists delete`, `lists + list`, `lists remove` subcommands, deprecate `lists`, `lists_accounts`, + `lists_add`, `lists_create`, `lists_delete`, `lists_remove` commands. * Add `toot --width` option for setting your prefered terminal width * Add `--media-viewer` and `--colors` options to `toot tui`. These were previously accessible only via settings. diff --git a/tests/integration/test_lists.py b/tests/integration/test_lists.py index a5cbb1d..21b3f0b 100644 --- a/tests/integration/test_lists.py +++ b/tests/integration/test_lists.py @@ -4,83 +4,83 @@ from tests.integration.conftest import register_account def test_lists_empty(run): - result = run(cli.lists.lists) + result = run(cli.lists.list) assert result.exit_code == 0 assert result.stdout.strip() == "You have no lists defined." def test_list_create_delete(run): - result = run(cli.lists.list_create, "banana") + result = run(cli.lists.create, "banana") assert result.exit_code == 0 assert result.stdout.strip() == '✓ List "banana" created.' - result = run(cli.lists.lists) + result = run(cli.lists.list) assert result.exit_code == 0 assert "banana" in result.stdout - result = run(cli.lists.list_create, "mango") + result = run(cli.lists.create, "mango") assert result.exit_code == 0 assert result.stdout.strip() == '✓ List "mango" created.' - result = run(cli.lists.lists) + result = run(cli.lists.list) assert result.exit_code == 0 assert "banana" in result.stdout assert "mango" in result.stdout - result = run(cli.lists.list_delete, "banana") + result = run(cli.lists.delete, "banana") assert result.exit_code == 0 assert result.stdout.strip() == '✓ List "banana" deleted.' - result = run(cli.lists.lists) + result = run(cli.lists.list) assert result.exit_code == 0 assert "banana" not in result.stdout assert "mango" in result.stdout - result = run(cli.lists.list_delete, "mango") + result = run(cli.lists.delete, "mango") assert result.exit_code == 0 assert result.stdout.strip() == '✓ List "mango" deleted.' - result = run(cli.lists.lists) + result = run(cli.lists.list) assert result.exit_code == 0 assert result.stdout.strip() == "You have no lists defined." - result = run(cli.lists.list_delete, "mango") + result = run(cli.lists.delete, "mango") assert result.exit_code == 1 assert result.stderr.strip() == "Error: List not found" def test_list_add_remove(run, app): acc = register_account(app) - run(cli.lists.list_create, "foo") + run(cli.lists.create, "foo") - result = run(cli.lists.list_add, "foo", acc.username) + result = run(cli.lists.add, "foo", acc.username) assert result.exit_code == 1 assert result.stderr.strip() == f"Error: You must follow @{acc.username} before adding this account to a list." run(cli.accounts.follow, acc.username) - result = run(cli.lists.list_add, "foo", acc.username) + result = run(cli.lists.add, "foo", acc.username) assert result.exit_code == 0 assert result.stdout.strip() == f'✓ Added account "{acc.username}"' - result = run(cli.lists.list_accounts, "foo") + result = run(cli.lists.accounts, "foo") assert result.exit_code == 0 assert acc.username in result.stdout # Account doesn't exist - result = run(cli.lists.list_add, "foo", "does_not_exist") + result = run(cli.lists.add, "foo", "does_not_exist") assert result.exit_code == 1 assert result.stderr.strip() == "Error: Account not found" # List doesn't exist - result = run(cli.lists.list_add, "does_not_exist", acc.username) + result = run(cli.lists.add, "does_not_exist", acc.username) assert result.exit_code == 1 assert result.stderr.strip() == "Error: List not found" - result = run(cli.lists.list_remove, "foo", acc.username) + result = run(cli.lists.remove, "foo", acc.username) assert result.exit_code == 0 assert result.stdout.strip() == f'✓ Removed account "{acc.username}"' - result = run(cli.lists.list_accounts, "foo") + result = run(cli.lists.accounts, "foo") assert result.exit_code == 0 assert result.stdout.strip() == "This list has no accounts." diff --git a/toot/api.py b/toot/api.py index 1a7d3e5..c6aa91a 100644 --- a/toot/api.py +++ b/toot/api.py @@ -629,14 +629,6 @@ def get_lists(app, user): return http.get(app, user, "/api/v1/lists").json() -def find_list_id(app, user, title): - lists = get_lists(app, user) - for list_item in lists: - if list_item["title"] == title: - return list_item["id"] - return None - - def get_list_accounts(app, user, list_id): path = f"/api/v1/lists/{list_id}/accounts" return _get_response_list(app, user, path) diff --git a/toot/cli/lists.py b/toot/cli/lists.py index 089a2fa..0b485c6 100644 --- a/toot/cli/lists.py +++ b/toot/cli/lists.py @@ -2,14 +2,28 @@ import click from toot import api from toot.cli.base import Context, cli, pass_context -from toot.output import print_list_accounts, print_lists +from toot.output import print_list_accounts, print_lists, print_warning -@cli.command() -@pass_context -def lists(ctx: Context): - """List all lists""" - lists = api.get_lists(ctx.app, ctx.user) +@cli.group(invoke_without_command=True) +@click.pass_context +def lists(ctx: click.Context): + """Display and manage lists""" + if ctx.invoked_subcommand is None: + print_warning("`toot lists` is deprecated in favour of `toot lists list`") + lists = api.get_lists(ctx.obj.app, ctx.obj.user) + + if lists: + print_lists(lists) + else: + click.echo("You have no lists defined.") + + +@lists.command() +@click.pass_context +def list(ctx: click.Context): + """List all your lists""" + lists = api.get_lists(ctx.obj.app, ctx.obj.user) if lists: print_lists(lists) @@ -17,18 +31,99 @@ def lists(ctx: Context): click.echo("You have no lists defined.") -@cli.command(name="list_accounts") +@lists.command() @click.argument("title", required=False) @click.option("--id", help="List ID if not title is given") @pass_context -def list_accounts(ctx: Context, title: str, id: str): +def accounts(ctx: Context, title: str, id: str): """List the accounts in a list""" list_id = _get_list_id(ctx, title, id) response = api.get_list_accounts(ctx.app, ctx.user, list_id) print_list_accounts(response) -@cli.command(name="list_create") +@lists.command() +@click.argument("title") +@click.option( + "--replies-policy", + type=click.Choice(["followed", "list", "none"]), + default="none", + help="Replies policy" +) +@pass_context +def create(ctx: Context, title: str, replies_policy: str): + """Create a list""" + api.create_list(ctx.app, ctx.user, title=title, replies_policy=replies_policy) + click.secho(f"✓ List \"{title}\" created.", fg="green") + + +@lists.command() +@click.argument("title", required=False) +@click.option("--id", help="List ID if not title is given") +@pass_context +def delete(ctx: Context, title: str, id: str): + """Delete a list""" + list_id = _get_list_id(ctx, title, id) + api.delete_list(ctx.app, ctx.user, list_id) + click.secho(f"✓ List \"{title if title else id}\" deleted.", fg="green") + + +@lists.command() +@click.argument("title", required=False) +@click.argument("account") +@click.option("--id", help="List ID if not title is given") +@pass_context +def add(ctx: Context, title: str, account: str, id: str): + """Add an account to a list""" + list_id = _get_list_id(ctx, title, id) + found_account = api.find_account(ctx.app, ctx.user, account) + + try: + api.add_accounts_to_list(ctx.app, ctx.user, list_id, [found_account["id"]]) + except Exception: + # TODO: this is slow, improve + # if we failed to add the account, try to give a + # more specific error message than "record not found" + my_accounts = api.followers(ctx.app, ctx.user, found_account["id"]) + found = False + if my_accounts: + for my_account in my_accounts: + if my_account["id"] == found_account["id"]: + found = True + break + if found is False: + raise click.ClickException(f"You must follow @{account} before adding this account to a list.") + raise + + click.secho(f"✓ Added account \"{account}\"", fg="green") + + +@lists.command() +@click.argument("title", required=False) +@click.argument("account") +@click.option("--id", help="List ID if not title is given") +@pass_context +def remove(ctx: Context, title: str, account: str, id: str): + """Remove an account from a list""" + list_id = _get_list_id(ctx, title, id) + found_account = api.find_account(ctx.app, ctx.user, account) + api.remove_accounts_from_list(ctx.app, ctx.user, list_id, [found_account["id"]]) + click.secho(f"✓ Removed account \"{account}\"", fg="green") + + +@cli.command(name="list_accounts", hidden=True) +@click.argument("title", required=False) +@click.option("--id", help="List ID if not title is given") +@pass_context +def list_accounts(ctx: Context, title: str, id: str): + """List the accounts in a list""" + print_warning("`toot list_accounts` is deprecated in favour of `toot lists accounts`") + list_id = _get_list_id(ctx, title, id) + response = api.get_list_accounts(ctx.app, ctx.user, list_id) + print_list_accounts(response) + + +@cli.command(name="list_create", hidden=True) @click.argument("title") @click.option( "--replies-policy", @@ -39,28 +134,31 @@ def list_accounts(ctx: Context, title: str, id: str): @pass_context def list_create(ctx: Context, title: str, replies_policy: str): """Create a list""" + print_warning("`toot list_create` is deprecated in favour of `toot lists create`") api.create_list(ctx.app, ctx.user, title=title, replies_policy=replies_policy) click.secho(f"✓ List \"{title}\" created.", fg="green") -@cli.command(name="list_delete") +@cli.command(name="list_delete", hidden=True) @click.argument("title", required=False) @click.option("--id", help="List ID if not title is given") @pass_context def list_delete(ctx: Context, title: str, id: str): """Delete a list""" + print_warning("`toot list_delete` is deprecated in favour of `toot lists delete`") list_id = _get_list_id(ctx, title, id) api.delete_list(ctx.app, ctx.user, list_id) click.secho(f"✓ List \"{title if title else id}\" deleted.", fg="green") -@cli.command(name="list_add") +@cli.command(name="list_add", hidden=True) @click.argument("title", required=False) @click.argument("account") @click.option("--id", help="List ID if not title is given") @pass_context def list_add(ctx: Context, title: str, account: str, id: str): """Add an account to a list""" + print_warning("`toot list_add` is deprecated in favour of `toot lists add`") list_id = _get_list_id(ctx, title, id) found_account = api.find_account(ctx.app, ctx.user, account) @@ -83,13 +181,14 @@ def list_add(ctx: Context, title: str, account: str, id: str): click.secho(f"✓ Added account \"{account}\"", fg="green") -@cli.command(name="list_remove") +@cli.command(name="list_remove", hidden=True) @click.argument("title", required=False) @click.argument("account") @click.option("--id", help="List ID if not title is given") @pass_context def list_remove(ctx: Context, title: str, account: str, id: str): """Remove an account from a list""" + print_warning("`toot list_remove` is deprecated in favour of `toot lists remove`") list_id = _get_list_id(ctx, title, id) found_account = api.find_account(ctx.app, ctx.user, account) api.remove_accounts_from_list(ctx.app, ctx.user, list_id, [found_account["id"]]) @@ -97,8 +196,19 @@ def list_remove(ctx: Context, title: str, account: str, id: str): def _get_list_id(ctx: Context, title, list_id): - if not list_id: - list_id = api.find_list_id(ctx.app, ctx.user, title) - if not list_id: + if not list_id and not title: + raise click.ClickException("Please specify list title or ID") + + lists = api.get_lists(ctx.app, ctx.user) + matched_ids = [ + list["id"] for list in lists + if list["title"].lower() == title.lower() or list["id"] == list_id + ] + + if not matched_ids: raise click.ClickException("List not found") - return list_id + + if len(matched_ids) > 1: + raise click.ClickException("Found multiple lists with the same title, please specify the ID instead") + + return matched_ids[0]