Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #199 +/- ##
=======================================
Coverage ? 97.62%
=======================================
Files ? 8
Lines ? 379
Branches ? 135
=======================================
Hits ? 370
Misses ? 8
Partials ? 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kricsleo
changed the title
multiPositionalmultiPositional
|
/cc @pi0: this has been sitting since April, and I wanted to make sure it was on your radar. I'm working on a CLI app that could make use of this feature and found my way to this PR. |
Preserve multiPositional feature while adopting main's changes: - Remove number type (dropped in main) - Use colors.cyan instead of backtick formatting for multiPositional - Adopt satisfies ParseOptions pattern - Update inline snapshots Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new multiPositional argument type across the CLI: types, parser, usage rendering, tests, and an example command now accept and handle multiple positional values (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/usage.test.ts (1)
28-32: Redundantnameproperty.The
name: "pos"property appears to be unused here. The argument name is derived from the object key (multiPositional), as evidenced by the snapshot showingMULTIPOSITIONALin the output. This pattern exists in other args in this test (lines 25, 35, 41), so it's not introduced by this PR, but you could clean it up for consistency.🧹 Optional cleanup
multiPositional: { type: "multiPositional", - name: "pos", description: "Multi positional", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/usage.test.ts` around lines 28 - 32, Remove the redundant name property from the multiPositional arg object in the test (the object keyed as multiPositional in test/usage.test.ts); the arg name is derived from the key, so delete the line containing name: "pos" from the multiPositional object (mirroring the pattern used for the other args in this test such as the nearby entries) to keep the test consistent and avoid unused properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/usage.test.ts`:
- Around line 28-32: Remove the redundant name property from the multiPositional
arg object in the test (the object keyed as multiPositional in
test/usage.test.ts); the arg name is derived from the key, so delete the line
containing name: "pos" from the multiPositional object (mirroring the pattern
used for the other args in this test such as the nearby entries) to keep the
test consistent and avoid unused properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f33afb4-ebb3-4aed-b6d2-b2e8bc6ec111
📒 Files selected for processing (6)
playground/hello.tssrc/args.tssrc/types.tssrc/usage.tstest/args.test.tstest/usage.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/usage.ts (2)
43-47: Consider aligning column structure and description hints with positional args.Two inconsistencies with the
positionalbranch (line 52):
Column count: multiPositional pushes 3 columns
[name, description, valueHint], while positional pushes 2 columns. This may cause formatting inconsistencies informatLineColumnswhen both types appear together inposLines.Description format: positional uses
renderDescription(arg, isRequired)which includes(Required)and(Default: ...)hints in the description column. multiPositional only usesarg.description || "", omitting these hints.♻️ Proposed fix for consistency
const name = arg.name.toUpperCase(); const isRequired = arg.required !== false && arg.default === undefined; - // (isRequired ? " (required)" : " (optional)" - const defaultHint = arg.default ? `=[${arg.default}]` : ""; posLines.push([ - colors.cyan(name + defaultHint), - arg.description || "", - arg.valueHint ? `<${arg.valueHint}>` : "", + colors.cyan(name + renderValueHint(arg)), + renderDescription(arg, isRequired), ]); usageLine.push(isRequired ? `<...${name}>` : `[...${name}]`);This also requires updating
renderValueHintto handle multiPositional:- if (!arg.type || arg.type === "positional" || arg.type === "boolean") { + if (!arg.type || arg.type === "positional" || arg.type === "multiPositional" || arg.type === "boolean") { return valueHint; }Note:
renderDescriptionwill display array defaults as(Default: a,b)via implicit toString. If a different format is preferred for arrays, consider extendingrenderDescriptionto handle array defaults explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/usage.ts` around lines 43 - 47, posLines assembly for multiPositional is inconsistent with positional: multiPositional pushes three columns [name, description, valueHint] while positional pushes two, and multiPositional omits required/default hints that renderDescription provides; fix by making multiPositional push the same column structure as positional (use colors.cyan(name + defaultHint) and renderDescription(arg, isRequired) for the description column) and move any value-hint rendering into renderValueHint so both branches call renderValueHint(arg) for the third column (which may return "" when not applicable); update renderValueHint to return an appropriate string for multiPositional entries (e.g., format <value> or empty) and ensure formatLineColumns is fed uniform column counts.
41-41: Remove orphaned comment.This appears to be leftover debug or development code. The comment is incomplete and serves no purpose.
🧹 Proposed fix
const isRequired = arg.required !== false && arg.default === undefined; - // (isRequired ? " (required)" : " (optional)" const defaultHint = arg.default ? `=[${arg.default}]` : "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/usage.ts` at line 41, Remove the orphaned commented fragment "// (isRequired ? \" (required)\" : \" (optional)\"" from the codebase; this incomplete comment serves no purpose and should be deleted so the surrounding function (the usage/help text builder) no longer contains the stray leftover debug text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/usage.ts`:
- Around line 43-47: posLines assembly for multiPositional is inconsistent with
positional: multiPositional pushes three columns [name, description, valueHint]
while positional pushes two, and multiPositional omits required/default hints
that renderDescription provides; fix by making multiPositional push the same
column structure as positional (use colors.cyan(name + defaultHint) and
renderDescription(arg, isRequired) for the description column) and move any
value-hint rendering into renderValueHint so both branches call
renderValueHint(arg) for the third column (which may return "" when not
applicable); update renderValueHint to return an appropriate string for
multiPositional entries (e.g., format <value> or empty) and ensure
formatLineColumns is fed uniform column counts.
- Line 41: Remove the orphaned commented fragment "// (isRequired ? \"
(required)\" : \" (optional)\"" from the codebase; this incomplete comment
serves no purpose and should be deleted so the surrounding function (the
usage/help text builder) no longer contains the stray leftover debug text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fdd11df2-6d80-48ab-aa01-238f1293fea6
📒 Files selected for processing (2)
src/usage.tstest/usage.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/usage.test.ts
3 participants
resolves #115
resolves #80
This PR adds support for
multiPositional.Summary by CodeRabbit