Skip to content

feat(plugin-renderer): improve positional argument with multiple: true usage display - #432

Merged
kazupon merged 1 commit into
kazupon:mainfrom
ota-meshi:help
Dec 11, 2025
Merged

feat(plugin-renderer): improve positional argument with multiple: true usage display#432
kazupon merged 1 commit into
kazupon:mainfrom
ota-meshi:help

Conversation

@ota-meshi

@ota-meshi ota-meshi commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Description

This PR improves the help text when there are positional arguments with multiple: true.

Linked Issues

close #431

Additional context

If the output format is not what you want, please let me know.

Summary by CodeRabbit

  • Bug Fixes

    • Improved rendering of positional arguments in usage output to correctly display optional and multiple argument syntax.
  • Documentation

    • Clarified when positional arguments are implicitly required based on their configuration.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 11, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request implements support for variable-length positional arguments by updating the usage rendering to display multiple positional arguments with the [<name> ...] syntax. The documentation is clarified, rendering logic is modified, and tests are added to verify the new behavior.

Changes

Cohort / File(s) Summary
Documentation updates
packages/docs/src/guide/essentials/declarative.md
Clarifies that positional arguments are implicitly required only when they do not have multiple: true
Test additions
packages/plugin-renderer/src/usage.test.ts
Adds two new test cases verifying usage rendering for multiple positional arguments, with and without the required flag
Rendering logic
packages/plugin-renderer/src/usage.ts
Modifies generatePositionalSymbols() to render positional arguments based on multiplicity: single <name> for non-multiple or required args, and [<name> ...] for multiple args

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • The logic change in generatePositionalSymbols() requires careful verification of conditional rendering for various combinations of multiple and required flags
  • Snapshot test outputs should be verified to ensure the generated usage strings match expected formatting

Possibly related PRs

Suggested labels

feature

Poem

🐰 Variable-length args now shine so bright,
With [<name> ...] rendering just right,
No more confusion in the help display,
Multiple positionals now have their way!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ 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 accurately describes the main change: improving how positional arguments with multiple: true are displayed in usage/help output.
Linked Issues check ✅ Passed The PR successfully implements the requested feature from issue #431: support for multiple: true on positional argument definitions and improved usage display showing variable-length positional arguments.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing multiple: true support for positional arguments and updating their usage display, with no unrelated modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5233c3a and f1c1190.

⛔ Files ignored due to path filters (1)
  • packages/plugin-renderer/src/__snapshots__/usage.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (3)
  • packages/docs/src/guide/essentials/declarative.md (1 hunks)
  • packages/plugin-renderer/src/usage.test.ts (1 hunks)
  • packages/plugin-renderer/src/usage.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/*/src/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

packages/*/src/**/*.ts: Use TypeScript Strict Mode for all source code
Use ES modules (import/export) throughout the codebase, not require
Code style must be enforced by ESLint and Prettier, and auto-fixed with 'pnpm fix'
Maintain strict TypeScript types throughout the codebase and use type inference where possible

Files:

  • packages/plugin-renderer/src/usage.test.ts
  • packages/plugin-renderer/src/usage.ts
packages/*/src/**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place test files alongside source files with .test.ts extension for unit tests

Files:

  • packages/plugin-renderer/src/usage.test.ts
packages/plugin-*/src/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Implement plugins using @gunshi/plugin package for minimal dependencies following the plugin API in packages/plugin/src/

Files:

  • packages/plugin-renderer/src/usage.test.ts
  • packages/plugin-renderer/src/usage.ts
🧠 Learnings (3)
📚 Learning: 2025-12-05T09:55:04.596Z
Learnt from: CR
Repo: kazupon/gunshi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T09:55:04.596Z
Learning: Applies to packages/gunshi/src/**/*.ts : Test changes across all supported runtimes (Node.js, Deno, Bun) when modifying core functionality

Applied to files:

  • packages/plugin-renderer/src/usage.test.ts
📚 Learning: 2025-12-05T09:55:04.596Z
Learnt from: CR
Repo: kazupon/gunshi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T09:55:04.596Z
Learning: Applies to packages/*/src/**/__snapshots__/* : Use snapshot tests in __snapshots__/ directories for renderer output validation

Applied to files:

  • packages/plugin-renderer/src/usage.test.ts
📚 Learning: 2025-12-05T09:55:04.596Z
Learnt from: CR
Repo: kazupon/gunshi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-05T09:55:04.596Z
Learning: When modifying command parsing or resolution logic, verify impact on all playground examples

Applied to files:

  • packages/plugin-renderer/src/usage.test.ts
🧬 Code graph analysis (2)
packages/plugin-renderer/src/usage.test.ts (6)
packages/gunshi/src/constants.ts (1)
  • NOOP (13-13)
packages/gunshi/src/definition.ts (4)
  • Command (58-58)
  • GunshiParams (63-63)
  • Args (55-55)
  • createCommandContext (52-52)
packages/gunshi/src/index.ts (1)
  • createCommandContext (32-32)
packages/gunshi/src/context.ts (1)
  • createCommandContext (122-230)
packages/plugin-renderer/src/usage.ts (1)
  • renderUsage (39-77)
packages/plugin-renderer/src/index.ts (1)
  • renderUsage (51-51)
packages/plugin-renderer/src/usage.ts (1)
playground/essentials/getting-started/context/cli.js (1)
  • name (5-5)
🔇 Additional comments (3)
packages/plugin-renderer/src/usage.ts (1)

555-564: LGTM! Excellent implementation of variable-length positional argument rendering.

The logic correctly handles all cases:

  • Non-multiple positionals render as <name> (implicitly required)
  • Multiple-only positionals render as [<name> ...] (zero or more)
  • Multiple + required positionals render as <name> [<name> ...] (one or more)

This elegantly communicates the cardinality to users in a familiar format.

packages/docs/src/guide/essentials/declarative.md (1)

168-168: LGTM! Documentation accurately reflects the updated behavior.

The clarification that implicit requirement applies only to positionals "without multiple: true" is essential for users to understand that variable-length positionals can be optional. This aligns perfectly with the rendering logic changes.

packages/plugin-renderer/src/usage.test.ts (1)

326-411: LGTM! Comprehensive test coverage for the new multiple positional functionality.

The two new tests properly verify:

  1. Optional variable-length positionals (multiple: true)
  2. Required variable-length positionals (multiple: true, required: true)

The test structure follows established patterns, correctly sets up command contexts, and uses snapshot assertions for output validation. Based on learnings, snapshot tests are the standard approach for renderer output validation in this codebase.


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.

- `description`: A description of what the option does
- `default`: Default value if the option is not provided
- `required`: Set to `true` if the option is required (Note: Positional arguments defined with `type: 'positional'` are implicitly required by the parser).
- `required`: Set to `true` if the option is required (Note: Positional arguments defined with `type: 'positional'` without `multiple: true` are implicitly required by the parser).

@ota-meshi ota-meshi Dec 11, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that this part probably wasn't correctly explaining the behavior before (v0.27 onwards), so I changed it.

@kazupon kazupon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good first contribution!
Thank you!

@pkg-pr-new

pkg-pr-new Bot commented Dec 11, 2025

Copy link
Copy Markdown

Open in StackBlitz

@gunshi/bone

npm i https://pkg.pr.new/@gunshi/bone@432

@gunshi/definition

npm i https://pkg.pr.new/@gunshi/definition@432

@gunshi/docs

npm i https://pkg.pr.new/@gunshi/docs@432

gunshi

npm i https://pkg.pr.new/gunshi@432

@gunshi/plugin

npm i https://pkg.pr.new/@gunshi/plugin@432

@gunshi/plugin-completion

npm i https://pkg.pr.new/@gunshi/plugin-completion@432

@gunshi/plugin-dryrun

npm i https://pkg.pr.new/@gunshi/plugin-dryrun@432

@gunshi/plugin-global

npm i https://pkg.pr.new/@gunshi/plugin-global@432

@gunshi/plugin-i18n

npm i https://pkg.pr.new/@gunshi/plugin-i18n@432

@gunshi/plugin-renderer

npm i https://pkg.pr.new/@gunshi/plugin-renderer@432

@gunshi/resources

npm i https://pkg.pr.new/@gunshi/resources@432

@gunshi/shared

npm i https://pkg.pr.new/@gunshi/shared@432

commit: f1c1190

@kazupon
kazupon merged commit 8a899d1 into kazupon:main Dec 11, 2025
9 of 10 checks passed
@ota-meshi
ota-meshi deleted the help branch December 11, 2025 22:46
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.

Variable-length positional arguments

2 participants