From b42b0a473e7ffcb99646fd7e4deeb56adfc66d63 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Tue, 8 Sep 2026 04:21:54 -0400 Subject: [PATCH] Take the flag where it is typed, and stop the heading from lying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `meshterm contacts --json` is what a person types, and it failed with a bare usage error: Click binds an option to whatever command declares it, so a global on the callback is a parse error one word later. For an output format that is the wrong way round — `-o json` goes after the verb in every tool that has one, and --json is the first thing anyone reaches for. The group's own options are now lifted to the front before Click parses, so both orders mean the same run. `--` stops the lifting and --help is never lifted, since `contacts --help` must stay the contacts help rather than quietly becoming the program's. And a column headed AGE held `2026-09-08T04:18:25-04:00` under --absolute. Every other time column already names the fact — HEARD, RECORDED, FINISHED — which stays true whichever form the run asked for; only chat's named the form. `when()` now says so where the header is declared. Co-Authored-By: Claude Opus 5 (1M context) --- meshterm/cli.py | 66 ++++++++++++++++++++++++++++++++++++++ meshterm/tools/chat.py | 6 ++-- meshterm/ui/fields.py | 4 ++- tests/test_cli_contract.py | 47 +++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 4 deletions(-) diff --git a/meshterm/cli.py b/meshterm/cli.py index f51839a..23e6d72 100644 --- a/meshterm/cli.py +++ b/meshterm/cli.py @@ -15,6 +15,7 @@ from pathlib import Path import typer from rich.console import Console +from typer.core import TyperGroup from .context import AppContext from .core import exitcodes, win32dll @@ -43,7 +44,72 @@ EXIT_STATUS_EPILOG = "Exit status: " + " · ".join( f"{code} {meaning}" for code, meaning in exitcodes.MEANINGS.items() ) + +class _GlobalOptionsAnywhere(TyperGroup): + """A group whose own options may be typed after the subcommand as well as before. + + Click binds an option to whatever command declares it, so a global declared on the + callback is a parse error one word later: ``meshterm contacts --json`` failed with a + bare usage error while ``meshterm --json contacts`` worked. That is the wrong way + round for an output-format flag — ``-o json`` goes *after* the verb in every tool that + has one, and ``--json`` is the first thing anyone will reach for here. + + So the group's own options are lifted to the front before Click sees them, and both + orders mean the same run. ``--`` stops the lifting, which is the standard way to say + the rest is data; ``--help`` is deliberately never lifted, since ``contacts --help`` + must stay the *contacts* help rather than silently becoming the program's. + """ + + def parse_args(self, ctx: typer.Context, args: list[str]) -> list[str]: + """Lift this group's options out of ``args``, then parse as Click normally would.""" + return super().parse_args(ctx, _globals_first(self, args)) + + +def _globals_first(group: TyperGroup, args: list[str]) -> list[str]: + """``args`` with ``group``'s own options moved ahead of the subcommand. + + An option is told from an argument by carrying ``is_flag`` — Typer vendors Click, so + there is no importable ``click.Option`` to test against, and the attribute is the same + question asked without reaching into a private package. + + Args: + group: The command whose options count as global. + args: The argument list as typed. + + Returns: + A reordered list. The relative order of the options, and of everything else, is + preserved, so a repeated flag resolves exactly as Click would resolve it. + """ + takes_value: dict[str, bool] = {} + for param in group.params: + if getattr(param, "is_flag", None) is None or param.name == "help": + continue + for opt in (*param.opts, *param.secondary_opts): + takes_value[opt] = not param.is_flag + + lifted: list[str] = [] + kept: list[str] = [] + index = 0 + while index < len(args): + token = args[index] + if token == "--": + kept.extend(args[index:]) + break + name = token.split("=", 1)[0] + if name not in takes_value: + kept.append(token) + index += 1 + elif "=" in token or not takes_value[name] or index + 1 >= len(args): + lifted.append(token) + index += 1 + else: + lifted.extend(args[index : index + 2]) + index += 2 + return lifted + kept + + app = typer.Typer( + cls=_GlobalOptionsAnywhere, add_completion=False, no_args_is_help=False, # Click's own plain help, not Typer's rich one: no boxed Options/Commands panels, no diff --git a/meshterm/tools/chat.py b/meshterm/tools/chat.py index d792dd1..64273f0 100644 --- a/meshterm/tools/chat.py +++ b/meshterm/tools/chat.py @@ -659,7 +659,7 @@ def _picker_header(width: int) -> str: [ Lane("CONVERSATION", _LABEL_WIDTH + 2), Lane("UNREAD", _BADGE_WIDTH + 2), - Lane("AGE", _AGE_WIDTH + 2), + Lane("TIME", _AGE_WIDTH + 2), Lane(("LAST MESSAGE", "LAST MSG", "LAST")), ], width, @@ -856,7 +856,7 @@ def _message_columns() -> tuple: from ..ui import fields return ( - fields.when("created_at", "AGE"), + fields.when("created_at", "TIME"), fields.word("direction", "DIR"), # One column for whichever end the conversation had; the document keeps the two # apart, because a parser cannot tell a channel from a contact by its label. @@ -898,7 +898,7 @@ def _transcript(messages: list[ChatMessage]) -> Listing: key="messages", columns=_message_columns(), rows=rows, - order=("AGE", "DIR", "PEER", "SNR_DB", "TEXT"), + order=("TIME", "DIR", "PEER", "SNR_DB", "TEXT"), ) diff --git a/meshterm/ui/fields.py b/meshterm/ui/fields.py index c1b0835..831d628 100644 --- a/meshterm/ui/fields.py +++ b/meshterm/ui/fields.py @@ -250,7 +250,9 @@ def when(key: str, header: str, *, absent: str = script.NONE) -> Column: Args: key: The machine key. It ends in ``_at`` by convention — a JSON key is a type hint for a parser, and the suffix is the hint. - header: The plain heading. + header: The plain heading. It must name the *fact* — ``HEARD``, ``RECORDED`` — + rather than the form, because ``--absolute`` changes the form underneath it + and a column headed ``AGE`` holding an ISO instant is a heading that lies. absent: What an unknown time reads as plain. ``never`` where the row exists and the event has not happened (a node not yet heard, a conversation with nothing said in it); the default token where the row has no such time at all. diff --git a/tests/test_cli_contract.py b/tests/test_cli_contract.py index 199bd2d..a065eee 100644 --- a/tests/test_cli_contract.py +++ b/tests/test_cli_contract.py @@ -250,6 +250,53 @@ def test_the_sweep_actually_covers_the_whole_command_surface() -> None: assert expected in leaves, expected +def test_a_global_option_may_be_typed_after_the_subcommand(run) -> None: # noqa: ANN001 + """``meshterm contacts --json`` is what a person types, so it has to be what works. + + Click binds an option to whatever command declares it, so a global declared on the + callback is a parse error one word later. For an output format that is the wrong way + round — ``-o json`` goes after the verb in every tool that has one — and the failure + was a bare usage error with no hint that the flag simply needed moving. + """ + after = run("contacts", "--json") + before = run("--json", "contacts") + assert after.exit_code == exitcodes.OK, after.output + assert after.stdout == before.stdout + + +def test_a_global_option_taking_a_value_moves_with_its_value(run) -> None: # noqa: ANN001 + """Lifting ``--db`` to the front must take the path with it, not orphan it.""" + assert run("contacts", "--sort", "name", "--json").exit_code == exitcodes.OK + + +def test_lifting_a_global_never_steals_a_subcommands_own_help(run) -> None: # noqa: ANN001 + """``contacts --help`` must stay the *contacts* help, not silently become the app's. + + ``--help`` is on every command, so a rule that lifts "the group's options" would take + it too and answer a different question than the one asked. + """ + contacts_help = run("contacts", "--help").stdout + assert "contacts [OPTIONS]" in contacts_help + assert "--sort" in contacts_help # a contacts option, absent from the app's own help + app_help = run("--help").stdout + assert "COMMAND [ARGS]" in app_help + assert "--sort" not in app_help + + +def test_a_time_column_is_headed_by_its_fact_not_by_its_format(run) -> None: # noqa: ANN001 + """``--absolute`` changes what is under the heading, so the heading must survive it. + + A column headed ``AGE`` holding ``2026-09-08T04:18:25-04:00`` is a heading that lies — + and every other time column already names the fact (``HEARD``, ``RECORDED``), which + reads correctly whichever form the run asked for. + """ + run("chat", "send", "--to", "Alice", "hello") + relative = result_lines(run("chat", "history", "--to", "Alice"))[0].split() + absolute = result_lines(run("chat", "history", "--to", "Alice", "--absolute"))[0].split() + assert relative == absolute + assert "AGE" not in relative + + # -- listings -------------------------------------------------------------------------