Skip to content

fix: make --quiet and --silent flags work across all commands#782

Open
nkakouros wants to merge 2 commits intojdx:mainfrom
nkakouros:worktree-fix-quiet-silent
Open

fix: make --quiet and --silent flags work across all commands#782
nkakouros wants to merge 2 commits intojdx:mainfrom
nkakouros:worktree-fix-quiet-silent

Conversation

@nkakouros
Copy link
Copy Markdown
Contributor

Summary

  • Replace direct println!() with info!() in install, init, and migrate commands so output respects log level filtering
  • Use ProgressOutput::Quiet (new clx variant) instead of ProgressOutput::Text for --quiet/--silent, fully suppressing
    progress output
  • Guard stats() and step output summary with quiet/silent settings checks
  • Update --quiet/--silent help text and settings.toml docs to accurately describe behavior
  • Bump clx and ensembler submodules to latest, is this ok?
  • Add integration tests for quiet/silent on install, check, and init

Depends on jdx/clx#80

Test plan

  • mise run build — compiles clean
  • mise run test:cargo — all tests pass
  • mise run test:bats test/quiet_silent.bats — 5/5 pass across all 3 variants (libgit2, nolibgit2, nogit)
  • Manual verification:
    • hk install --quiet / --silent — no output
    • hk check --quiet / --silent — no progress or info output
    • hk init --force --quiet — no output
    • hk version — still prints (output IS the purpose)

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes output suppression by migrating println! calls to info! logs in several commands, updating flag documentation, and adding integration tests. A review comment suggests replacing explicit output guards in src/hook.rs with info! calls to improve consistency with the rest of the PR's logging strategy.

Comment thread src/hook.rs
@nkakouros nkakouros force-pushed the worktree-fix-quiet-silent branch 2 times, most recently from 17f7bf8 to ac1f982 Compare March 29, 2026 12:34
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 29, 2026

Greptile Summary

This PR makes --quiet and --silent flags effective across install, init, migrate, and hook-run commands by replacing direct println!() calls with info!(), switching to the new ProgressOutput::Quiet clx variant, and adding explicit quiet/silent guards on stats() and the output_by_step summary display. Integration tests cover install, check, and init; the submodule bumps (clx 0.3.2 → 1.3.0, ensembler 0.3.0 → 1.1.0) also allow removal of tracing-appender and crossbeam-channel.

Confidence Score: 5/5

  • Safe to merge; the flag handling is correct and all remaining findings are minor test-coverage suggestions.
  • All P0/P1 concerns from the previous review threads are addressed. The implementation correctly layers ProgressOutput::Quiet over log-level filtering, the guards in stats() and output_by_step are logically sound, and the build + test suite passes. The only open finding is a P2 test-coverage gap where --silent and --quiet share identical test bodies.
  • test/quiet_silent.bats — the check --silent test body is identical to check --quiet and doesn't verify warning suppression.

Important Files Changed

Filename Overview
src/cli/mod.rs Switches both --quiet and --silent to ProgressOutput::Quiet (new clx variant) and updates help text; flag handling order and clap overrides_with_all interactions are correct.
src/cli/install.rs Replaces println! with info! so "Installed hk hook" messages now respect log level; straightforward and correct.
src/cli/init/mod.rs Converts println! to info! throughout, including the blank-line separator which is now info!(""); correctly handles interactive and auto modes.
src/cli/migrate/pre_commit.rs Converts println! to info! for "Successfully migrated", "Next steps", and language version guidance; also fixes a println! for "Language versions detected".
src/hook.rs Adds early-return guard in stats() and adds !settings.quiet && !settings.silent guard for output_by_step summary display; both guards are correctly placed and logically sound.
test/quiet_silent.bats New integration tests cover install, check, and init commands, but the check --quiet and check --silent test bodies are identical — the warning-suppression difference of --silent vs --quiet is not tested.
test/trace.bats Removes assert_output --partial "hook.run" from four text-mode trace tests due to the ensembler/tracing-subscriber upgrade changing span rendering; the span is still verified in the JSON output test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CLI args parsed] --> B{--verbose?}
    B -->|yes| C[ProgressOutput::Text\nLogLevel::Debug or Trace]
    B -->|no| D{--quiet?}
    D -->|yes| E[ProgressOutput::Quiet\nLogLevel::Warn]
    D -->|no| F{--silent?}
    F -->|yes| G[ProgressOutput::Quiet\nLogLevel::Error]
    F -->|no| H{non-TTY or\n--no-progress?}
    H -->|yes| I[ProgressOutput::Text]
    H -->|no| J[ProgressOutput::Animated]
    E --> K[stats returns early\noutput_by_step skipped\ninfo! suppressed]
    G --> L[stats returns early\noutput_by_step skipped\ninfo! + warn! suppressed]
    I --> M[Normal text output]
    J --> M
Loading

Reviews (6): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile

Comment thread src/hook.rs
Comment thread src/cli/migrate/pre_commit.rs
Comment thread src/cli/init/mod.rs
@nkakouros nkakouros force-pushed the worktree-fix-quiet-silent branch 2 times, most recently from b90ba90 to c49009c Compare March 29, 2026 13:55
@nkakouros
Copy link
Copy Markdown
Contributor Author

The only open question is a P2 clarification about why four trace-test assertions were removed; the tests themselves still pass and no runtime path is broken.

hook.run span is verified in the JSON test; text-mode tracing does not render span names the same way after the ensembler/tracing-subscriber upgrade.

- Add ProgressOutput::Quiet variant to clx to suppress all progress output
  (spinners, status lines, text-mode updates)
- Use ProgressOutput::Quiet instead of ProgressOutput::Text for --quiet/--silent
- Replace direct println!() with info!() in install, init, and migrate commands
  so output is controlled by log level filtering
- Guard stats() and step output summary with quiet/silent settings checks
- Update help text and settings docs to accurately describe behavior
- Add integration tests for quiet/silent on install, check, and init

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nkakouros nkakouros force-pushed the worktree-fix-quiet-silent branch from 3074e53 to e05bf2e Compare April 14, 2026 13:04
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.

2 participants