Skip to content

cli: palinode blame, rollback and push print a Python dict instead of JSON when piped #210

Description

@Paul-Kyle

palinode/cli/_format.py provides emit_json and its docstring explains why machine-readable output must never be routed through rich.Console. Its closing line is "this makes that the single path."

Three commands in palinode/cli/git.py are not on that path. Each hands the raw API response dict to rich.Console via console.print(data):

  • blame — the branch taken without --claims
  • rollback
  • push

What goes wrong. console.print() on a dict renders a Python repr:

{'note': 'contains some text', 'n': None}

Single-quoted keys and None instead of null, so palinode blame FILE | jq does not get invalid JSON — it gets output that was never JSON.

A trap worth naming before you start. The tempting one-line fix is console.print(json.dumps(data)). Do not do that. Rich consumes [...] as style markup in a string, so a value containing [bold] or [git: abc] comes back with that substring silently deleted — output that parses cleanly and is wrong. (Printing the raw dict does not have this problem, which is why it is not a symptom today and would become one.) Route through emit_json / click.echo instead.

blame --claims needs the same treatment. It currently renders prose through format_claims_resolution whether or not stdout is a terminal, so it is equally unusable when piped. Wanted: JSON for both blame modes when stdout is not a TTY, and the existing interactive rendering unchanged in both.

The correct pattern is in this repository, twice.

  • palinode/cli/git.py:55history, the fourth command in the same file, already calls print_result(data, fmt=get_default_format()).
  • palinode/cli/depends.py — the fuller shape: resolve the format once, emit JSON on the JSON path, keep a hand-written human rendering on the other.

Please do not simply replace console.print(data) with emit_json(data). That would make palinode blame unreadable for the human who runs it at a terminal, which is its main use. What is wanted is the TTY-aware split.

Scope: palinode/cli/git.py and tests. history is already correct — leave it alone. No API changes; the response shapes are fine, only the printing is wrong.

Tests:

  • For blame (both with and without --claims), rollback and push: with a non-TTY stdout, the captured output parses with json.loads.
  • A value containing a [...] substring survives that JSON output intact. This is the assertion that pins the trap above shut.
  • At least one human-path test that the interactive rendering is unchanged — blame --claims is the one that matters, since its prose formatting is deliberate.

How to see it: run any of the three with stdout piped and try to parse the result.

Comment here to claim it. Please wait to be assigned before opening a PR — two contributors landed on one issue once because we had no such line.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinggood first issueGood for newcomers

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions