Skip to content

Warn that --output / --format are ignored under --tui (#220)#241

Open
cboos wants to merge 1 commit into
mainfrom
dev/tui-output-warn
Open

Warn that --output / --format are ignored under --tui (#220)#241
cboos wants to merge 1 commit into
mainfrom
dev/tui-output-warn

Conversation

@cboos

@cboos cboos commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

The last of the --output / --format / TUI group (#220#223). #221/#222/#223-pt1 landed in #237; #223-pt2 in #239.

Problem

In --tui mode, --output and --format were silently ignored — no effect, no warning. run_session_browser never receives them, and the TUI's export actions write to a fixed per-session path. A user running claude-code-log --tui --format markdown --output log.md got no log.md and no indication why.

Fix (minimal)

Warn (to stderr) when --output or --format is passed with --tui, mirroring the existing --expand-paths / --filter-path ignored-flag warnings. An explicit -f is detected via the Click parameter source, so the html default doesn't false-trigger.

Also gate the #222 format-inference/conflict block (added in #237) on not tui: those flags are no-ops under --tui, so hard-erroring on a -o x.md -f html conflict there would contradict the warning and block the TUI from launching. Under --tui we warn and proceed; the non-TUI conflict error is unchanged.

The in-app "export to path" action (the other half of #220) is intentionally deferred — left for a follow-up once this minimal warn lands.

Tests

test/test_tui_output_warn.py (6): warn on -o/explicit--f under --tui; no warn without them; warn on stderr not stdout; no warn for non-TUI -o; and -o x.md -f html --tui warns-and-proceeds (no conflict error). Driven via an empty --projects-dir so the TUI returns early without launching Textual — runs in the fast unit suite. just ci green.

Closes #220

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved TUI mode so output and format options are safely ignored instead of causing conflicts or errors.
    • Added clear warnings in the terminal when unsupported output settings are used in TUI mode.
    • Prevented accidental format detection from file names during TUI runs, reducing unexpected behavior.
  • Tests

    • Added coverage for TUI warning behavior, including supported and unsupported option combinations and correct warning placement.

In TUI mode, --output and --format had no effect and emitted no warning:
run_session_browser never receives them, and the TUI's export actions
write to a fixed per-session path. Now the CLI warns (to stderr) when
either is passed with --tui, mirroring the existing --expand-paths /
--filter-path ignored-flag warnings. An explicit -f is detected via the
Click parameter source so the html default doesn't false-trigger.

Also gate the #222 format-inference/conflict block on `not tui`: those
flags are no-ops under --tui (now warned), so erroring on a
`-o x.md -f html` conflict there would contradict the warning and block
the TUI from launching. Under --tui we warn and proceed.

This is the minimal half of #220; an in-app "export to path" action is a
possible follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI now warns when --output or explicit --format are used with --tui, and it skips output-suffix format inference in that mode. New tests cover the warning, stream routing, conflicting flags under TUI, and the non-TUI baseline.

Changes

TUI output flag warnings

Layer / File(s) Summary
CLI warning handling
claude_code_log/cli.py
The CLI warns on --output and explicit --format under --tui, and skips output-suffix format inference in that mode.
TUI warning tests
test/test_tui_output_warn.py
The tests cover the TUI warning cases, stderr/stdout routing, the conflicting -o/-f combination under TUI, and the non-TUI baseline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • daaain/claude-code-log#237: Updates the same CLI flag-handling path for --output/--format, including suffix-based format inference and conflict behavior.

Poem

A bunny hopped through TUI land,
With --output flags in paw and hand.
“If TUI’s on, I’ll warn you bright,
Then hop along and set things right.
No silent skips!” თქვა the rabbit, grand 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main behavior change: warning that --output/--format are ignored under --tui.
Linked Issues check ✅ Passed The PR meets #220 by warning when --output/--format are used with --tui and avoiding the former hard-error path.
Out of Scope Changes check ✅ Passed The CLI change and tests stay focused on the linked TUI flag-handling behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev/tui-output-warn

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/test_tui_output_warn.py`:
- Around line 90-91: The non-TUI baseline test using self.runner.invoke(main,
[str(src), "-o", str(out)]) only checks output and can miss failures, so update
this test to also assert r.exit_code == 0 after invoking main; keep the existing
output assertion alongside it so the test validates both successful execution
and the expected TUI-related message behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef284442-9f42-4f26-825c-18c0affab07b

📥 Commits

Reviewing files that changed from the base of the PR and between 4bd633f and 0a306e2.

📒 Files selected for processing (2)
  • claude_code_log/cli.py
  • test/test_tui_output_warn.py

Comment on lines +90 to +91
r = self.runner.invoke(main, [str(src), "-o", str(out)])
assert "ignored in --tui mode" not in r.output

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add an exit-code assertion in the non-TUI baseline test.

Line 90 should also assert r.exit_code == 0; otherwise this test can pass even when invocation fails for unrelated reasons.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/test_tui_output_warn.py` around lines 90 - 91, The non-TUI baseline test
using self.runner.invoke(main, [str(src), "-o", str(out)]) only checks output
and can miss failures, so update this test to also assert r.exit_code == 0 after
invoking main; keep the existing output assertion alongside it so the test
validates both successful execution and the expected TUI-related message
behavior.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--tui silently ignores --output/--format and has no "export to path" action

1 participant