Skip to content

feat: Show friendly help when run with no arguments#52

Open
HuiNeng6 wants to merge 1 commit intosystemblueteam:mainfrom
HuiNeng6:feat/friendly-help-no-args
Open

feat: Show friendly help when run with no arguments#52
HuiNeng6 wants to merge 1 commit intosystemblueteam:mainfrom
HuiNeng6:feat/friendly-help-no-args

Conversation

@HuiNeng6
Copy link
Copy Markdown

Improve CLI user experience for first-time users.

Changes:

  • Make file_a and file_b optional arguments
  • Show full help when no args provided (exit 0, not error)
  • Show clear message when only one file provided
  • Add file existence validation with helpful context

Fixes: #49

- Make file_a and file_b optional arguments
- Show full help when no args provided (exit 0)
- Show clear message when only one file provided
- Add file existence validation with helpful paths

Fixes: systemblueteam#49
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

📝 Walkthrough

Walkthrough

The CLI argument handling is refactored to make file_a and file_b optional, removing Click's built-in path validation. Manual runtime validation is introduced with custom error messages for missing arguments, incomplete input, and non-existent files using Rich-colored output. Help text displays on zero arguments; errors display on incomplete or invalid input.

Changes

Cohort / File(s) Summary
CLI Argument Validation
src/sounddiff/cli.py
Made arguments optional, replaced Click path existence validation with manual checks. Added conditional flows: show help on no arguments (exit 0), error on one argument or missing files (exit 1). Introduced Rich Console for styled error messages including parent directory context.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hopped and did declare,
"When users stumble, we must care!
No cryptic errors, cold and bare—
Just friendly words that show we're there.
Two files needed, or help displayed,
A welcoming path, perfectly made!" 🎵

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses objectives 1 and 3 from issue #49 (no args help display, file existence validation). Objectives 2 and 4 (one file message, wrong file type) appear incomplete based on the code summary. Implement clear messaging when only one file is provided and add validation for wrong file types before attempting to load audio.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: improving CLI help display when no arguments are provided.
Description check ✅ Passed The description is directly related to the changeset, outlining the key improvements and linking to issue #49.
Out of Scope Changes check ✅ Passed All changes are directly related to improving CLI argument handling and validation, staying within the scope of issue #49.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/sounddiff/cli.py (1)

127-135: 🧹 Nitpick | 🔵 Trivial

Consider consistent Rich formatting for error messages.

The new validation code uses Rich's console.print with colored output, but these exception handlers still use plain click.echo. This creates inconsistent error styling for users.

Additionally, PR objective #4 (wrong file type) requested clearer messages before lower-level errors. The ValueError handler at lines 130-132 catches format errors from load_audio(), but passes through the raw exception message without Rich formatting.

♻️ Optional refactor for consistent Rich formatting
     except FileNotFoundError as e:
-        click.echo(f"Error: {e}", err=True)
+        console.print(f"[red]Error:[/red] {e}")
         sys.exit(1)
     except ValueError as e:
-        click.echo(f"Error: {e}", err=True)
+        console.print(f"[red]Error:[/red] {e}")
         sys.exit(1)
     except RuntimeError as e:
-        click.echo(f"Error: {e}", err=True)
+        console.print(f"[red]Error:[/red] {e}")
         sys.exit(1)

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: efdec335-1cfb-4a3b-a977-66eda1505e34

📥 Commits

Reviewing files that changed from the base of the PR and between e3a4c78 and c32e555.

📒 Files selected for processing (1)
  • src/sounddiff/cli.py

Compares FILE_A (reference) against FILE_B (comparison) and reports
differences in loudness, spectral content, timing, and potential issues.
"""
console = Console()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

The error console doesn't respect --no-color.

The Console() instance created here will always output colors, ignoring the --no-color flag. The progress console at line 120 correctly passes no_color=no_color.

🐛 Proposed fix
-    console = Console()
+    console = Console(no_color=no_color)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console = Console()
console = Console(no_color=no_color)

@systemblueteam
Copy link
Copy Markdown
Owner

Console() on line 68 doesn't pass no_color=no_color, so --no-color gets ignored for these error messages. Line 120 already does it right. We fixed this same thing in #40.

@systemblueteam
Copy link
Copy Markdown
Owner

path.exists() returns True for directories too. If someone passes a directory by mistake they'll get a confusing soundfile error instead of the friendly message. path.is_file() is the fix for both checks.

Copy link
Copy Markdown
Owner

@systemblueteam systemblueteam left a comment

Choose a reason for hiding this comment

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

Two small fixes needed, see comments above.

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.

Show friendly help when run with no arguments

2 participants