chore: --pick-transcript-mode requires --pick-transcript #637
Conversation
WalkthroughThe changes involve an update to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Args
User->>CLI: Specify pick_transcript_mode
CLI->>Args: Check if pick_transcript is provided
alt pick_transcript is provided
Args-->>CLI: Accept input
else pick_transcript not provided
Args-->>CLI: Error: pick_transcript required
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/annotate/seqvars/mod.rs (1)
Line range hint
108-116: Consider adding documentation about the dependency requirement.While the code change is correct, it would be helpful to document this dependency in the field's documentation comment. This would make the requirement more explicit to users reading the code.
/// Determines how to handle multiple transcripts. Default is to keep all. /// /// When transcript picking is enabled via `--pick-transcript`, /// either keep the first one found or keep all that match. + /// + /// Note: This option requires --pick-transcript to be specified. #[arg(long, default_value = "all", requires = "pick_transcript")] pub pick_transcript_mode: TranscriptPickMode,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/annotate/seqvars/mod.rs(1 hunks)
🔇 Additional comments (2)
src/annotate/seqvars/mod.rs (2)
113-113: LGTM: The requires attribute correctly enforces the dependency.
The addition of requires = "pick_transcript" to the pick_transcript_mode field is correct and aligns with the PR objective. This ensures that users must specify --pick-transcript when using --pick-transcript-mode.
Line range hint 108-116: Verify the default value interaction with the requirement.
The field has both a default_value = "all" and requires = "pick_transcript". Let's verify that this combination works as expected and the default value doesn't bypass the requirement.
Also applies to: 73-77
✅ Verification successful
The requires = "pick_transcript" and default_value = "all" combination is safe and works as expected.
The code inspection reveals that:
TranscriptPickModeis an enum withFirstandAllvariants, whereFirstis marked as#[default]- The field's default value of
"all"will only be used when the--pick-transcriptflag is present (due torequires) - The implementation in
Providercorrectly handles both modes:TranscriptPickMode::Firstkeeps only the first matching transcriptTranscriptPickMode::Allkeeps all matching transcripts
The requires attribute ensures the option can only be used when pick_transcript is specified, regardless of the default value. This is a valid clap configuration pattern where the default value is only considered when the required dependency is satisfied.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any tests verifying the default value and requires interaction
rg -A 5 "pick_transcript_mode.*default_value.*requires" tests/
Length of output: 64
Script:
#!/bin/bash
# Let's check the TranscriptPickMode enum and its usage
ast-grep --pattern 'enum TranscriptPickMode'
# Also check for any tests related to pick_transcript_mode
rg "pick_transcript_mode" tests/
# Check for any CLI tests or integration tests
fd "test" --type f --exec rg -l "pick_transcript"
Length of output: 127
Script:
#!/bin/bash
# Let's check the definition of TranscriptPickMode and where it's used
rg -A 5 "TranscriptPickMode"
# Let's also check for any clap-related tests or configuration
rg -A 5 "clap::.*Parser"
# And check for any CLI argument parsing tests
rg -A 5 "#\[arg\(.*requires.*\)\]"
Length of output: 7776
Summary by CodeRabbit
This change improves the clarity and usability of the command-line interface.