diff --git a/tools/developer_tools/bely-cli/CLAUDE.md b/tools/developer_tools/bely-cli/CLAUDE.md index 7f98f9d15..91f0e808f 100644 --- a/tools/developer_tools/bely-cli/CLAUDE.md +++ b/tools/developer_tools/bely-cli/CLAUDE.md @@ -102,14 +102,11 @@ Conventions that hold across all command modules: Text output is printed inline as work happens; structured output is accumulated into a `result` dict and emitted once at the end via `print_result`/`print_items`. Guard human-readable prints with `if fmt == "text"`. -- **`--format` and `--no-prompt` are per-leaf-command**, applied by the `common_options` - decorator in `cli.py` — never hoisted to the group level. `run_test.sh` asserts this. - The one exception is `tui_group`: it's declared with `@cli.group("tui", - invoke_without_command=True)` and carries `common_options` plus `--limit` itself, because - bare `bely-cli tui` (no subcommand) *is* a leaf invocation — `ctx.invoked_subcommand is - None` is what triggers `cmd_tui(mode="app", ...)`. `tui lookup` is a normal leaf - subcommand underneath it. `run_test.sh` checks both: `--format` on `tui lookup -h` and - `--limit` on the bare `tui -h`. +- **`--format` is per format-capable leaf command**, applied by `common_options` in + `cli.py`; `prompting_options` additionally supplies `--no-prompt` only to commands that + can request input or credentials. Delete commands use `--force` instead. The one group + exception is `tui_group`: it is declared with `invoke_without_command=True` and carries + `common_options` plus `--limit`, because bare `bely-cli tui` is itself a leaf invocation. - **Non-interactive mode** is a module-level flag in `common.py` (`set_no_prompt()` / `is_no_prompt()`), set by `--no-prompt` or automatically when `--file=-`. Any code path that would prompt must check `is_no_prompt()` first and raise @@ -182,7 +179,7 @@ Split so that most of it needs no terminal to test: behavior, and `source` picks drilling in from logbook types vs. starting at the document level with `core.recent_documents(...)`. Its `_exit_top()` pops back to whatever pushed this browse (e.g. a command-palette-pushed "My documents" browse), or -- when `root=True` - (the landing screen `app.py` pushes on mount) -- does nothing but notify, since `q` is the + (the landing screen `app.py` pushes on mount) -- does nothing but notify, since `ctrl+q` is the only quit key and an accidental escape shouldn't drop the user into an empty terminal. - `app.py` — `BelyTuiApp`, the shared CSS, `ensure_auth()` (see below), and `get_system_commands()`. Pushes `BrowseScreen` directly on mount for both modes; diff --git a/tools/developer_tools/bely-cli/README.md b/tools/developer_tools/bely-cli/README.md index 1dd9410fd..fd9b9ea46 100644 --- a/tools/developer_tools/bely-cli/README.md +++ b/tools/developer_tools/bely-cli/README.md @@ -207,7 +207,7 @@ Aliases accept the same arguments and produce the same output as their canonical - `bely-cli auth verify` checks whether the cached token is accepted by the server. - `bely-cli auth logout` invalidates the current server session and removes the cached token. -All commands support the shared `--format` and `--no-prompt` options. +Commands that produce output support `--format`. Commands that may request missing input or credentials support `--no-prompt`; delete commands use `--force` instead. ### `doc` — log documents @@ -248,12 +248,11 @@ bely-cli doc show --doc-id 99 --format json #### `bely-cli doc delete` / `rm` Delete a document selected by `--doc-name` or `--doc-id`. Confirmation is required unless -`--yes` is supplied. A document containing entries requires `--force`; `--force` does not -skip confirmation. +`--force` is supplied. Deleting a document that contains entries also requires `--force`. ```bash bely-cli doc delete --doc-id 99 -bely-cli doc rm --doc-name "Disposable Log" --force --yes +bely-cli doc rm --doc-name "Disposable Log" --force ``` ### `tui` — interactive terminal UIs @@ -276,7 +275,7 @@ bely-cli tui --limit 50 Browsing needs no authentication. `Enter` on an entry just keeps it in the preview instead of exiting, and `Esc` at the logbook list does nothing (there's nothing above it to -pop back to) — press `q` to quit. A few keys are available once you've drilled in: +pop back to) — press `ctrl+q` to quit. Pressing `q` alone shows a reminder instead of exiting. A few keys are available once you've drilled in: | Key | Level | Action | |-----|-------|--------| @@ -419,7 +418,7 @@ only appear there. `t` appears at the entries level only when the document has r | `Up` / `Down`, `PgUp` / `PgDn` | Move the highlight; the preview/info panel follows. | | `/` | Reveal and focus the filter box; incrementally filters the current table (case-insensitive substring, matched against the entry text — not the reply tree's glyphs or reply count). | | `Enter` | In the filter box, return focus to the table. Elsewhere, drill into the highlighted row, or select the entry at the entries level. | -| `Esc` / `Backspace` | Go back one level (from the table); does nothing at the logbook list — press `q` to quit. In the filter box, `Esc` returns focus to the table. | +| `Esc` / `Backspace` | Go back one level (from the table); does nothing at the logbook list — press `ctrl+q` to quit. In the filter box, `Esc` returns focus to the table. | | `d` | Logbook/document levels only: create a new document (see `bely-cli tui`'s "New document" above) — a mutation, so this is where the app authenticates if it hasn't already. | | `s` | Entries level only: save the highlighted entry's markdown to a file in the current directory. | | `y` | Entries level only: copy a `bely-cli entry get` reference for the highlighted entry to the clipboard. | @@ -431,7 +430,7 @@ only appear there. `t` appears at the entries level only when the document has r | `i` | Logbook/document levels only: toggle the side info panel. | | `f` | Entries level only: toggle the table to widen the preview pane. | | `r` | Refresh the current level, bypassing the in-session cache. Entry selection, preview scroll, filters, and collapsed threads are preserved when possible. | -| `q` | Quit without selecting. | +| `ctrl+q` | Quit without selecting. Pressing `q` alone shows a reminder. | Replies only nest one level deep. Pressing `p` on either a top-level entry or one of its replies targets the top-level thread; `n` still adds a separate top-level entry. @@ -552,10 +551,10 @@ bely-cli entry attachment ls -n "Shift Report" --id 42 --format json #### `bely-cli entry attachment delete` / `rm` -Delete an attachment using its numeric ID from `attachment list`: +Delete an attachment using its numeric ID from `attachment list`. Use `--force` to skip confirmation: ```bash -bely-cli entry attachment delete -d 99 --id 42 --attachment-id 7 +bely-cli entry attachment delete -d 99 --id 42 --attachment-id 7 --force ``` #### `bely-cli entry update` @@ -593,12 +592,11 @@ Delete a top-level entry or reply by log ID, with confirmation by default: ```bash bely-cli entry delete -d 99 --id 42 -bely-cli entry rm -n "Shift Report" --id 42 --yes +bely-cli entry rm -n "Shift Report" --id 42 --force ``` All delete commands support structured JSON/YAML output. Declining confirmation returns -`status: cancelled`; successful deletion returns `status: deleted`. In `--no-prompt` mode, -`--yes` is required. +`status: cancelled`; successful deletion returns `status: deleted`. Use `--force` to skip confirmation. #### `bely-cli entry get` diff --git a/tools/developer_tools/bely-cli/run_test.sh b/tools/developer_tools/bely-cli/run_test.sh index 069952369..6ff44e98c 100755 --- a/tools/developer_tools/bely-cli/run_test.sh +++ b/tools/developer_tools/bely-cli/run_test.sh @@ -32,11 +32,17 @@ $RUNNER bely-cli entry attachment add -h | grep -q -- --format $RUNNER bely-cli entry attachment list -h | grep -q -- --format $RUNNER bely-cli entry attachment ls -h | grep -q -- --format $RUNNER bely-cli doc delete -h | grep -q -- --force -$RUNNER bely-cli doc rm -h | grep -q -- --yes -$RUNNER bely-cli entry delete -h | grep -q -- --yes +$RUNNER bely-cli doc rm -h | grep -q -- --force +$RUNNER bely-cli entry delete -h | grep -q -- --force $RUNNER bely-cli entry rm -h | grep -q -- --id $RUNNER bely-cli entry attachment delete -h | grep -q -- --attachment-id -$RUNNER bely-cli entry attachment rm -h | grep -q -- --yes +$RUNNER bely-cli entry attachment rm -h | grep -q -- --force +# --no-prompt is limited to commands with interactive input or credential fallback. +$RUNNER bely-cli doc new -h | grep -q -- --no-prompt +$RUNNER bely-cli entry update -h | grep -q -- --no-prompt +! $RUNNER bely-cli doc show -h | grep -q -- --no-prompt +! $RUNNER bely-cli entry delete -h | grep -q -- --no-prompt +! $RUNNER bely-cli tui -h | grep -q -- --no-prompt $RUNNER bely-cli tui lookup -h | grep -q -- --format $RUNNER bely-cli shell -h | grep -q -- cache $RUNNER bely-cli shell init -h | grep -q -- --shell diff --git a/tools/developer_tools/bely-cli/src/bely_cli/cli.py b/tools/developer_tools/bely-cli/src/bely_cli/cli.py index 34584103e..0f2de42c7 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/cli.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/cli.py @@ -108,14 +108,19 @@ def decorator(f): def common_options(f): - """Options shared by all leaf commands (--format and --no-prompt).""" - return format_option(no_prompt_option(f)) + """Options shared by format-capable leaf commands.""" + return format_option(f) + + +def prompting_options(f): + """Format and non-interactive options for commands that may prompt.""" + return common_options(no_prompt_option(f)) @click.group(context_settings=CONTEXT_SETTINGS) def cli(): """BELY logbook CLI""" - pass + set_no_prompt(False) # -- auth -- @@ -127,7 +132,7 @@ def auth_group(): @auth_group.command("login") -@common_options +@prompting_options def auth_login(output_format): """Log in and cache an authentication token.""" cmd_auth_login(fmt=output_format) @@ -173,7 +178,7 @@ def doc_group(): type=click.Choice(["system", "type", "template"]), default=None, help="List available values for the given option and exit") -@common_options +@prompting_options def doc_new(output_format, **kwargs): """Create a new log document.""" if kwargs.get('file') == '-': @@ -183,7 +188,7 @@ def doc_new(output_format, **kwargs): @doc_group.command("list") @click.option("--limit", default=20, type=int, help="Max documents to return (default 20)") -@common_options +@prompting_options def doc_list(output_format, **kwargs): """List recent log documents created by you.""" cmd_list_docs(fmt=output_format, **kwargs) @@ -201,11 +206,15 @@ def doc_show(output_format, **kwargs): @doc_group.command("delete") @doc_name_option @doc_id_option -@click.option("--yes", is_flag=True, help="Delete without confirmation") -@click.option("--force", is_flag=True, help="Allow deletion when the document contains entries") +@click.option( + "--force", is_flag=True, + help="Delete without confirmation, including documents that contain entries", +) @common_options def doc_delete(output_format, **kwargs): """Delete a log document.""" + if kwargs.get("force"): + set_no_prompt() cmd_delete_doc(fmt=output_format, **kwargs) @@ -234,7 +243,7 @@ def entry_attachment_group(): @entry_id_option() @click.option("--file", "-f", required=True, type=click.Path(path_type=str), help="File to attach") -@common_options +@prompting_options def entry_attachment_add(output_format, **kwargs): """Upload an attachment to an existing log entry.""" cmd_add_attachment(fmt=output_format, **kwargs) @@ -258,10 +267,12 @@ def entry_attachment_list(output_format, **kwargs): "--attachment-id", required=True, type=int, shell_complete=complete_attachment_ids, help="Numeric attachment ID", ) -@click.option("--yes", is_flag=True, help="Delete without confirmation") +@click.option("--force", is_flag=True, help="Delete without confirmation") @common_options def entry_attachment_delete(output_format, **kwargs): """Delete an attachment from a log entry.""" + if kwargs.get("force"): + set_no_prompt() cmd_delete_attachment(fmt=output_format, **kwargs) @@ -277,7 +288,7 @@ def entry_attachment_delete(output_format, **kwargs): @click.option("--text", "-t", default=None, help="Inline text for the entry") @click.option("--add-attachment", default=None, type=click.Path(path_type=str), help="File to attach to the entry") -@common_options +@prompting_options def entry_add(output_format, **kwargs): """Add a new log entry to an existing document.""" if kwargs.get('file') == '-': @@ -297,7 +308,7 @@ def entry_add(output_format, **kwargs): @click.option("--text", "-t", default=None, help="Inline reply text") @click.option("--add-attachment", default=None, type=click.Path(path_type=str), help="File to attach to the reply") -@common_options +@prompting_options def entry_reply(output_format, **kwargs): """Reply to an existing log entry.""" if kwargs.get('file') == '-': @@ -314,7 +325,7 @@ def entry_reply(output_format, **kwargs): @click.option("--text", "-t", default=None, help="Inline text for the entry") @click.option("--add-attachment", default=None, type=click.Path(path_type=str), help="File to attach to the entry") -@common_options +@prompting_options def entry_update(output_format, **kwargs): """Update an existing log entry or reply.""" if kwargs.get('file') == '-': @@ -352,10 +363,12 @@ def entry_get(output_format, **kwargs): @doc_name_option @doc_id_option @entry_id_option(help="Log entry or reply ID") -@click.option("--yes", is_flag=True, help="Delete without confirmation") +@click.option("--force", is_flag=True, help="Delete without confirmation") @common_options def entry_delete(output_format, **kwargs): """Delete a log entry or reply.""" + if kwargs.get("force"): + set_no_prompt() cmd_delete_entry(fmt=output_format, **kwargs) @@ -369,7 +382,7 @@ def entry_delete(output_format, **kwargs): # Bare `bely-cli tui` launches the full interactive app, so the group itself # is a leaf invocation when no subcommand is given -- the one place -# --format/--no-prompt sit on a group rather than a leaf command. +# --format sits on a group rather than a leaf command. @cli.group("tui", cls=AliasHelpGroup, invoke_without_command=True) @click.option("--limit", default=100, type=int, help="Recent documents to load per logbook (default 100)") diff --git a/tools/developer_tools/bely-cli/src/bely_cli/commands.py b/tools/developer_tools/bely-cli/src/bely_cli/commands.py index d5d47c0b8..6aaf1524a 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/commands.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/commands.py @@ -214,7 +214,7 @@ def cmd_show_doc(doc_name, doc_id, fmt="text"): print(f"{labels[key]}: {value}") -def cmd_delete_doc(doc_name, doc_id, yes=False, force=False, fmt="text"): +def cmd_delete_doc(doc_name, doc_id, force=False, fmt="text"): """Delete a log document after validation and confirmation.""" factory = auth.get_factory() logbook_api = factory.get_logbook_api() @@ -224,7 +224,7 @@ def cmd_delete_doc(doc_name, doc_id, yes=False, force=False, fmt="text"): raise ValueError( f'document "{doc.name}" contains {len(entries)} entries; use --force to delete it') result = {"doc_id": doc.id, "doc": doc.name, "status": "cancelled"} - if not confirm_delete(f'Delete document "{doc.name}" (id={doc.id})?', yes): + if not confirm_delete(f'Delete document "{doc.name}" (id={doc.id})?', force): print_result(result, "Deletion cancelled.", fmt) return result with auth.get_authenticated_factory() as auth_factory: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/common.py b/tools/developer_tools/bely-cli/src/bely_cli/common.py index 7a7a4202b..dd03a0f71 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/common.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/common.py @@ -25,12 +25,10 @@ def is_no_prompt(): return _no_prompt -def confirm_delete(prompt, yes=False): - """Confirm a destructive operation, respecting non-interactive mode.""" - if yes: +def confirm_delete(prompt, force=False): + """Confirm a destructive operation unless forced.""" + if force: return True - if is_no_prompt(): - raise ValueError("--yes is required with --no-prompt for deletion") return input(f"{prompt} [y/N] ").strip().lower() in ("y", "yes") diff --git a/tools/developer_tools/bely-cli/src/bely_cli/entry.py b/tools/developer_tools/bely-cli/src/bely_cli/entry.py index 12b94482a..4385554e5 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/entry.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/entry.py @@ -207,7 +207,7 @@ def cmd_add_attachment(doc_name, doc_id, entry_id, file, fmt="text"): print_result({"doc": doc.name, "log_id": entry_id, "attachment": info}, "", fmt) -def cmd_delete_attachment(doc_name, doc_id, entry_id, attachment_id, yes=False, fmt="text"): +def cmd_delete_attachment(doc_name, doc_id, entry_id, attachment_id, force=False, fmt="text"): """Delete an entry attachment by numeric ID after confirmation.""" factory = auth.get_factory() logbook_api = factory.get_logbook_api() @@ -223,7 +223,7 @@ def cmd_delete_attachment(doc_name, doc_id, entry_id, attachment_id, yes=False, "attachment_id": attachment_id, "status": "cancelled", } if not confirm_delete( - f'Delete attachment {attachment_id} from entry {entry_id}?', yes): + f'Delete attachment {attachment_id} from entry {entry_id}?', force): print_result(result, "Deletion cancelled.", fmt) return result with auth.get_authenticated_factory() as auth_factory: @@ -253,7 +253,7 @@ def cmd_list_attachments(doc_name, doc_id, entry_id, fmt="text"): print_items(items, columns, fmt) -def cmd_delete_entry(doc_name, doc_id, entry_id, yes=False, fmt="text"): +def cmd_delete_entry(doc_name, doc_id, entry_id, force=False, fmt="text"): """Delete a log entry or reply after confirmation.""" factory = auth.get_factory() logbook_api = factory.get_logbook_api() @@ -268,7 +268,7 @@ def cmd_delete_entry(doc_name, doc_id, entry_id, yes=False, fmt="text"): "status": "cancelled", } if not confirm_delete( - f'Delete entry/reply {entry_id} from document "{doc.name}"?', yes): + f'Delete entry/reply {entry_id} from document "{doc.name}"?', force): print_result(result, "Deletion cancelled.", fmt) return result with auth.get_authenticated_factory() as auth_factory: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/attachments.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/attachments.py index 575f8bc1d..3aeeabbfa 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/attachments.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/attachments.py @@ -462,7 +462,8 @@ def action_delete(self): async def _delete(self, attachment): confirmed = await self.app.push_screen_wait(ConfirmScreen( f'Delete attachment "{attachment.original_filename}" (id={attachment.id})?', - confirm_label="Delete", cancel_label="Cancel", confirm_variant="error")) + confirm_label="Delete", cancel_label="Cancel", confirm_variant="error", + confirm_shortcut="y")) if not confirmed: return api = await self.app.ensure_auth() diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py index f9b2fb0e3..116bed36c 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py @@ -10,7 +10,7 @@ with the current user's recently modified documents (core.recent_documents) instead of drilling in from logbook types. -Escape at the top level pops back to whatever pushed this screen, or (when `root=True`) does nothing -- `q` quits instead. +Escape at the top level pops back to whatever pushed this screen, or (when `root=True`) does nothing -- `ctrl+q` quits instead. All three levels (types/docs/entries) render as one #nav-table DataTable, so navigation, filtering, and the info/full-width toggles share a single code @@ -106,7 +106,8 @@ class BrowseScreen(Screen): BINDINGS = [ Binding("escape", "back", "Back"), Binding("backspace", "back", "Back", show=False), - Binding("q", "quit_app", "Quit"), + Binding("ctrl+q", "quit_app", "Quit"), + Binding("q", "quit_hint", "", show=False), Binding("question_mark", "show_shortcuts", "Shortcuts"), Binding("slash", "focus_filter", "Filter"), Binding("f", "toggle_full", "Full"), @@ -671,7 +672,10 @@ def _exit_top(self): if not self.root: self.app.pop_screen() return - self.notify("Press q to quit.") + self.notify("Press Ctrl+Q to quit.") + + def action_quit_hint(self): + self.notify("Press Ctrl+Q to quit.") def action_quit_app(self): self.app.exit(None) @@ -803,7 +807,8 @@ async def _delete_document(self, doc): confirmed = await self.app.push_screen_wait(ConfirmScreen( f'Delete document "{doc.name}"?', - confirm_label="Delete", cancel_label="Cancel", confirm_variant="error")) + confirm_label="Delete", cancel_label="Cancel", confirm_variant="error", + confirm_shortcut="y")) if not confirmed: return try: @@ -846,7 +851,8 @@ async def _delete_entry(self, entry): confirmed = await self.app.push_screen_wait(ConfirmScreen( f"Delete entry/reply #{entry.log_id}?", - confirm_label="Delete", cancel_label="Cancel", confirm_variant="error")) + confirm_label="Delete", cancel_label="Cancel", confirm_variant="error", + confirm_shortcut="y")) if not confirmed: return api = await self.app.ensure_auth() @@ -945,6 +951,7 @@ async def _edit_entry_externally(self, entry): f"Save changes to entry #{entry.log_id}?", confirm_label="Save", cancel_label="Discard", + confirm_shortcut="ctrl+s", ) ) if not save: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py index f43bb34fb..a12749402 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py @@ -109,7 +109,7 @@ async def _confirm_discard(self): discard = await self.app.push_screen_wait( ConfirmScreen( "Discard unsaved changes?", confirm_label="Discard", cancel_label="Keep editing", - confirm_variant="error", + confirm_variant="error", confirm_shortcut="y", ) ) if discard: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/confirm.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/confirm.py index 5274fc2e4..203432c25 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/confirm.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/confirm.py @@ -7,10 +7,11 @@ """ from textual.app import ComposeResult +from textual.binding import Binding from textual.containers import Vertical from textual.widgets import Button, Input, Static -from .dialog import CANCEL_HINT, DialogButtons, DialogScreen, hinted_label +from .dialog import CANCEL_HINT, SAVE_HINT, DialogButtons, DialogScreen, hinted_label class ConfirmScreen(DialogScreen): @@ -20,22 +21,35 @@ class ConfirmScreen(DialogScreen): } """ + BINDINGS = [ + *DialogScreen.BINDINGS, + Binding("y", "semantic_confirm('y')", "Confirm", show=False), + Binding("n", "semantic_cancel", "Cancel", show=False), + Binding("ctrl+s", "semantic_confirm('ctrl+s')", "Save", show=False), + ] BUTTON_ROWS = [["confirm-confirm", "confirm-cancel"]] def __init__(self, message, *, confirm_label="Yes", cancel_label="No", - confirm_variant="primary"): + confirm_variant="primary", confirm_shortcut=None): super().__init__() self.message = message self.confirm_label = confirm_label self.cancel_label = cancel_label self.confirm_variant = confirm_variant + self.confirm_shortcut = confirm_shortcut def compose(self) -> ComposeResult: with Vertical(id="confirm-dialog", classes="dialog"): yield Static(self.message, id="confirm-message") with DialogButtons(): - yield Button(hinted_label(self.confirm_label), variant=self.confirm_variant, id="confirm-confirm") - yield Button(hinted_label(self.cancel_label, CANCEL_HINT), id="confirm-cancel") + confirm_hint = SAVE_HINT if self.confirm_shortcut == "ctrl+s" else ( + "Y" if self.confirm_shortcut == "y" else None) + cancel_hint = "N" if self.confirm_shortcut == "y" else CANCEL_HINT + yield Button( + hinted_label(self.confirm_label, confirm_hint), + variant=self.confirm_variant, id="confirm-confirm") + yield Button( + hinted_label(self.cancel_label, cancel_hint), id="confirm-cancel") def on_mount(self): # Destructive confirmations default focus to "cancel" rather than the risky action. @@ -48,6 +62,14 @@ def on_button_pressed(self, event): elif event.button.id == "confirm-cancel": self.dismiss(False) + def action_semantic_confirm(self, shortcut): + if shortcut == self.confirm_shortcut: + self.dismiss(True) + + def action_semantic_cancel(self): + if self.confirm_shortcut == "y": + self.dismiss(False) + def action_cancel(self): self.dismiss(False) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py index 91e8aa5a9..26a69edbd 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py @@ -226,6 +226,7 @@ async def _post_create(self, api, doc): ConfirmScreen( f"Template generated log entry #{entry.log_id}. Edit it now?", confirm_label="Edit now", cancel_label="Leave as-is", + confirm_shortcut="y", ) ) if edit_now: @@ -233,7 +234,9 @@ async def _post_create(self, api, doc): self.app, doc, api, entry=entry, factory=self.session.factory) else: create_entry = await self.app.push_screen_wait( - ConfirmScreen("Create a log entry now?", confirm_label="Create entry", cancel_label="Skip") + ConfirmScreen( + "Create a log entry now?", confirm_label="Create entry", + cancel_label="Skip", confirm_shortcut="y") ) if create_entry: await open_composer(self.app, doc, api, factory=self.session.factory) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/shortcuts.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/shortcuts.py index a77664aba..e41c39690 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/shortcuts.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/shortcuts.py @@ -61,7 +61,7 @@ class Shortcut(NamedTuple): Shortcut("Space", "Toggle an item in a multi-select picker", "Dialogs and editing", ("dialogs",)), Shortcut("?", "Open or close keyboard shortcuts", "Application", ("types", "docs", "entries")), Shortcut("Ctrl+P", "Open the command palette", "Application", ("types", "docs", "entries")), - Shortcut("q", "Quit", "Application", ("types", "docs", "entries")), + Shortcut("Ctrl+Q", "Quit", "Application", ("types", "docs", "entries")), ) diff --git a/tools/developer_tools/bely-cli/test/test_cli_aliases.py b/tools/developer_tools/bely-cli/test/test_cli_aliases.py index a9f0a1c3d..b33dbc028 100644 --- a/tools/developer_tools/bely-cli/test/test_cli_aliases.py +++ b/tools/developer_tools/bely-cli/test/test_cli_aliases.py @@ -1,7 +1,7 @@ """CLI alias registration and help behavior.""" import unittest -from unittest.mock import patch +from unittest.mock import call, patch from click.testing import CliRunner @@ -10,8 +10,8 @@ class CliAliasTests(unittest.TestCase): ALIASES = { - "doc": {"ls": "list", "add": "new"}, - "entry": {"ls": "list", "show": "get", "edit": "update"}, + "doc": {"ls": "list", "add": "new", "rm": "delete"}, + "entry": {"ls": "list", "show": "get", "edit": "update", "rm": "delete"}, "config": {"ls": "show"}, } @@ -39,8 +39,8 @@ def test_alias_help_matches_canonical_help(self): def test_group_help_combines_aliases(self): runner = CliRunner() expected = { - "doc": ("add, new", "list, ls"), - "entry": ("edit, update", "get, show", "list, ls"), + "doc": ("add, new", "delete, rm", "list, ls"), + "entry": ("delete, rm", "edit, update", "get, show", "list, ls"), "config": ("ls, show",), } for group_name, labels in expected.items(): @@ -78,6 +78,43 @@ def test_entry_show_stdout_invokes_handler(self): output_dir=None, stdout=True, ) + def test_delete_force_invokes_handler(self): + runner = CliRunner() + with patch("bely_cli.cli.cmd_delete_entry") as command, \ + patch("bely_cli.cli.set_no_prompt") as non_interactive: + result = runner.invoke(cli, [ + "entry", "delete", "--doc-id", "99", "--id", "42", "--force", + ]) + self.assertEqual(result.exit_code, 0) + self.assertEqual(non_interactive.call_args_list, [call(False), call()]) + command.assert_called_once_with( + fmt="text", doc_id=99, doc_name=None, entry_id=42, force=True) + + def test_no_prompt_only_appears_on_prompting_commands(self): + runner = CliRunner() + prompting = [ + ("auth", "login"), ("doc", "new"), ("doc", "list"), + ("entry", "add"), ("entry", "reply"), ("entry", "update"), + ("entry", "attachment", "add"), + ] + non_prompting = [ + ("auth", "logout"), ("auth", "verify"), ("doc", "show"), + ("doc", "delete"), ("entry", "list"), ("entry", "get"), + ("entry", "delete"), ("entry", "attachment", "list"), + ("entry", "attachment", "delete"), ("config", "show"), + ("config", "set"), ("tui",), ("tui", "lookup"), + ] + for command in prompting: + with self.subTest(command=command): + result = runner.invoke(cli, [*command, "--help"]) + self.assertEqual(result.exit_code, 0) + self.assertIn("--no-prompt", result.output) + for command in non_prompting: + with self.subTest(command=command): + result = runner.invoke(cli, [*command, "--help"]) + self.assertEqual(result.exit_code, 0) + self.assertNotIn("--no-prompt", result.output) + def test_reply_invokes_handler(self): runner = CliRunner() with patch("bely_cli.cli.cmd_reply_entry") as command: diff --git a/tools/developer_tools/bely-cli/test/test_commands.py b/tools/developer_tools/bely-cli/test/test_commands.py index 30477626c..9df1fe7a5 100644 --- a/tools/developer_tools/bely-cli/test/test_commands.py +++ b/tools/developer_tools/bely-cli/test/test_commands.py @@ -149,14 +149,14 @@ def setUp(self): self.auth_ctx.__enter__.return_value = self.auth_factory self.auth_ctx.__exit__.return_value = False - def test_delete_yes_returns_structured_result(self): + def test_force_returns_structured_result(self): self.api.get_log_entries.return_value = [] with patch.object(commands.auth, "get_factory", return_value=self.factory), \ patch.object(commands.auth, "get_authenticated_factory", return_value=self.auth_ctx), \ patch("bely_cli.shell_completion.remove_cached_document") as remove_cached: buf = io.StringIO() with redirect_stdout(buf): - commands.cmd_delete_doc("My Doc", None, yes=True, fmt="json") + commands.cmd_delete_doc("My Doc", None, force=True, fmt="json") self.assertEqual(__import__("json").loads(buf.getvalue())["status"], "deleted") self.api.delete_log_document.assert_called_once_with(log_document_id=42) remove_cached.assert_called_once_with(42) @@ -176,23 +176,17 @@ def test_nonempty_requires_force(self): self.api.get_log_entries.return_value = [SimpleNamespace(log_id=1)] with patch.object(commands.auth, "get_factory", return_value=self.factory): with self.assertRaisesRegex(ValueError, "--force"): - commands.cmd_delete_doc("My Doc", None, yes=True) + commands.cmd_delete_doc("My Doc", None) - def test_force_does_not_imply_yes(self): + def test_force_deletes_nonempty_document_without_prompt(self): self.api.get_log_entries.return_value = [SimpleNamespace(log_id=1)] with patch.object(commands.auth, "get_factory", return_value=self.factory), \ - patch.object(commands.auth, "get_authenticated_factory") as authenticated, \ - patch("builtins.input", return_value="n"): + patch.object(commands.auth, "get_authenticated_factory", return_value=self.auth_ctx), \ + patch("builtins.input") as prompt, \ + patch("bely_cli.shell_completion.remove_cached_document"): commands.cmd_delete_doc("My Doc", None, force=True) - authenticated.assert_not_called() - - def test_no_prompt_requires_yes(self): - self.api.get_log_entries.return_value = [] - with patch.object(commands.auth, "get_factory", return_value=self.factory), \ - patch.object(commands, "is_no_prompt", return_value=True), \ - patch("bely_cli.common.is_no_prompt", return_value=True): - with self.assertRaisesRegex(ValueError, "--yes"): - commands.cmd_delete_doc("My Doc", None) + prompt.assert_not_called() + self.api.delete_log_document.assert_called_once_with(log_document_id=42) class CmdNewDocTests(unittest.TestCase): diff --git a/tools/developer_tools/bely-cli/test/test_entry.py b/tools/developer_tools/bely-cli/test/test_entry.py index b23db8618..93434cd41 100644 --- a/tools/developer_tools/bely-cli/test/test_entry.py +++ b/tools/developer_tools/bely-cli/test/test_entry.py @@ -330,7 +330,7 @@ def test_delete_reply(self): try: buf = io.StringIO() with redirect_stdout(buf): - entry.cmd_delete_entry("My Doc", None, 11, yes=True, fmt="json") + entry.cmd_delete_entry("My Doc", None, 11, force=True, fmt="json") finally: for patcher in patches: patcher.stop() @@ -349,7 +349,7 @@ def test_delete_attachment_by_numeric_id(self): try: buf = io.StringIO() with redirect_stdout(buf): - entry.cmd_delete_attachment("My Doc", None, 10, 7, yes=True, fmt="json") + entry.cmd_delete_attachment("My Doc", None, 10, 7, force=True, fmt="json") finally: for patcher in patches: patcher.stop() @@ -366,9 +366,9 @@ def test_missing_entry_and_attachment_fail(self): patcher.start() try: with self.assertRaisesRegex(ValueError, "entry with log_id=99 not found"): - entry.cmd_delete_entry("My Doc", None, 99, yes=True) + entry.cmd_delete_entry("My Doc", None, 99, force=True) with self.assertRaisesRegex(ValueError, "attachment id=99 not found"): - entry.cmd_delete_attachment("My Doc", None, 10, 99, yes=True) + entry.cmd_delete_attachment("My Doc", None, 10, 99, force=True) finally: for patcher in patches: patcher.stop() diff --git a/tools/developer_tools/bely-cli/test/test_tui_app.py b/tools/developer_tools/bely-cli/test/test_tui_app.py index 4ca2fcad3..039457e78 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_app.py +++ b/tools/developer_tools/bely-cli/test/test_tui_app.py @@ -463,7 +463,7 @@ async def test_entry_shortcut_modal_knows_document_has_replies(self): self.assertTrue(app.screen.has_replies) async def test_escape_at_landing_screen_does_not_quit(self): - # BrowseScreen is the landing screen in mode="app" too; only `q` quits. + # BrowseScreen is the landing screen in mode="app" too; only `ctrl+q` quits. data = LogbookData(FakeLogbookApi()) app = BelyTuiApp(FakeSession(data), limit=10, mode="app") async with app.run_test() as pilot: @@ -477,7 +477,7 @@ async def test_escape_at_landing_screen_does_not_quit(self): self.assertEqual(len(app._notifications), 1) async def test_escape_at_lookup_landing_screen_does_not_quit(self): - # tui lookup's landing screen behaves the same as tui's -- q is the only quit key. + # tui lookup's landing screen behaves the same as tui's -- ctrl+q is the only quit key. data = LogbookData(FakeLogbookApi()) app = BelyTuiApp(FakeSession(data), limit=10, mode="lookup") async with app.run_test() as pilot: @@ -489,13 +489,25 @@ async def test_escape_at_lookup_landing_screen_does_not_quit(self): self.assertIs(app.screen, landing) self.assertTrue(app.is_running) - async def test_q_quits_the_app(self): + async def test_q_shows_quit_hint_without_quitting(self): data = LogbookData(FakeLogbookApi()) app = BelyTuiApp(FakeSession(data), limit=10, mode="app") async with app.run_test() as pilot: await pilot.pause() await pilot.press("q") await pilot.pause() + self.assertTrue(app.is_running) + self.assertEqual(len(app._notifications), 1) + self.assertEqual( + next(iter(app._notifications)).message, "Press Ctrl+Q to quit.") + + async def test_ctrl_q_quits_the_app(self): + data = LogbookData(FakeLogbookApi()) + app = BelyTuiApp(FakeSession(data), limit=10, mode="app") + async with app.run_test() as pilot: + await pilot.pause() + await pilot.press("ctrl+q") + await pilot.pause() self.assertIsNone(app.return_value) async def test_command_palette_offers_config_recent_and_logout_when_authenticated(self): diff --git a/tools/developer_tools/bely-cli/test/test_tui_screens.py b/tools/developer_tools/bely-cli/test/test_tui_screens.py index 4869321d6..e13bd68a5 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_screens.py +++ b/tools/developer_tools/bely-cli/test/test_tui_screens.py @@ -1333,6 +1333,38 @@ async def test_escape_dismisses_false(self): result = await task.wait() self.assertFalse(result) + async def test_yes_shortcut_confirms_without_moving_focus(self): + app = App() + async with app.run_test() as pilot: + task = app.run_worker(app.push_screen_wait( + ConfirmScreen("Delete it?", confirm_variant="error", confirm_shortcut="y"))) + await pilot.pause() + self.assertIs(app.screen.focused, app.screen.query_one("#confirm-cancel", Button)) + await pilot.press("y") + self.assertTrue(await task.wait()) + + async def test_no_shortcut_cancels_without_moving_focus(self): + app = App() + async with app.run_test() as pilot: + task = app.run_worker(app.push_screen_wait( + ConfirmScreen("Delete it?", confirm_shortcut="y"))) + await pilot.pause() + self.assertIs(app.screen.focused, app.screen.query_one("#confirm-confirm", Button)) + await pilot.press("n") + self.assertFalse(await task.wait()) + + async def test_ctrl_s_confirms_save_semantics(self): + app = App() + async with app.run_test() as pilot: + task = app.run_worker(app.push_screen_wait( + ConfirmScreen("Save it?", confirm_label="Save", confirm_shortcut="ctrl+s"))) + await pilot.pause() + await pilot.press("y") + await pilot.pause() + self.assertFalse(task.is_finished) + await pilot.press("ctrl+s") + self.assertTrue(await task.wait()) + async def test_default_focus_is_confirm(self): app = App() async with app.run_test() as pilot: