Take the flag where it is typed, and stop the heading from lying
`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) <noreply@anthropic.com>
This commit is contained in:
1 parent
2a71284d7d
commit
b42b0a473e
4 files changed
+119
-4
No files matched your search
@@ -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
|
||||
|
||||
@@ -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"),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 -------------------------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user