feat: support camel to kebab style arguments - #138
Conversation
|
Warning Rate limit exceeded@kazupon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes implement kebab-case conversion for command-line argument names, update usage rendering to reflect this, introduce a utility function for case conversion, modify tests to verify kebab-case output, update dependency versions, and alter CLI argument parsing to support short option grouping and kebab-case formatting. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant UsageRenderer
participant Utils
User->>CLI: Provide command-line arguments
CLI->>CLI: Parse arguments (shortGrouping: true)
CLI->>UsageRenderer: Request usage/help output
UsageRenderer->>Utils: kebabnize argument names (if toKebab)
Utils-->>UsageRenderer: Return kebab-case names
UsageRenderer-->>CLI: Rendered usage/help output
CLI-->>User: Display usage/help
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Deploying gunshi with
|
| Latest commit: |
e970a04
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad1f58cd.gunshi.pages.dev |
| Branch Preview URL: | https://feat-kebab.gunshi.pages.dev |
commit: |
![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
🧹 Nitpick comments (2)
src/renderer.test.ts (1)
55-56: Consider using a union type instead ofanyfor better type safety.While using
anyworks for test purposes, you could maintain better type safety by defining a union type for the different argument structures used in your tests.-// eslint-disable-next-line @typescript-eslint/no-explicit-any -const COMMANDS = new Map<string, Command<any> | LazyCommand<any>>() +type TestArgs = Args & { + foo?: { type: 'string'; short?: string; description?: string }; + bar?: { type: 'boolean'; short?: string; description?: string }; + // Add other test argument types as needed +}; +const COMMANDS = new Map<string, Command<TestArgs> | LazyCommand<TestArgs>>()src/renderer/usage.ts (1)
273-291: Comprehensive kebab-case support across all option types!The implementation correctly handles parameter placeholders and negatable options. The consistent use of the
toKebabflag ensures backward compatibility.Consider making the inline comments slightly more descriptive:
- // Use kebab-case for parameter placeholders too if toKebab is true + // Convert parameter placeholders to kebab-case format when toKebab is enabled const displayName = schema.toKebab ? kebabnize(name) : name- // Use kebab-case for negated options too if toKebab is true + // Convert negatable option names to kebab-case format when toKebab is enabled const displayName = schema.toKebab ? kebabnize(name) : name
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/__snapshots__/renderer.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
.github/renovate.json(1 hunks)package.json(3 hunks)src/cli.ts(1 hunks)src/renderer.test.ts(2 hunks)src/renderer/usage.ts(3 hunks)src/utils.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
.github/renovate.json (1)
5-5: LGTM! Proper dependency management configuration.The addition of
gunshi019to theignoreDepsarray correctly prevents Renovate from automatically upgrading this dependency, which aligns with the intentional downgrade inpackage.jsonfrom version0.24.0to0.19.0.src/cli.ts (1)
49-49: LGTM! Improves CLI option parsing behavior.The change from
optionGroupingtoshortGrouping: trueenables proper short option grouping (e.g.,-abcinstead of-a -b -c), which is a standard CLI convention that enhances user experience and complements the new kebab-case argument formatting feature.src/utils.ts (1)
76-84: LGTM! Well-implemented kebab-case conversion utility.The
kebabnizefunction correctly converts camelCase strings to kebab-case using an appropriate regex pattern. The implementation handles the transformation efficiently and includes proper documentation.Note: The function works well for typical camelCase identifiers like
userName→user-name, but may produce verbose output for strings with consecutive uppercase letters (e.g.,someAPI→some-a-p-i). This is acceptable for command-line argument formatting where clarity is preferred.package.json (3)
140-140: LGTM! Strategic version management for testing purposes.The downgrade to
npm:gunshi@0.19.0(from0.24.0) appears intentional, likely for compatibility testing or benchmarking against a previous version. This aligns with the Renovate ignore configuration to prevent automatic upgrades.
123-123: LGTM! Routine dependency maintenance updates.These patch and minor version updates for development dependencies are appropriate and should provide bug fixes and improvements without breaking changes:
@vitest/eslint-plugin:^1.2.0→^1.2.1(patch update)eslint-import-resolver-typescript:^4.3.5→^4.4.1(minor update)knip:^5.57.2→^5.58.0(minor update)tsdown:^0.12.2→^0.12.3(patch update)vitepress-plugin-llms:^1.3.3→^1.3.4(patch update)Also applies to: 128-128, 143-143, 150-150, 157-157
114-114:Details
✅ Verification successful
Verify the args-tokens upgrade supports the new kebab-case feature.
The upgrade from
^0.18.0to^0.19.0appears to support the newshortGroupingoption used insrc/cli.ts. This is a strategic update that enables the kebab-case argument formatting feature.
🏁 Script executed:
#!/bin/bash # Description: Verify that args-tokens v0.19.0 supports shortGrouping option # Expected: Confirmation that the API supports the new option used in cli.ts # Check the args-tokens package information and changelog curl -s https://registry.npmjs.org/args-tokens/0.19.0 | jq '.version, .description' # Search for recent commits or releases mentioning shortGrouping curl -s "https://api.github.com/search/repositories?q=args-tokens+user:kazupon" | jq '.items[0].html_url' | xargs -I {} echo "Check repository: {}"Length of output: 369
🏁 Script executed:
#!/bin/bash # Search for “shortGrouping” in args-tokens v0.19.0 docs echo ">> Checking README.md for shortGrouping" curl -s https://raw.githubusercontent.com/kazupon/args-tokens/v0.19.0/README.md \ | grep -C2 shortGrouping || echo "No match in README.md" echo echo ">> Checking CHANGELOG.md for shortGrouping" curl -s https://raw.githubusercontent.com/kazupon/args-tokens/v0.19.0/CHANGELOG.md \ | grep -C2 shortGrouping || echo "No match in CHANGELOG.md"Length of output: 520
🏁 Script executed:
#!/bin/bash # Clone args-tokens v0.19.0 and search for shortGrouping in source git clone --depth 1 --branch v0.19.0 https://github.com/kazupon/args-tokens.git /tmp/args-tokens echo ">> Searching for shortGrouping in code" grep -R "shortGrouping" /tmp/args-tokens || echo "No shortGrouping found in code"Length of output: 1226
Approve dependency updates; args-tokens v0.19.0 includes
shortGroupingsupportVerified that the
shortGroupingoption is defined and exercised in args-tokens v0.19.0 (seesrc/resolver.tsand tests), so the upgrade properly enables the kebab-case formatting feature insrc/cli.ts. All other dependency bumps and the intentionalgunshi019downgrade look safe—approving these changes.src/renderer.test.ts (1)
505-558: Well-structured test case for kebab-case functionality!The test comprehensively covers various scenarios including negatable options and required arguments with the
toKebabproperty. The examples clearly demonstrate the expected kebab-case output format.src/renderer/usage.ts (2)
10-10: LGTM!The import follows the existing pattern and integrates cleanly with other utility imports.
258-265: Clean implementation of kebab-case transformation!The conditional transformation based on
schema.toKebabis well-implemented and maintains backward compatibility. The comment clearly explains the purpose.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/definition.ts (1)
2-2: Fix the typo in the comment.The word "deifinition" should be "definition".
- * The entry for command deifinition. + * The entry for command definition.src/renderer.test.ts (1)
55-56: Consider a more type-safe approach instead of usingany.While the ESLint disable comment acknowledges the issue, using
anyreduces type safety. Consider creating a more specific union type or using generics to maintain type safety while accommodating the test requirements.-// eslint-disable-next-line @typescript-eslint/no-explicit-any -const COMMANDS = new Map<string, Command<any> | LazyCommand<any>>() +const COMMANDS = new Map<string, Command<Args> | LazyCommand<Args>>()If this change causes type issues, consider creating a test-specific interface that extends the base types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/__snapshots__/renderer.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
src/context.ts(1 hunks)src/definition.test.ts(2 hunks)src/definition.ts(1 hunks)src/renderer.test.ts(2 hunks)src/renderer/usage.ts(4 hunks)src/types.ts(2 hunks)
🧰 Additional context used
🪛 GitHub Check: Spell check with Typos
src/types.ts
[warning] 292-292:
"conext" should be "context" or "connect" or "connects".
🪛 GitHub Actions: Typos
src/types.ts
[error] 292-292: conext should be context, connect, connects
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
src/context.ts (1)
221-221: LGTM!The
toKebabproperty is correctly propagated from the command to the context, following the same pattern as other properties.src/definition.test.ts (1)
29-29: Good test coverage for the new feature!The test properly verifies that the
toKebabproperty propagates correctly through lazy command definitions.Also applies to: 48-48
src/definition.ts (1)
47-47: LGTM!The
toKebabproperty is correctly propagated to the lazy command loader, following the established pattern.src/renderer.test.ts (2)
505-558: Comprehensive test coverage for individual argument kebab-case conversion.The test effectively covers various argument types (string, boolean with negatable, number, required) with individual
toKebabflags. The test data includes proper examples showing the expected kebab-case output format.
560-610: Good test coverage for command-level kebab-case conversion.This test validates that the command-level
toKebabproperty works correctly, with one argument (bazQux) explicitly settingtoKebab: trueto test override behavior. The test demonstrates that command-level settings can be overridden at the argument level.src/renderer/usage.ts (4)
10-10: Good addition of kebab-case utility import.The import of
kebabnizefunction provides the necessary utility for case conversion.
257-265: Well-implemented kebab-case conversion in option pair generation.The function correctly handles kebab-case conversion by checking both the passed
toKebabparameter and the schema-leveltoKebabproperty. The logic prioritizes schema-level settings, which provides good flexibility.
273-291: Comprehensive kebab-case handling for all option types.The implementation correctly handles:
- Option keys with kebab-case conversion
- Parameter placeholders with consistent case conversion
- Negatable boolean options with proper
--no-prefix handlingThe logic ensures consistent kebab-case formatting across all option representations.
359-359: Consistent kebab-case application in negatable option generation.The update ensures that negatable options also respect the kebab-case formatting when generating help text for negated boolean flags.
| /** | ||
| * Whether to convert the camel-case style argument name to kebab-case. | ||
| * This conext value is set from {@link Command.toKebab} option. | ||
| */ | ||
| toKebab?: boolean |
There was a problem hiding this comment.
Fix the typo in the documentation.
The word "conext" on line 292 should be "context".
/**
* Whether to convert the camel-case style argument name to kebab-case.
- * This conext value is set from {@link Command.toKebab} option.
+ * This context value is set from {@link Command.toKebab} option.
*/
toKebab?: boolean📝 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.
| /** | |
| * Whether to convert the camel-case style argument name to kebab-case. | |
| * This conext value is set from {@link Command.toKebab} option. | |
| */ | |
| toKebab?: boolean | |
| /** | |
| * Whether to convert the camel-case style argument name to kebab-case. | |
| * This context value is set from {@link Command.toKebab} option. | |
| */ | |
| toKebab?: boolean |
🧰 Tools
🪛 GitHub Check: Spell check with Typos
[warning] 292-292:
"conext" should be "context" or "connect" or "connects".
🪛 GitHub Actions: Typos
[error] 292-292: conext should be context, connect, connects
🤖 Prompt for AI Agents
In src/types.ts around lines 290 to 294, the documentation comment contains a
typo where "conext" should be "context". Correct the typo by replacing "conext"
with "context" to improve clarity and accuracy of the documentation.
1 participant
Description
Linked Issues
close #136
kazupon/args-tokens#99
Additional context
Summary by CodeRabbit
args-tokens,@vitest/eslint-plugin,eslint-import-resolver-typescript,knip,tsdown, andvitepress-plugin-llms.gunshi019from0.24.0to0.19.0and added it to ignore list in configuration.toKebabflag to enable conversion of argument names to kebab-case.toKebabproperty.