docs: ready for v0.27 - #275
Conversation
WalkthroughDocumentation-first v0.27 overhaul: extensive docs added/rewritten to introduce a plugin-first architecture, consolidated feature list, i18n moved to a plugin model with key-resolution helpers, mermaid support added, examples updated to use plugin ID constants and define()/run() patterns, plus a few type/exports and devDependency bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (app)
participant PS as Plugin System
participant P as Plugin
participant PR as Parser/Resolver
participant CTX as CommandContext
participant CMD as Command
User->>CLI: Invoke (args)
CLI->>PS: Register plugins
PS->>PS: Resolve dependencies (toposort)
PS->>P: setup(ctx)
CLI->>PR: Parse args, resolve command
CLI->>CTX: Create context (values, positionals, extensions)
PS->>P: extension(ctx, cmd) in dependency order
PS->>P: onExtension(ctx, cmd) in dependency order
CLI->>CLI: onBeforeCommand?(ctx)
CLI->>CMD: run(ctx) via decorator chain
alt success
CLI->>CLI: onAfterCommand?(ctx)
else error
CLI->>CLI: onErrorCommand?(ctx, error)
end
sequenceDiagram
autonumber
actor User
participant CLI as CLI (--help)
participant Resolver as Renderer Resolver
participant PluginR as Plugin Renderer
participant CLIR as CLI Renderer
participant CMD as Command Renderer
User->>CLI: Request help
CLI->>Resolver: Resolve renderer chain (Command → CLI → Plugin → Default)
Resolver->>CMD: call command-level renderer (if present)
CMD-->>CLIR: transformed output
CLIR-->>PluginR: transformed output
PluginR-->>CLI: final rendered help
CLI-->>User: Help text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
@gunshi/bone
@gunshi/definition
gunshi
@gunshi/plugin
@gunshi/plugin-completion
@gunshi/plugin-dryrun
@gunshi/plugin-global
@gunshi/plugin-i18n
@gunshi/plugin-renderer
@gunshi/resources
@gunshi/shared
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/docs/src/index.md (2)
22-24: Refine wording for smoother reading“Run commands with simple API and support for universal runtime …” is slightly awkward. Consider re-ordering:
-Run commands with simple API and support for universal runtime (Node.js, Deno, Bun). +Run commands with a simple API and universal-runtime support (Node.js, Deno, Bun).
41-43: Link out to plugin documentationSince “Pluggable” is a new headline feature, adding an inline link to the plugin-system guide (if one exists or will exist) helps readers jump straight to the details.
-details: Extensible plugin system with dependency management and lifecycle hooks for modular CLI development. +details: Extensible plugin system with dependency management and lifecycle hooks for modular CLI development. [Learn more »](/guide/plugins/overview)README.md (2)
11-12: Capitalize “JavaScript” for consistencyLine 11 currently reads “Gunshi is a modern javascript command-line library”. Industry convention is “JavaScript”.
24-29: Minor copy-editing & parallelism
- Add the article “a” before “simple API”.
- Prefer plural “runtimes”.
- Trailing period after every bullet for uniform style.
-📏 **Simple & Universal**: Run commands with simple API and support for universal runtime (Node.js, Deno, Bun) +📏 **Simple & Universal**: Run commands with a simple API and universal-runtime support (Node.js, Deno, Bun). -⚙️ **Declarative & Type Safe**: Configure commands declaratively with full TypeScript support and type-safe argument parsing by [args-tokens](https://github.com/kazupon/args-tokens) +⚙️ **Declarative & Type Safe**: Configure commands declaratively with full TypeScript support and type-safe argument parsing by [args-tokens](https://github.com/kazupon/args-tokens). -🧩 **Composable & Lazy**: Create modular sub-commands with context sharing and lazy loading for better performance +🧩 **Composable & Lazy**: Create modular sub-commands with context sharing and lazy loading for better performance. -🎨 **Flexible Rendering**: Customize usage generation, validation errors, and help messages with pluggable renderers +🎨 **Flexible Rendering**: Customize usage generation, validation errors, and help messages with pluggable renderers. -🌍 **Internationalization**: Built with global users in mind, featuring locale-aware design, resource management, and multi-language support +🌍 **Internationalization**: Built with global users in mind, featuring locale-aware design, resource management, and multi-language support. -🔌 **Pluggable**: Extensible plugin system with dependency management and lifecycle hooks for modular CLI development +🔌 **Pluggable**: Extensible plugin system with dependency management and lifecycle hooks for modular CLI development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)packages/docs/src/index.md(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
📚 Learning: applies to packages/gunshi/src/**/*.ts : all source code is in typescript with strict mode enabled...
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Applied to files:
README.md
📚 Learning: applies to packages/gunshi/test/**/*.test.ts : add tests for new features in the corresponding test ...
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Applied to files:
README.md
📚 Learning: applies to packages/gunshi/src/**/*.ts : type safety is a core feature - maintain strict typescript ...
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Applied to files:
README.md
📚 Learning: applies to packages/gunshi/test/**/*.test.ts : test files are in 'packages/gunshi/test/' with '.test...
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Test files are in 'packages/gunshi/test/' with '.test.ts' extension
Applied to files:
README.md
📚 Learning: applies to packages/gunshi/test/**/*.test.ts : use 'describe' and 'it' blocks for test organization...
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Use 'describe' and 'it' blocks for test organization
Applied to files:
README.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test on Node.js 20
- GitHub Check: release
- GitHub Check: Cloudflare Pages
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/docs/src/release/v0.27.md (2)
140-146: Avoid passing the same command as both the entry command and a subcommand.Using
buildCommandas the entry and also assubCommands.buildis confusing. Prefer a small root command and registerbuildandanalyzeundersubCommands.-await cli(process.argv.slice(2), buildCommand, { - name: 'app', +await cli(process.argv.slice(2), { name: 'app', run: () => {} }, { subCommands: { build: buildCommand, analyze: analyzeCommand } })
53-54: Clarify that built-in plugins don’t require installation.Minor wording tweak to prevent readers from thinking they need to install these.
-These plugins are automatically included when using the standard `cli()` function from gunshi: +These plugins are automatically included when using the standard `cli()` function from gunshi (no separate installation required):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/docs/src/release/v0.27.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test on Node.js 20
- GitHub Check: release
- GitHub Check: release
🔇 Additional comments (2)
packages/docs/src/release/v0.27.md (2)
431-449: Verifyclisignature accepting a function as the entry command.Other examples pass a command object. If
cli(argv, handlerFn, options)is not supported, wrap the handler withdefine()or pass a command object.Suggested alternative if a function is not allowed:
import { define } from 'gunshi' const root = define<{ extensions: Record<I18nId, I18nExtension> & Record<MetricsId, MetricsExtension> }>({ name: 'my-cli', run: async ctx => { const greeting = ctx.extensions[i18nId]?.translate('welcome') console.log(greeting) ctx.extensions[metricsId]?.track('cli.started', { command: ctx.name, locale: ctx.extensions[i18nId]?.locale.toString() }) } }) await cli<{ extensions: Record<I18nId, I18nExtension> & Record<MetricsId, MetricsExtension> }>( process.argv.slice(2), root, { name: 'my-cli', plugins: [i18n(), metrics()] } )
545-546: JSON import attributes syntax may not be portable across environments.
with { type: 'json' }support varies. Many environments still useassert { type: 'json' }. Confirm target runtimes and adjust docs accordingly.Alternative:
import enUS from '@gunshi/resources/en-US.json' assert { type: 'json' } import jaJP from '@gunshi/resources/ja-JP.json' assert { type: 'json' }
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/docs/src/guide/essentials/composable.md (3)
13-13: Good addition: call out plugin-wide sharing across sub-commandsHighlighting that plugins apply uniformly to all sub-commands clarifies mental models for readers. Consider adding a short cross-link here to the Plugin Ecosystem section for immediate discoverability.
Apply this small tweak:
-**Plugin integration**: Plugins are shared across all sub-commands for consistent functionality +**Plugin integration**: Plugins are shared across all sub-commands for consistent functionality (see [Plugin Ecosystem](./plugin-ecosystem.md))
78-131: Type-safe example looks solid; add a note about ESM/top-level awaitThe example uses top-level await (requires ESM) and TypeScript generics (Map<string, Command>). A short note helps readers avoid copy/paste confusion in CJS environments.
Proposed addition right below the code block:
+Note: Examples assume an ESM environment (top-level `await`) and TypeScript. For CJS, wrap `await cli(...)` in an async IIFE, and for plain JS omit type annotations.
216-267: Keep command construction consistent in “Organized Command Structure”You use
define()for sub-commands but a plain object for the main command. For consistency and future extensibility (e.g., plugins augmenting metadata), consider usingdefine()for the main command too.-const mainCommand = { +const mainCommand = define({ name: 'resource-manager', run: () => { console.log('Use a sub-command') } -} +})packages/docs/src/guide/essentials/internationalization.md (4)
147-171: JSON import attributes: confirm target runtime support or show portable alternative
import ... with { type: 'json' }relies on Import Attributes support. Some Node/bundler setups still expectassert { type: 'json' }or allow bare JSON imports. Either note minimum runtime (Node version/bundler) or provide an alternative example to reduce friction.Possible portable alternative snippet:
// Node-compatible alternative import { createRequire } from 'node:module' const require = createRequire(import.meta.url) const enUS = require('@gunshi/resources/en-US.json') const jaJP = require('@gunshi/resources/ja-JP.json')Or use
assertif your toolchain prefers it:import enUS from '@gunshi/resources/en-US.json' assert { type: 'json' } import jaJP from '@gunshi/resources/ja-JP.json' assert { type: 'json' }Add a short note clarifying which syntax is supported in your minimum Node version.
193-210: Dynamic JSON imports also use Import Attributes—mirror the support noteSame concern as above for dynamic import with
with: { type: 'json' }. Add a runtime/tooling note or show a variant without attributes to keep examples copy-paste friendly.-const resource = await import('./locales/ja-JP.json', { - with: { type: 'json' } -}) +// If your runtime doesn't support import attributes, consider: +// const resource = await import('./locales/ja-JP.json')
414-426: Usei18nIdconsistently instead of a hard-coded extension keyElsewhere, you standardize on
pluginId as i18nId. Here you use a literal'g:i18n', which can drift from the actual plugin ID.-import { defineI18n, resolveKey } from '@gunshi/plugin-i18n' +import { defineI18n, resolveKey, pluginId as i18nId } from '@gunshi/plugin-i18n' ... - console.log(ctx.extensions['g:i18n'].translate(welcomeKey)) + console.log(ctx.extensions[i18nId].translate(welcomeKey))
739-746: Verify “v0.27 Release Notes” link targetThe absolute path (/gunshi-v027-release-notes#internationalization-migration) may not resolve correctly depending on the site router. Consider linking to the release note doc route actually used by the docs site (e.g., /release/v0.27 or a relative path).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/docs/src/guide/essentials/composable.md(2 hunks)packages/docs/src/guide/essentials/context-extensions.md(1 hunks)packages/docs/src/guide/essentials/internationalization.md(7 hunks)packages/docs/src/guide/essentials/plugin-ecosystem.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/docs/src/guide/essentials/plugin-ecosystem.md
- packages/docs/src/guide/essentials/context-extensions.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
packages/docs/src/guide/essentials/composable.md (1)
269-291: Fallback docs read wellClear example and comments; no issues spotted. Nice.
packages/docs/src/guide/essentials/internationalization.md (1)
28-32: Basic example reads well and aligns with the plugin APIThe end-to-end sample (defineI18n, resolveKey, ctx.extensions usage, and plugin wiring) is coherent and actionable.
Also applies to: 46-68, 70-91
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
packages/docs/src/guide/advanced/command-hooks.md (1)
357-360: Broken cross-links for Type System and Plugin LifecycleThe link to
./rendering-customization.mdis valid, but I couldn’t locate the other two files in the repo:
./type-system.md(no matching file found)/guide/plugin/lifecycle.md(no matching file found)Please confirm the correct filenames and locations for these docs—either add them or update the links in
packages/docs/src/guide/advanced/command-hooks.md(lines 357–360).
🧹 Nitpick comments (2)
packages/docs/src/guide/advanced/command-hooks.md (2)
330-350: Add missing import for plugin in the example.Readers may copy-paste; include the import for completeness.
```ts +import { plugin } from 'gunshi' // Using decorateCommand in a plugin export default plugin({ id: 'timing-plugin', setup: ctx => {
98-100: Capitalize section heading consistently.Use “Use Cases” for consistency with other headings.
-## Use cases +## Use Cases
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/docs/src/guide/advanced/command-hooks.md(1 hunks)packages/docs/src/guide/advanced/rendering-customization.md(1 hunks)packages/docs/src/guide/essentials/getting-started.md(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/docs/src/guide/advanced/rendering-customization.md
- packages/docs/src/guide/essentials/getting-started.md
🔇 Additional comments (3)
packages/docs/src/guide/advanced/command-hooks.md (3)
210-232: Authentication example uses validCommandContextproperties
Confirmed thatCommandContext(inpackages/gunshi/src/types.ts) defines both:
name: string | undefinedvalues: ArgValues<ExtractArgs<G>>No changes required.
319-323: Documentation correctly reflects LIFO decorator application.Verified in
packages/gunshi/src/cli/core.ts(lines 318–321) that plugins are applied viadecorators.reduceRight, confirming last-registered decorators execute first. No changes needed.
50-79: CLI signature and hooks placement are correctThe example aligns with the
cliAPI:
cli(args, entry, options?)accepts a singleCommandobject (or runner/lazy) as the second parameter—your{ name: 'server', run: … }is valid.- Lifecycle hooks (
onBeforeCommand,onAfterCommand,onErrorCommand) belong in the third‐argumentoptionsobject along withnameandversion.- (For multiple commands, you’d use the
subCommandsproperty onCliOptions.)No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
packages/plugin/README.md (2)
512-513: Typo: useAuthExtension(singular) in generic parameter.
AuthExtensionsis not defined; this breaks the example’s types.- AuthExtensions // Extension factory return type + AuthExtension // Extension factory return type
949-972: Inconsistent import and invalid extension access; use the plugin ID constant.
- Import path alias looks wrong:
your-auth-loggervsyour-logger-plugin.- Accessing
ctx.extensions[LoggerId]uses a type, not a value. Index with the importedloggerPluginId.- Declare dependencies with the same constant to keep value/type aligned.
-import { pluginId as loggerPluginId } from 'your-auth-logger' -import type { PluginId as LoggerId, LoggerExtension } from 'your-logger-plugin' +import { pluginId as loggerPluginId } from 'your-logger-plugin' +import type { PluginId as LoggerId, LoggerExtension } from 'your-logger-plugin' @@ -// Depend on optional plugin -const dependencies = [{ id: 'my:logger', optional: true }] as const +// Depend on optional plugin (using the exported ID constant) +const dependencies = [{ id: loggerPluginId, optional: true }] as const @@ - // May be `undefined` - const logger = ctx.extensions[LoggerId] + // May be `undefined` + const logger = ctx.extensions[loggerPluginId]
♻️ Duplicate comments (4)
packages/docs/src/guide/advanced/command-hooks.md (4)
9-34: Mermaid diagrams won’t render without plugin; verify lifecycle ordering.Your docs site needs a Mermaid plugin for these diagrams to render. Also double-check that the “Apply Plugins” step precedes hook invocation as depicted, matching the actual engine lifecycle.
Would you like me to open a follow-up PR to wire up markdown-it-mermaid in VitePress and validate the lifecycle diagram against the engine?
86-96: Hook signatures: broaden result/error types tounknown(keepAwaitable).Current types are too narrow and imply only string results and Error instances. Use unknown to reflect real-world cases.
{ // Before command execution onBeforeCommand?: (ctx: Readonly<CommandContext>) => Awaitable<void> // After successful execution - onAfterCommand?: (ctx: Readonly<CommandContext>, result: string | undefined) => Awaitable<void> + onAfterCommand?: (ctx: Readonly<CommandContext>, result: unknown) => Awaitable<void> // On command error - onErrorCommand?: (ctx: Readonly<CommandContext>, error: Error) => Awaitable<void> + onErrorCommand?: (ctx: Readonly<CommandContext>, error: unknown) => Awaitable<void> }Follow-on: in code examples that read error.message/stack, guard with
error instanceof Error.
114-132: Duration calculation uses an undefined timestamp; record start time first.
Date.now() - logger?.startTimewill produce NaN unless a start time is stored. Persist a timestamp in onBeforeCommand and read it in onAfterCommand.onBeforeCommand: ctx => { const logger = ctx.extensions[loggerId] + // Record start time for duration calculation (avoid mutating public API) + ;(ctx as any).__startTime = Date.now() // Log command start with arguments logger?.info('Command started', { command: ctx.name, args: ctx.values, timestamp: new Date().toISOString() }) }, onAfterCommand: (ctx, result) => { const logger = ctx.extensions[loggerId] // Log successful completion logger?.info('Command completed', { command: ctx.name, - duration: Date.now() - logger?.startTime, + duration: Date.now() - (ctx as any).__startTime, result: typeof result }) },
303-333: Harden transaction commit/rollback; failures here will escape and crash.Wrap commit/rollback in try/catch and always clear the transaction reference to avoid dangling state.
if (transaction) { - // Commit on success - await db.commit(transaction.id) - console.log(`Transaction ${transaction.id} committed successfully`) + // Commit on success + try { + await db.commit(transaction.id) + console.log(`Transaction ${transaction.id} committed successfully`) + } catch (e) { + console.error('Transaction commit failed:', (e as Error)?.message ?? e) + } // Clean up transaction reference await db.clearCurrentTransaction() }if (transaction) { - // Rollback on error - await db?.rollback(transaction.id) - console.error(`Transaction ${transaction.id} rolled back due to error:`, error.message) + // Rollback on error + try { + await db?.rollback(transaction.id) + console.error(`Transaction ${transaction.id} rolled back due to error:`, (error as Error)?.message ?? error) + } catch (e) { + console.error('Transaction rollback failed:', (e as Error)?.message ?? e) + } // Log the failed transaction for audit await db?.logTransactionFailure({ id: transaction.id, command: ctx.name, - error: error.message, + error: (error as Error)?.message ?? String(error), timestamp: new Date().toISOString() }) // Clean up transaction reference await db?.clearCurrentTransaction() }
🧹 Nitpick comments (4)
packages/plugin/README.md (4)
453-454: Nit: avoid unnecessaryreturn awaitin async function without try/catch.
return awaitadds an extra microtask and is unnecessary unless you need to catch/transform errors.- return await runner(ctx) + return runner(ctx)
518-524: Nit: avoid unnecessaryreturn awaitin async wrapper.Same rationale as above; let the promise propagate.
- // cmdCtx.extensions is typed as { auth: AuthExtension, logger: LoggerExtension } - cmdCtx.extensions.logger.log('Command started') - return await runner(cmdCtx) + // cmdCtx.extensions is typed as { auth: AuthExtension, logger: LoggerExtension } + cmdCtx.extensions.logger.log('Command started') + return runner(cmdCtx)
990-996: Typo in comment: “safelly” → “safely”.- // Use safelly extensions with optional chaining + // Use safely extensions with optional chaining
1035-1035: Typo: “referer” → “refer to”.-TODO: referer the API section of gunshi docs +TODO: refer to the API section of gunshi docs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/docs/src/guide/advanced/command-hooks.md(1 hunks)packages/docs/src/guide/advanced/rendering-customization.md(1 hunks)packages/plugin/README.md(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/docs/src/guide/advanced/rendering-customization.md
🔇 Additional comments (1)
packages/plugin/README.md (1)
857-913: Minor API consistency: prefer using the exportedi18nPluginIdvalue everywhere.You’re correctly using
i18nPluginIdin dependencies and extension access—good. After fixingtypeof pluginIdabove, this snippet will be type/value aligned. No further changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/docs/src/guide/essentials/internationalization.md (4)
215-223: Normalize locale when loading from environment in file-based exampleSame normalization concern as above; apply the helper here as well for consistency.
plugins: [ i18n({ - locale: process.env.LANG || 'en-US' + locale: (() => { + const raw = process.env.LANG || 'en-US' + const base = raw.split('.')[0].replace('_', '-') + try { return new Intl.Locale(base).toString() } catch { return 'en-US' } + })() }) ]Also applies to: 229-233
98-101: Normalize environment locale values (LANG often not BCP 47)Raw LANG values like en_US.UTF-8 will not match 'en-US' or 'ja-JP' without normalization. Add a small normalizer before passing to the plugin.
plugins: [ i18n({ - // Set locale from environment or default to en-US - locale: process.env.LANG || 'en-US' + // Normalize env locale (e.g., en_US.UTF-8 -> en-US) + locale: (() => { + const raw = process.env.LANG || 'en-US' + const base = raw.split('.')[0].replace('_', '-') + try { return new Intl.Locale(base).toString() } catch { return 'en-US' } + })() }) ]
515-526: Invalid object literal: duplicatelocalekeys in the same options objectThe latter
localeoverwrites the former. Split into two separate examples to avoid confusion.-// Use various detection methods -await cli(process.argv.slice(2), command, { - plugins: [ - i18n({ - // From environment variable - locale: process.env.LANG || 'en-US', - - // Or using Intl.Locale for advanced locale handling - locale: new Intl.Locale(process.env.LANG || 'en-US') - }) - ] -}) +// Option A: From environment variable (normalize as needed) +await cli(process.argv.slice(2), command, { + plugins: [ i18n({ locale: process.env.LANG || 'en-US' }) ] +}) + +// Option B: Using Intl.Locale for advanced locale handling +await cli(process.argv.slice(2), command, { + plugins: [ i18n({ locale: new Intl.Locale(process.env.LANG || 'en-US') }) ] +})
528-530: Clarify experimental navigator.language in Node and add robust fallbacknavigator is experimental in Node and only available on certain versions. Feature-detect and fall back to environment/Intl.
-// For Node.js v21+, use navigator.language -const locale = typeof navigator !== 'undefined' && navigator.language ? navigator.language : 'en-US' +// Browser or Node (v21.2.0+ experimental): prefer navigator.language; otherwise fall back +const locale = (() => { + if (typeof globalThis.navigator !== 'undefined' && navigator.language) { + return navigator.language + } + const env = process.env.LC_ALL || process.env.LC_MESSAGES || process.env.LANG || 'en-US' + const base = env.split('.')[0].replace('_', '-') + try { return new Intl.Locale(base).toString() } catch { return 'en-US' } +})()
🧹 Nitpick comments (10)
packages/docs/src/guide/essentials/declarative-configuration.md (4)
167-167: Document accepted types and semantics forconflictsClarify that
conflictsaccepts a string or an array of strings and whether conflicts need to be declared on both sides or are enforced bidirectionally from a single declaration.Apply this diff to make it explicit:
-- `conflicts`: Specify mutually exclusive options that cannot be used together +- `conflicts`: Specify mutually exclusive options that cannot be used together. + Accepts a string (single option key) or an array of option keys (string | string[]). + Conflicts are enforced bidirectionally; declaring on one option is sufficient.
349-405: Conflicts section reads well; add short/long alias behavior note near exampleThe examples are clear. Consider adding a one-liner here (close to the example) that short aliases (
-v,-q) also participate in conflict checks to reduce scroll to the later note.// These options are mutually exclusive verbose: { type: 'boolean', short: 'v', description: 'Enable verbose output', conflicts: 'quiet' // Cannot be used with --quiet }, + // Note: Conflicts are enforced for both long and short forms (e.g., -v vs -q).
410-612: Examples coverage is thorough; consider trimming and formatting hintsYou already use .trim() in multi-line examples. Add a brief tip that leading/trailing whitespace is commonly trimmed before rendering to help users avoid accidental indentation artifacts.
#### Multiple Examples @@ - examples: ` + // Tip: It's common to trim leading/trailing whitespace before rendering examples. + examples: ` # Deploy to production with a specific tag deploy --environment production --tag v1.2.3 @@ `.trim(),
619-629: Explicit argument tracking: excellent addition; add boolean nuanceVery useful feature. Add a reminder that “explicit false” for booleans (e.g., via --no-flag) still sets explicit[flag] = true, which differs from omitted.
- The `explicit` property allows you to determine whether an argument was explicitly provided by the user or if it's using a default value: + The `explicit` property allows you to determine whether an argument was explicitly provided by the user or if it's using a default value. + For booleans, an explicit negation (e.g., `--no-force`) sets `explicit.force === true` while `values.force === false`.Also applies to: 631-662
packages/docs/src/guide/essentials/internationalization.md (2)
404-405: Avoid hardcoded plugin ID; use the exported i18nId constantPrefer ctx.extensions[i18nId] over 'g:i18n' to avoid stringly-typed IDs and keep type safety.
- console.log(ctx.extensions['g:i18n'].translate(welcomeKey)) + console.log(ctx.extensions[i18nId].translate(welcomeKey))
292-297: Safer error message extraction in exampleserror may not always be an Error instance. Guard to avoid undefined property access.
- } catch (error) { - console.error(t(errorKey, { message: error.message })) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + console.error(t(errorKey, { message })) }Also applies to: 666-669
packages/plugin/README.md (2)
453-454: Remove redundantawaitbefore returningReturning the awaited value inside an async function is unnecessary; return the promise directly.
- return await runner(ctx) + return runner(ctx)
518-524: Remove redundantawaitbefore returning (decorator wrapper)Same rationale as above; simplifies without behavior change.
- return await runner(cmdCtx) + return runner(cmdCtx)packages/docs/src/guide/advanced/type-system.md (2)
55-77: MissingArgsimport in “Type-safe for Arguments” snippetSince you reference
satisfies Args, add a brief import or replace with an inline type to keep the snippet self-contained.-} satisfies Args +} satisfies import('gunshi').Args
117-138: Add missing types/imports forCommandRunnerandArgsin lazy examplesReaders will benefit from explicit imports to make the snippets copy-pastable.
-type AuthExt = { +import type { CommandRunner, Args } from 'gunshi' + +type AuthExt = { auth: { authenticated: boolean } } @@ - const runner: CommandRunner<{ args: Args; extensions: AuthExt }> = async ctx => { + const runner: CommandRunner<{ args: Args; extensions: AuthExt }> = async ctx => {And similarly for the second lazy example:
-import type { CommandRunner } from 'gunshi' +import type { CommandRunner } from 'gunshi'Also applies to: 145-163
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/docs/src/guide/advanced/type-system.md(1 hunks)packages/docs/src/guide/essentials/declarative-configuration.md(3 hunks)packages/docs/src/guide/essentials/internationalization.md(7 hunks)packages/docs/src/guide/essentials/type-safe.md(1 hunks)packages/plugin/README.md(12 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Applied to files:
packages/docs/src/guide/advanced/type-system.mdpackages/docs/src/guide/essentials/type-safe.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
packages/plugin/README.md (3)
107-116: Heading rename to “Manage Plugin Dependencies” is appropriateThis better matches the content that follows. No issues.
871-876: Correct generic parameter fromtypeof idtotypeof pluginIdGood fix; this aligns the generic with the exported constant and prevents type errors.
900-911: Verifyi18n.loadResourceAPI availability in plugin exampleThis uses a method that may not exist on the public i18n extension in v0.27. Cross-check against @gunshi/plugin-i18n docs. If not available, remove or replace with the supported API (e.g., rely on builtinResources or resource(ctx) loading).
Would you confirm the public extension surface for @gunshi/plugin-i18n? If
loadResourceisn’t part of it, I can propose a corrected snippet.packages/docs/src/guide/essentials/type-safe.md (1)
86-96: Solid addition; cross-reference to the new Type System doc is helpfulThe section clearly scopes advanced generics and points to the detailed guide. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/plugin-completion/README.md (1)
109-257: Comprehensive shell completion documentation enhances user experience.The new "Shell Completion Setup" section provides excellent coverage of cross-shell installation procedures with clear step-by-step instructions. The troubleshooting section and warnings about system-wide installation are particularly helpful for users.
A few minor improvements to consider:
-**Option 1: User-specific completion directory (Recommended)** +### Option 1: User-specific completion directory (Recommended)-**Option 2: Alternative user directory** +### Option 2: Alternative user directoryThese changes would address the markdownlint warnings about using emphasis instead of proper headings, improving document structure and accessibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugin-completion/README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/plugin-completion/README.md
123-123: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
136-136: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test on Node.js 20
🔇 Additional comments (4)
packages/plugin-completion/README.md (4)
299-313: LGTM! Clear integration example with i18n plugin.The internationalization support section provides a concise example of how the completion plugin integrates with the i18n plugin, showing the automatic localization of completion descriptions.
258-297: LGTM! Excellent custom completion handler documentation.The custom completion handlers section provides comprehensive examples showing both simple static completions and dynamic completions based on previous arguments. The TypeScript examples clearly demonstrate the API usage.
354-377: LGTM! Clear plugin system integration documentation.The plugin dependencies and context extensions sections properly document the optional i18n integration and explain the plugin namespace system. The note about the plugin ID being available for type-safe access is helpful.
114-116: Confirm Node.js requirementConfirmed — docs are accurate: packages/plugin-completion/src/utils.ts detects runtime and throws for deno/bun, Node-specific APIs appear in packages/plugin-completion/src/bombshell/shared.ts and tests (index.test.ts), and package.json lists @types/node.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eslint.config.ts (3)
111-115: Remove duplicate ignore entry fordesign/**.
design/**appears twice (Line 111 and Line 115). Drop one to avoid redundancy.- 'design/**'
108-108: Normalize ignore pattern style.Mix of
./-prefixed and non‑prefixed patterns; align for consistency.- './packages/**/docs/**', + 'packages/**/docs/**',
113-113: Optional: ignore the whole.githubdirectory instead.If you don’t intend to lint any files under
.github/, broaden the pattern.- '.github/FUNDING.yml', + '.github/**',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eslint.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use ES modules throughout the codebase
Follow existing code style (enforced by ESLint and Prettier)
Files:
eslint.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test on Node.js 20
🔇 Additional comments (1)
eslint.config.ts (1)
113-113: Addition looks good; reduces noise from third‑party metadata.Ignoring
.github/FUNDING.ymlviaglobalIgnoresis reasonable.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
packages/docs/src/.vitepress/config.ts (1)
73-90: Fix the inconsistent link format in the new Plugin section.The Plugin section has inconsistent link formats - some include
.mdextensions while others don't. VitePress typically handles routing without file extensions.Apply this diff to standardize the link format:
{ text: 'Plugin', collapsed: false, items: [ - { text: 'Plugin System Introduction', link: '/guide/plugin/introduction.md' }, + { text: 'Plugin System Introduction', link: '/guide/plugin/introduction' }, { text: 'Getting Started with Plugin Development', - link: '/guide/plugin/getting-started.md' + link: '/guide/plugin/getting-started' }, - { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle.md' }, - { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies.md' }, - { text: 'Plugin Decorators', link: '/guide/plugin/decorators.md' }, - { text: 'Plugin Extensions', link: '/guide/plugin/extensions.md' }, - { text: 'Plugin Type System', link: '/guide/plugin/type-system.md' }, - { text: 'Plugin Testing', link: '/guide/plugin/testing.md' }, - { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines.md' } + { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle' }, + { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies' }, + { text: 'Plugin Decorators', link: '/guide/plugin/decorators' }, + { text: 'Plugin Extensions', link: '/guide/plugin/extensions' }, + { text: 'Plugin Type System', link: '/guide/plugin/type-system' }, + { text: 'Plugin Testing', link: '/guide/plugin/testing' }, + { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines' } ] },packages/docs/src/guide/advanced/docs-gen.md (9)
7-15: Trim unused type import; rely on inference for usageText.
import type { Command } from 'gunshi'is unused.usageTexttype is inferable.Apply:
import { promises as fs } from 'node:fs' -import type { Command } from 'gunshi' // Define your command const command = define({ @@ - const usageText: string = await generate(null, command, { + const usageText = await generate(null, command, {Also applies to: 40-40
46-48: Ensure target directory exists before writing.Create
docs/to avoid ENOENT in fresh repos.// Now you can use the usage text to generate documentation - await fs.writeFile('docs/cli-usage.md', `# CLI Usage\n\n\`\`\`sh\n${usageText}\n\`\`\``, 'utf8') + await fs.mkdir('docs', { recursive: true }) + await fs.writeFile('docs/cli-usage.md', `# CLI Usage\n\n\`\`\`sh\n${usageText}\n\`\`\``, 'utf8')
56-63: Avoid leaking internals; soften return‑semantics claim.
- Saying “internally sets usageSilent: true” exposes implementation detail.
- “returns empty string if generation fails” is ambiguous.
Suggested copy:
-- **Silent Mode**: Internally sets `usageSilent: true` to capture output as a string rather than printing to console. When this flag is set, the internal `ctx.log` function is replaced with a no-op function, preventing console output while still returning the generated usage text +- **Silent**: Captures usage text as a string instead of printing to the console. - **No Execution**: Only generates usage text without running command logic -- **Return Value**: Returns the generated usage text as a string (or empty string if generation fails) +- **Return Value**: Resolves to the generated usage text. If no usage can be produced (e.g., no renderer), resolves to an empty string.
66-75: Clarify parameter names and subCommands type to match examples.
- First parameter name “command” conflicts with the second parameter.
- You use a plain object for
subCommands, not aMap.Suggested copy:
-- `command` (string | null): The sub-command name to generate documentation for. Pass `null` when generating documentation for the entry command or when you don't have sub-commands +- `subCommandName` (string | null): The sub-command name to generate documentation for. Pass `null` for the entry command. - `entry` (Command | LazyCommand): The command object containing the command definition, or a lazy command that will be loaded to get the command - `opts` (CliOptions): Optional configuration including: @@ - - `subCommands`: Map of sub-commands (if applicable) + - `subCommands`: Object map of sub-commands (e.g., `Record<string, Command>`)
80-120: Multiple-commands example is clear; add directory creation before writes.Add mkdir to avoid write failures and keep parity with the earlier example.
// Generate main help const mainUsage = await generate(null, mainCommand, cliOptions) - await fs.writeFile('docs/cli-main.md', `# CLI Usage\n\n\`\`\`sh\n${mainUsage}\n\`\`\``, 'utf8') + await fs.mkdir('docs', { recursive: true }) + await fs.writeFile('docs/cli-main.md', `# CLI Usage\n\n\`\`\`sh\n${mainUsage}\n\`\`\``, 'utf8') @@ for (const name of Object.keys(subCommands)) { const commandUsage = await generate(name, mainCommand, cliOptions) await fs.writeFile( `docs/cli-${name}.md`, `# ${name.charAt(0).toUpperCase() + name.slice(1)} Command\n\n\`\`\`sh\n${commandUsage}\n\`\`\``, 'utf8' ) }Also applies to: 122-125, 128-135, 138-138, 150-157
170-201: Rich docs: ensure docs directory exists.// Create rich documentation const documentation = ` @@ - await fs.writeFile('docs/data-processor.md', documentation, 'utf8') + await fs.mkdir('docs', { recursive: true }) + await fs.writeFile('docs/data-processor.md', documentation, 'utf8')Also applies to: 205-256
371-386: Renderer polish and signature simplification.
- Make the renderer
asyncand return a string directly.- Minor nicety: keep return type simple.
-function renderManPageUsage(ctx: CommandContext) { +async function renderManPageUsage(ctx: CommandContext) { const lines: string[] = [] @@ - return Promise.resolve(lines.join('\n')) + return lines.join('\n') }Also applies to: 391-409, 425-426
477-480: Prefer local devDependency over global tool.Recommend adding
marked-manas a devDependency and invoking vianpxfor reproducibility.-// npm install -g marked-man -// or add it to your project: npm install --save-dev marked-man +// npm install --save-dev marked-man +// npx marked-man --help
503-519: Add macOS note for updating man databaseOn macOS use sudo /usr/libexec/makewhatis -u instead of mandb; optionally mention man-db (mandb) can be installed via Homebrew or MacPorts for Linux-style behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/docs/src/.vitepress/config.ts(1 hunks)packages/docs/src/guide/advanced/docs-gen.md(17 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test on Node.js 20
🔇 Additional comments (8)
packages/docs/src/.vitepress/config.ts (3)
50-56: LGTM! Sidebar structure updated for v0.27 features.The additions to the Essentials section properly reflect the new documentation structure, including the updated "Declarative Configuration" link, new "Auto Usage Generation" section, and "Plugin System" introduction.
62-71: Address the missing redirect for 'custom-usage-generation' to prevent broken links.Based on the past review comment, there's a broken link reference to
guide/advanced/custom-usage-generationwhich should be redirected toguide/advanced/custom-rendering. The VitePress config needs a rewrites section to handle this redirect.Add the following rewrites configuration to prevent 404 errors:
export default defineConfig({ title: 'Gunshi', description: 'Modern JavaScript Command-line library', lastUpdated: true, cleanUrls: true, + + rewrites: { + 'guide/advanced/custom-usage-generation': 'guide/advanced/custom-rendering' + }, head: [
66-66: Link path verified — docs-gen.md exists.
Confirmed: packages/docs/src/guide/advanced/docs-gen.md is present; no change required.packages/docs/src/guide/advanced/docs-gen.md (5)
267-285: ESM __dirname pattern LGTM.The
fileURLToPath+path.dirnameapproach is correct and portable.
353-368: Great addition: man-page structure overview.Clear, useful breakdown of sections. Nice touch.
166-166: Overall: strong, practical docs.Examples are cohesive, TS-friendly, and actionable. Minor nits above aside, this is a solid improvement.
Also applies to: 267-272, 336-345, 537-558
553-553: Confirmed: relative link target exists. The file packages/docs/src/guide/advanced/custom-rendering.md exists; ./custom-rendering.md resolves correctly from docs-gen.md.
291-296: Verify CLI types and docs-gen setup
- Confirm
CliOptionsis exported in your installedgunshipackage’s type declarations.- Ensure
scripts/generate-docs.tsexists, add adocs:generatescript invoking it viatsx, and installtsxas a devDependency.
There was a problem hiding this comment.
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)
packages/docs/package.json (1)
25-35: Fix CI: update and commit pnpm-lock.yaml (frozen lockfile failing).CI fails at
pnpm install --frozen-lockfilebecause pnpm-lock.yaml is out of date with packages/docs/package.json — two devDependencies were added: mermaid@^11.12.0 and vitepress-plugin-mermaid@^2.0.17. Update the workspace lockfile and commit it.Run locally and push the updated lockfile:
pnpm -w install --no-frozen-lockfile rg -n 'vitepress-plugin-mermaid|\"mermaid\"' pnpm-lock.yaml || true git add pnpm-lock.yaml git commit -m "chore(docs): update pnpm-lock.yaml for new devDependencies" git pushFile: packages/docs/package.json (lines 25-35).
🧹 Nitpick comments (15)
packages/docs/src/guide/advanced/custom-rendering.md (5)
558-567: Add a language hint to satisfy markdownlint (MD040).The fenced block lacks a language. Use
textto keep formatting neutral.-``` +```text 1. Base renderer output: "Usage: my-cli [options]" 2. After oceanThemePlugin: "\x1b[38;5;39mUsage: my-cli [options]\x1b[0m" (colored) 3. After emojiPlugin: "📖 \x1b[38;5;39mUsage: my-cli [options]\x1b[0m" (emoji + colored)--- `519-552`: **Make the “Combining Multiple Rendering Plugins” snippet self‑contained (missing imports).** This block uses `cli` and `renderer()` without importing them, so it won’t run when copy‑pasted. ```diff ```ts +import { cli } from 'gunshi' +import renderer from '@gunshi/plugin-renderer' import { plugin } from 'gunshi/plugin'
411-419: Verify import path:gunshi/bonelooks inconsistent with other examples.Other snippets import
clifromgunshi. Unlessgunshi/boneis the intended public entry, prefer the consistent top‑level import.-import { cli } from 'gunshi/bone' +import { cli } from 'gunshi'
359-395: DefinecustomHeaderRendererin this example for copy‑paste completeness.You call
renderHeader: customHeaderRendererbut this block doesn’t define it. Add a minimal header renderer inline or define it above the CLI call.// Define custom validation errors renderer const customValidationErrorsRenderer = (ctx, error) => { const lines = [] @@ return lines.join('\n') } +// Minimal header renderer for completeness +const customHeaderRenderer = (ctx) => `${ctx.env.name} v${ctx.env.version}\n` + // Run the CLI with all custom renderers await cli(process.argv.slice(2), command, { name: 'task-manager', version: '1.0.0', description: 'A task management utility', renderHeader: customHeaderRenderer, renderUsage: customUsageRenderer, renderValidationErrors: customValidationErrorsRenderer })
403-405: Terminology consistency: use “CLI level” instead of “global‑level”.Elsewhere you use “CLI Level”. Align wording to avoid confusion.
-> Plugin-level rendering has the lowest priority in the rendering hierarchy. Command-level and global-level renderers will override plugin decorators. For detailed information on how renderer decorators work and how to implement them, see the [How Renderer Decorators Work](../plugin/decorators.md#how-renderer-decorators-work) documentation. +> Plugin-level rendering has the lowest priority in the rendering hierarchy. Command-level and CLI-level renderers will override plugin decorators. For detailed information on how renderer decorators work and how to implement them, see the [How Renderer Decorators Work](../plugin/decorators.md#how-renderer-decorators-work) documentation.packages/docs/src/guide/plugin/testing.md (1)
7-12: Fix MD028 (blank line inside blockquote) in admonitions.Ensure no blank line inside the NOTE/IMPORTANT blocks to satisfy markdownlint.
packages/docs/src/guide/essentials/plugin-system.md (1)
190-195: Fix MD028 (blank line inside blockquote).Remove any blank line within the NOTE block.
packages/docs/src/guide/essentials/getting-started.md (1)
131-133: Complete the sentence describing help output.Current text ends abruptly.
-You'll see a help message that includes. +You'll see a help message that includes usage, available options, and descriptions.packages/docs/src/guide/essentials/composable.md (2)
125-135: Add a language to the project tree code fence (MD040).Use “text” to silence markdownlint and improve rendering.
-``` +```text my-cli/ ├── src/ │ ├── commands/ │ │ ├── create.ts # Create command implementation │ │ └── list.ts # List command implementation │ ├── main.ts # Main command definition │ └── cli.ts # CLI entry point ├── package.json └── tsconfig.json--- `105-107`: **Fix MD028 (blank line inside blockquote) in TIP.** Ensure no blank line between the blockquote marker and content. </blockquote></details> <details> <summary>packages/docs/src/.vitepress/config.ts (3)</summary><blockquote> `1-1`: **Remove commented-out import.** The commented import for `defineConfig` should be removed since it's no longer used with the new `withMermaid` configuration approach. ```diff -// import { defineConfig } from 'vitepress'
83-86: Remove.mdextensions from plugin links for consistency.The plugin section links include
.mdextensions while other sections don't. Remove these extensions for consistency with the rest of the navigation structure.- { text: 'Plugin System Introduction', link: '/guide/plugin/introduction.md' }, + { text: 'Plugin System Introduction', link: '/guide/plugin/introduction' }, { text: 'Getting Started with Plugin Development', - link: '/guide/plugin/getting-started.md' + link: '/guide/plugin/getting-started' }, - { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle.md' }, - { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies.md' }, - { text: 'Plugin Decorators', link: '/guide/plugin/decorators.md' }, - { text: 'Plugin Extensions', link: '/guide/plugin/extensions.md' }, - { text: 'Plugin Type System', link: '/guide/plugin/type-system.md' }, - { text: 'Plugin Testing', link: '/guide/plugin/testing.md' }, - { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines.md' } + { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle' }, + { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies' }, + { text: 'Plugin Decorators', link: '/guide/plugin/decorators' }, + { text: 'Plugin Extensions', link: '/guide/plugin/extensions' }, + { text: 'Plugin Type System', link: '/guide/plugin/type-system' }, + { text: 'Plugin Testing', link: '/guide/plugin/testing' }, + { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines' }
137-158: Consider extracting path resolution aliases to a separate configuration.The alias configuration contains hardcoded paths to specific package versions in node_modules. Consider extracting these to a separate configuration file or using a more maintainable approach.
Consider creating a separate
vite.aliases.tsfile:// vite.aliases.ts import path from 'node:path' import { fileURLToPath } from 'node:url' const __dirname = path.dirname(fileURLToPath(import.meta.url)) export const devAliases = { debug: path.resolve(__dirname, '../../../../node_modules/.pnpm/debug@4.4.1/node_modules/debug/src/browser.js'), '@braintree/sanitize-url': path.resolve(__dirname, '../../../../node_modules/.pnpm/@braintree+sanitize-url@7.1.1/node_modules/@braintree/sanitize-url/dist/index.js'), dayjs: path.resolve(__dirname, '../../../../node_modules/.pnpm/dayjs@1.11.18/node_modules/dayjs/esm/index.js') }Then import and use it in the main config:
+import { devAliases } from './vite.aliases' vite: { optimizeDeps: { include: ['@braintree/sanitize-url', 'dayjs', 'debug', 'cytoscape-cose-bilkent', 'cytoscape'] }, resolve: { - alias: - process.env.NODE_ENV !== 'production' - ? { - debug: path.resolve( - __dirname, - '../../../../node_modules/.pnpm/debug@4.4.1/node_modules/debug/src/browser.js' - ), - '@braintree/sanitize-url': path.resolve( - __dirname, - '../../../../node_modules/.pnpm/@braintree+sanitize-url@7.1.1/node_modules/@braintree/sanitize-url/dist/index.js' - ), - dayjs: path.resolve( - __dirname, - '../../../../node_modules/.pnpm/dayjs@1.11.18/node_modules/dayjs/esm/index.js' - ) - } - : undefined + alias: process.env.NODE_ENV !== 'production' ? devAliases : undefined },packages/docs/package.json (2)
29-29: Mermaid v11 often needs SSR bundling hints in VitePress.To avoid SSR bundling errors, add this to docs/.vitepress/config.(ts|js):
import { defineConfig } from 'vitepress' export default defineConfig({ // If you wire the plugin, keep its config too (see next comment). vite: { optimizeDeps: { include: ['mermaid'] }, ssr: { noExternal: ['mermaid'] } } })This pattern is commonly used with VitePress + Mermaid. Example configurations in community docs show the same optimizeDeps/noExternal combo. (vitepress.yiov.top)
25-35: Optional: consider alternatives and maintenance posture.
- Current plugin appears maintained on npm (2.0.17). If you want server-side SVG caching or broader diagram types, consider vitepress-plugin-diagrams. This is optional and depends on your needs. (socket.dev)
Would you like me to draft the .vitepress/config.ts changes for your chosen path and add a smoke test page with a couple of Mermaid blocks?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/docs/package.json(1 hunks)packages/docs/src/.vitepress/config.ts(3 hunks)packages/docs/src/guide/advanced/command-hooks.md(1 hunks)packages/docs/src/guide/advanced/context-extensions.md(1 hunks)packages/docs/src/guide/advanced/custom-rendering.md(1 hunks)packages/docs/src/guide/advanced/docs-gen.md(16 hunks)packages/docs/src/guide/advanced/internationalization.md(1 hunks)packages/docs/src/guide/advanced/type-system.md(1 hunks)packages/docs/src/guide/essentials/auto-usage.md(1 hunks)packages/docs/src/guide/essentials/composable.md(2 hunks)packages/docs/src/guide/essentials/declarative.md(1 hunks)packages/docs/src/guide/essentials/getting-started.md(7 hunks)packages/docs/src/guide/essentials/lazy-async.md(1 hunks)packages/docs/src/guide/essentials/plugin-system.md(1 hunks)packages/docs/src/guide/essentials/type-safe.md(3 hunks)packages/docs/src/guide/plugin/decorators.md(1 hunks)packages/docs/src/guide/plugin/dependencies.md(1 hunks)packages/docs/src/guide/plugin/extensions.md(1 hunks)packages/docs/src/guide/plugin/guidelines.md(1 hunks)packages/docs/src/guide/plugin/introduction.md(1 hunks)packages/docs/src/guide/plugin/lifecycle.md(1 hunks)packages/docs/src/guide/plugin/testing.md(1 hunks)packages/docs/src/guide/plugin/type-system.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/docs/src/guide/essentials/auto-usage.md
- packages/docs/src/guide/plugin/introduction.md
- packages/docs/src/guide/essentials/type-safe.md
- packages/docs/src/guide/plugin/extensions.md
- packages/docs/src/guide/essentials/declarative.md
- packages/docs/src/guide/advanced/context-extensions.md
- packages/docs/src/guide/advanced/docs-gen.md
- packages/docs/src/guide/advanced/internationalization.md
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Applied to files:
packages/docs/src/guide/plugin/type-system.mdpackages/docs/src/guide/advanced/type-system.md
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: The project supports multiple JavaScript runtimes - ensure changes work across Node.js, Deno, and Bun
Applied to files:
packages/docs/src/guide/essentials/getting-started.md
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Applied to files:
packages/docs/src/guide/plugin/testing.md
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Mock external dependencies when needed in tests
Applied to files:
packages/docs/src/guide/plugin/testing.md
🪛 markdownlint-cli2 (0.18.1)
packages/docs/src/guide/essentials/plugin-system.md
192-192: Blank line inside blockquote
(MD028, no-blanks-blockquote)
packages/docs/src/guide/essentials/composable.md
107-107: Blank line inside blockquote
(MD028, no-blanks-blockquote)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/docs/src/guide/advanced/custom-rendering.md
558-558: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/docs/src/guide/plugin/decorators.md
107-107: Blank line inside blockquote
(MD028, no-blanks-blockquote)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/docs/src/guide/plugin/testing.md
9-9: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🪛 GitHub Actions: CI
packages/docs/package.json
[error] 1-1: Process completed with exit code 1 during 'pnpm install --frozen-lockfile'.
🔇 Additional comments (26)
packages/docs/src/guide/plugin/decorators.md (2)
90-96: LGTM! The comment is now correctly aligned with wrapper semantics.The inline comment "Registered third (innermost wrapper; executes after A and B)" correctly reflects that with reduceRight, the third decorator becomes the innermost wrapper whose "before" runs after A and B.
108-111: LGTM! The execution flow examples now correctly show composition model.The renderer decorator chain examples properly demonstrate that each decorator receives a baseRenderer function and returns a new renderer, rather than the incorrect previous model that showed decorators passing rendered strings.
Also applies to: 337-345
packages/docs/src/guide/advanced/command-hooks.md (3)
92-102: Hook parameter types are now properly defined with unknown.The type definitions correctly use
unknownfor the result parameter inonAfterCommandand error parameter inonErrorCommand, providing appropriate type safety without constraining values unnecessarily.
113-129: Timestamp handling is now properly implemented.The code correctly records a start time in
onBeforeCommandand uses it inonAfterCommandto calculate duration, avoiding the NaN issue that would occur without recording the timestamp.
235-244: Error handling now properly narrows unknown error types.The code correctly narrows unknown errors to Error instances before accessing properties like
.messageand.stack, providing safe type handling throughout the hook implementations.packages/docs/src/guide/plugin/guidelines.md (3)
118-131: Plugin ID should use consistent namespaced format.The example uses a plain
id: 'logger'which contradicts the earlier recommendation to use namespaced IDs. Consider updating to use a namespaced format like'docs:logger'or'@docs/logger'to align with the guidance in the document.
158-159: Fix relative link path to Plugin Type System.The link
./type-system.mdshould be../plugin/type-system.mdsince this file is inguide/plugin/and the targettype-system.mdis also inguide/plugin/, not inguide/advanced/.Wait, let me check the actual file locations from the file names provided...
Looking at the provided files, I can see:
- This file is at
packages/docs/src/guide/plugin/guidelines.md- The target file is at
packages/docs/src/guide/plugin/type-system.mdSo both files are in the same directory (
guide/plugin/), making the correct relative link./type-system.md. The existing link is actually correct.
457-471: Path validation still uses unsafe techniques.The current path validation using
path.includes('..')andpath.startsWith('/')is insufficient for preventing path traversal attacks across different platforms. Use Node.js path utilities for robust validation:readFile: async (path: string) => { + const path = require('path') + const basePath = path.resolve('./safe-directory') + const targetPath = path.resolve(basePath, relPath) + // Prevent path traversal - if (path.includes('..') || path.startsWith('/')) { + if (!targetPath.startsWith(basePath + path.sep) && targetPath !== basePath) { throw new Error('Invalid file path') } // Validate file extension (case-insensitive) const allowed = ['.json', '.yaml', '.yml'] - if (!allowed.some(ext => path.endsWith(ext))) { + if (!allowed.some(ext => targetPath.toLowerCase().endsWith(ext))) { throw new Error('Unsupported file type') } - return await fs.readFile(path, 'utf-8') + return await fs.readFile(targetPath, 'utf-8') }packages/docs/src/guide/advanced/type-system.md (2)
23-55: LGTM! Type system documentation is comprehensive and accurate.The documentation correctly explains the
GunshiParamstype and its usage with conditional types. The examples clearly demonstrate how to use type parameters for end-to-end type safety across commands and plugin extensions.
314-423: Plugin extension type combination examples are well structured.The intersection operator usage and Record-based mappings are properly demonstrated for both official Gunshi plugins (with plugin IDs) and custom plugins. The examples provide clear guidance for different scenarios.
packages/docs/src/guide/plugin/type-system.md (3)
173-181: Fix mapped type syntax for dependency extension keys.The current syntax using computed property names with const values is invalid TypeScript. Use mapped types instead:
-type DependencyExtensions = { - [loggerId]: LoggerExtension - [authId]: AuthExtension -} +type DependencyExtensions = + { [K in typeof loggerId]: LoggerExtension } & + { [K in typeof authId]: AuthExtension }
222-227: Apply same mapped type fix for optional dependencies.Use proper mapped type syntax for optional dependency keys:
-type DependencyExtensions = { - [loggerId]: LoggerExtension - [cacheId]?: CacheExtension // Optional with ? -} +type DependencyExtensions = + { [K in typeof loggerId]: LoggerExtension } & // Required + { [K in typeof cacheId]?: CacheExtension } // Optional
329-337: Fix mapped types in multi-dependency example.Apply the same mapped type pattern:
-export default plugin< - { - [baseId]: BaseExtension - [loggerId]: LoggerExtension - }, +export default plugin< + { [K in typeof baseId]: BaseExtension } & + { [K in typeof loggerId]: LoggerExtension }, typeof apiId, typeof apiDeps, ApiExtension >({packages/docs/src/guide/plugin/lifecycle.md (2)
182-184: LGTM! Execution order comment is now accurate.The comment correctly shows "A → B → C → original command → C → B → A" which properly reflects the LIFO wrapper structure where A is outermost, C is innermost.
217-240: Mermaid diagrams need rendering support in VitePress.The documentation contains multiple Mermaid diagrams that won't render without proper markdown plugin configuration. The VitePress setup needs a Mermaid plugin added.
packages/docs/src/guide/plugin/dependencies.md (1)
38-48: Dependency graph and descriptions are now consistent.The Mermaid diagram, bullet points, and loading order have been corrected to properly reflect the dependency relationships where Auth depends on Cache, not vice versa. The loading order Logger → Cache → Auth → API is accurate.
Also applies to: 52-56, 57-73
packages/docs/src/guide/essentials/lazy-async.md (3)
282-283: LGTM! TypeScriptsatisfiesusage is correct.The use of
satisfies Commandis valid TypeScript syntax since thesatisfieskeyword was introduced in TypeScript 4.9. This provides type checking while preserving the inferred type of the command definition.
286-294: Type-safe command runner implementation is well structured.The generic type parameters
CommandRunner<GunshiParams<{ args: ProcessDataArgs }>>andCommandContext<GunshiParams<{ args: ProcessDataArgs }>>provide end-to-end type safety from definition to execution context.
197-219: Node.js TypeScript support information needs clarification.The documentation about Node.js flags needs updating to reflect the current status. The information about
--experimental-transform-typesbeing a "rename" of--experimental-strip-typesis incorrect.packages/docs/src/guide/plugin/testing.md (1)
254-274: Simplify extensions wiring; support factory function/object/prebuilt shapes.The helper still uses defineProperty + Object.assign and assumes
.factory. Prefer direct assignment and flexible handling (function | {factory,onFactory?} | prebuilt object).- if (options.extensions) { - const ext = {} as any - Object.defineProperty(ctx, 'extensions', { - value: ext, - writable: false, - enumerable: true, - configurable: true - }) - for (const [key, extension] of Object.entries(options.extensions)) { - ext[key] = await (extension as CommandContextExtension).factory( - ctx, - options.command as Command - ) - if (extension.onFactory) { - await extension.onFactory(ctx as CommandContext, options.command as Command) - } - } - ctx = Object.assign({}, ctx, { extensions: ext }) - } else { - ctx = Object.assign({}, ctx, { extensions: {} }) - } + if (options.extensions) { + const ext: Record<string, unknown> = {} + for (const [key, extension] of Object.entries(options.extensions)) { + const maybe: any = extension + if (typeof maybe === 'function') { + ext[key] = await maybe(ctx, options.command as Command) + } else if (maybe && typeof maybe.factory === 'function') { + ext[key] = await maybe.factory(ctx, options.command as Command) + if (typeof maybe.onFactory === 'function') { + await maybe.onFactory(ctx as CommandContext, options.command as Command) + } + } else { + ext[key] = maybe + } + } + ctx.extensions = ext + } else { + ctx.extensions = {} + }packages/docs/src/guide/essentials/plugin-system.md (3)
126-127: Use exported i18nId instead of the string key 'g:i18n'.Accessing
ctx.extensionsvia string is brittle; import and usei18nIdfor type-safety.-import i18n, { defineI18n, resolveKey } from '@gunshi/plugin-i18n' +import i18n, { defineI18n, resolveKey, i18nId } from '@gunshi/plugin-i18n' @@ - const locale = ctx.extensions['g:i18n'].locale + const locale = ctx.extensions[i18nId].locale @@ - const t = ctx.extensions['g:i18n'].translate + const t = ctx.extensions[i18nId].translateAlso applies to: 149-166
172-175: Normalize LANG to BCP‑47 (en_US.UTF-8->en-US).Prevents fallback misses when env provides underscore/encoding formats.
- i18n({ - locale: process.env.LANG || 'en-US' - }) + i18n({ + locale: (process.env.LANG || 'en-US').replace('_', '-').split('.')[0] + })
298-329: Apply i18nId in “Combining Plugins” example too.Keep imports/usage consistent across docs.
-import i18n, { defineI18n, resolveKey } from '@gunshi/plugin-i18n' +import i18n, { defineI18n, resolveKey, i18nId } from '@gunshi/plugin-i18n' @@ - const locale = ctx.extensions['g:i18n'].locale + const locale = ctx.extensions[i18nId].locale @@ - const t = ctx.extensions['g:i18n'].translate + const t = ctx.extensions[i18nId].translatepackages/docs/src/guide/essentials/composable.md (1)
108-116: Node TypeScript note looks correct for v22.6 flag and v23.6+ default.Nice update reflecting current Node behavior.
packages/docs/src/.vitepress/config.ts (2)
12-165: Add URL rewrite for renamed documentation path.Based on the previous review comment, the path
guide/advanced/custom-usage-generationneeds to be redirected to prevent broken links. The old path is still referenced indocumentation-generation.md.Add this configuration to the exported config object to handle the redirect:
export default withMermaid({ title: 'Gunshi', description: 'Modern JavaScript Command-line library', lastUpdated: true, cleanUrls: true, + + rewrites: { + 'guide/advanced/custom-usage-generation.md': 'guide/advanced/custom-rendering.md' + },
163-164: LGTM! Mermaid configuration properly added.The Mermaid plugin integration with the 'forest' theme and custom CSS classes is correctly configured.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docs/src/.vitepress/config.ts (1)
1-167: Add URL rewrites; remove.mdlinks; confirm Node engine
- No
rewritesblock in packages/docs/src/.vitepress/config.ts — add redirects for renamed slugs.- Sidebar contains explicit
.mdlinks in packages/docs/src/.vitepress/config.ts (lines 83, 86, 88–95) and a content link at packages/docs/src/guide/advanced/command-hooks.md:123 — remove the.mdsuffixes or add rewrites so URLs still resolve.- package.json engines are
>=20and several>=24— ensure CI/dev Node meets the highest minimum (>=24) or adjust JSON import usage accordingly.
🧹 Nitpick comments (36)
packages/docs/src/guide/essentials/lazy-async.md (3)
90-111: Avoid fragile line-number references in proseThe note mentions “lines 55–57,” which will drift as the doc evolves. Remove the hardcoded line numbers.
-> > The `commandName` property on `LazyCommand` can be `undefined` at the TypeScript type level, which is why the example includes a runtime check at lines 55-57. This happens because the `lazy()` function accepts an optional definition parameter - when omitted, there's no guaranteed command name. +> The `commandName` property on `LazyCommand` can be `undefined` at the TypeScript type level, which is why the example includes a runtime check. This happens because the `lazy()` function accepts an optional definition parameter—when omitted, there's no guaranteed command name.
183-195: Top‑level await: add CJS fallback noteExamples use top‑level
await. Add a short note for CommonJS users showing an async IIFE fallback.) )
+> [!NOTE]
+> If your project is CommonJS (no ESM/top‑levelawait), wrap the call:
+>
+>ts +> (async () => { +> await cli(process.argv.slice(2), { /* ... */ }, { /* ... */ }) +> })() +>--- `338-338`: **Remove TODO from docs or link a tracking issue** The inline TODO (“gunshiにバグがあるので治す”) shouldn’t ship in user docs. Replace with a neutral note or link to a public issue and summarize impact. Want me to open a docs issue and propose wording that avoids referencing internal TODOs? </blockquote></details> <details> <summary>packages/docs/src/guide/plugin/dependencies.md (4)</summary><blockquote> `236-236`: **Fix MD036 on “Problem” label.** Use a heading instead of bold text. Apply: ```diff -**Problem: Circular dependency between two plugins** +#### Problem: Circular dependency between two plugins
266-266: Fix MD036 on “Solution” label.Use a heading instead of bold text.
Apply:
-**Solution: Extract shared functionality into a common plugin** +#### Solution: Extract shared functionality into a common plugin
269-269: Import path inconsistency and undefined symbols in the snippet.This block imports
clifromgunshi/pluginand later callscli(args, command, …)with undefinedargs/command. Elsewhere,cliis imported fromgunshi. Align import and either remove the call here or reference the CLI example below.Apply:
-import { plugin, cli } from 'gunshi/plugin' +import { plugin } from 'gunshi/plugin' @@ -// Usage - no circular dependency! -await cli(args, command, { - plugins: [ - sharedPlugin, // Loads first - pluginA, // Loads second (depends on shared) - pluginB // Loads third (depends on shared) - ] -}) +// Usage is demonstrated in the CLI example below.Also applies to: 311-317
198-199: Keep case consistent in diagnostic comments.IDs in messages should match the examples’ casing.
Apply:
-// Circular dependency detected: `a -> b -> a` +// Circular dependency detected: `A -> B -> A` @@ -// Circular dependency detected: `x -> y -> z -> x` +// Circular dependency detected: `X -> Y -> Z -> X`Also applies to: 225-226
packages/docs/src/guide/advanced/docs-gen.md (6)
73-79: ClarifysubCommandstype: it's an object/record, not a Map.The text says “Map,” but examples use a plain object. Align the description to avoid confusion.
- - `subCommands`: Map of sub-commands (if applicable) + - `subCommands`: Record of sub-commands (e.g., { [name]: Command })
129-134: TypesubCommandsand fix the comment to match usage.Consistently present it as a Record to match the code and likely
CliOptionsshape.-// Create a Map of sub-commands -const subCommands = { +// Create an object (Record<string, Command>) of sub-commands +const subCommands: Record<string, Command> = { create: createCommand, list: listCommand }Also applies to: 146-151, 158-165
275-280: Correct Deno note:import.meta.dirnamedoes not exist in Deno.Use
fromFileUrl(import.meta.url)with std/path, or URL-based resolution.-> - **Deno**: Use `import.meta.dirname` or `fromFileUrl(import.meta.url)` +> - **Deno**: Use `fromFileUrl(import.meta.url)` with `dirname` from std/path +> (e.g., `dirname(fromFileUrl(import.meta.url))`), or +> `new URL('.', import.meta.url).pathname`
48-50: Ensure target directory exists before writing.Avoids ENOENT when
docs/isn’t present.- // Now you can use the usage text to generate documentation - await fs.writeFile('docs/cli-usage.md', `# CLI Usage\n\n\`\`\`sh\n${usageText}\n\`\`\``, 'utf8') + // Now you can use the usage text to generate documentation + await fs.mkdir('docs', { recursive: true }) + await fs.writeFile('docs/cli-usage.md', `# CLI Usage\n\n\`\`\`sh\n${usageText}\n\`\`\``, 'utf8')
153-156: Create docs dir in multi-command example before writes.Prevents failures on fresh repos/CI.
// Generate main help const mainUsage = await generate(null, mainCommand, cliOptions) - await fs.writeFile('docs/cli-main.md', `# CLI Usage\n\n\`\`\`sh\n${mainUsage}\n\`\`\``, 'utf8') + await fs.mkdir('docs', { recursive: true }) + await fs.writeFile('docs/cli-main.md', `# CLI Usage\n\n\`\`\`sh\n${mainUsage}\n\`\`\``, 'utf8') // Generate help for each sub-command for (const name of Object.keys(subCommands)) { const commandUsage = await generate(name, mainCommand, cliOptions) await fs.writeFile( `docs/cli-${name}.md`, `# ${name.charAt(0).toUpperCase() + name.slice(1)} Command\n\n\`\`\`sh\n${commandUsage}\n\`\`\``, 'utf8' ) }Also applies to: 158-165
334-336: EnsuredocsDirexists before writing.Minor reliability improvement in the automation script.
- await fs.writeFile(path.join(docsDir, 'cli-reference.md'), fullReference, 'utf8') + await fs.mkdir(docsDir, { recursive: true }) + await fs.writeFile(path.join(docsDir, 'cli-reference.md'), fullReference, 'utf8')packages/docs/src/guide/plugin/extensions.md (3)
59-62: Clarify lifecycle step labels (H/H collision).Both “Extension Creation” and “Post‑Extension Hook” are marked Step H. Either rename to H1/H2 or align to the exact letters used in lifecycle.md.
20-26: Use namespaced plugin IDs in examples to avoid copy‑paste collisions.Prefer an explicit namespace, e.g., example:logger / example:database.
- id: 'logger', + id: 'example:logger', ... - id: 'database', + id: 'example:database', ... - id: pluginId, + id: 'example:logger',Also show access via ctx.extensions['example:logger'] in snippets that reference logger.
Also applies to: 170-175, 233-241
45-46: Standardize callouts for VitePress.Replace GitHub-style [!NOTE]/[!TIP]/[!IMPORTANT] with VitePress containers.
- > [!TIP] - > **It's strongly recommended ... +::: tip +**It's strongly recommended ... +:::(Apply similarly to NOTE/IMPORTANT blocks in this file.)
Also applies to: 54-56, 111-113, 116-118, 167-175, 187-201
packages/docs/src/guide/essentials/composable.md (2)
125-135: Add a language to the project-tree fence to satisfy MD040.-``` +```text my-cli/ ├── src/ ...--- `105-116`: **Fix blockquote spacing and adopt VitePress callouts.** Avoid blank lines inside blockquotes (MD028) or switch to ::: tip/::: note. ```diff - > [!TIP] - > [`tsx`](https://github.com/privatenumber/tsx) ... +::: tip +[`tsx`](https://github.com/privatenumber/tsx) ... +:::(Apply to the NOTE at Lines 109–116 and NOTE at Lines 209–211.)
Also applies to: 209-211
packages/docs/src/guide/plugin/list.md (1)
25-27: Tighten microcopy and use VitePress callout.- > [!NOTE] - > Welcome your plugins! Submit a PR to add your plugin to this list. +::: info +We welcome your plugins. Submit a PR to add your plugin to this list. +:::packages/docs/src/guide/essentials/plugin-system.md (1)
179-181: Replace GitHub callouts with VitePress containers; remove blank lines in blockquotes (MD028).- > [!NOTE] - > Plugin IDs use namespacing ... +::: note +Plugin IDs use namespacing ... +:::(Apply similarly to other NOTE/IMPORTANT blocks.)
Also applies to: 190-195, 216-218
packages/docs/src/guide/plugin/decorators.md (1)
201-203: Adopt VitePress callouts for consistency.Replace [!NOTE]/[!IMPORTANT] with ::: note/::: important.
Also applies to: 403-405, 460-462
packages/docs/src/guide/plugin/guidelines.md (1)
7-12: Unify callouts (MD028) — use VitePress containers.Replace [!TIP]/[!NOTE] with ::: tip/::: note and remove blank lines in quoted blocks.
Also applies to: 62-64, 778-780
packages/docs/src/guide/advanced/custom-rendering.md (2)
403-405: Standardize callouts to VitePress containers.Replace [!NOTE]/[!TIP]/[!IMPORTANT] with ::: note/tip/important.
Also applies to: 426-428, 569-571, 748-750
559-567: Add language to the transformation chain fence (MD040).-``` +```text 1. Base renderer output: "Usage: my-cli [options]" ...</blockquote></details> <details> <summary>packages/docs/src/guide/plugin/lifecycle.md (1)</summary><blockquote> `385-385`: **Use the right file name in the error-path example.** Earlier you run node cli.js; keep it consistent here. ```diff -node index.js --fail +node cli.js --failpackages/docs/src/guide/plugin/testing.md (8)
254-274: Simplify extensions wiring and accept factory/function/object shapes.Avoid defineProperty/Object.assign churn and support flexible inputs to match examples.
- if (options.extensions) { - const ext = {} as any - Object.defineProperty(ctx, 'extensions', { - value: ext, - writable: false, - enumerable: true, - configurable: true - }) - for (const [key, extension] of Object.entries(options.extensions)) { - ext[key] = await (extension as CommandContextExtension).factory( - ctx, - options.command as Command - ) - if (extension.onFactory) { - await extension.onFactory(ctx as CommandContext, options.command as Command) - } - } - ctx = Object.assign({}, ctx, { extensions: ext }) - } else { - ctx = Object.assign({}, ctx, { extensions: {} }) - } + if (options.extensions) { + const ext: Record<string, unknown> = {} + for (const [key, extension] of Object.entries(options.extensions)) { + const maybe: any = extension + if (typeof maybe === 'function') { + ext[key] = await maybe(ctx, options.command as Command) + } else if (maybe && typeof maybe.factory === 'function') { + ext[key] = await maybe.factory(ctx, options.command as Command) + if (typeof maybe.onFactory === 'function') { + await maybe.onFactory(ctx as CommandContext, options.command as Command) + } + } else { + ext[key] = maybe + } + } + ctx.extensions = ext + } else { + ctx.extensions = {} + }
371-382: Fix example: use myPlugin (createMyPlugin is undefined).- const plugin1 = createMyPlugin() - const plugin2 = createMyPlugin() + const plugin1 = myPlugin() + const plugin2 = myPlugin()
494-496: Correct mixed default/named imports from the same file.-import { myValidatingPlugin, myStrictPlugin, myPlugin } from './plugin.ts' +import myPlugin from './plugin.ts' +import { myValidatingPlugin, myStrictPlugin } from './plugin.ts'
949-951: Import types from the types module, not the plugin file.-import type { MyExtension } from './plugin.ts' +import type { MyExtension } from './types.ts'
1268-1271: Call the plugin’s setup, don’t invoke a non-existent function.cliEnhancer is a plugin object; execute its setup method.
- // Execute the plugin function directly with the plugin context - await cliEnhancerPlugin(pluginContext) + // Invoke plugin setup with the plugin context + await cliEnhancer.setup(pluginContext as any)Also applies to: 1300-1302, 1340-1342
1554-1563: Align import and factory usage for logging plugin.Export is a named function logging(); tests should import and call it.
-import { describe, expect, test, vi } from 'vitest' -import { cli, define } from 'gunshi' -import logging from './logging.ts' +import { describe, expect, test, vi } from 'vitest' +import { cli, define } from 'gunshi' +import { logging } from './logging.ts' @@ - const plugin = createLoggingPlugin() + const plugin = logging()
1581-1593: Same fix for second logging test.- const plugin = createLoggingPlugin() + const plugin = logging()
1709-1712: Use the actual plugin factories in interaction tests.- const loggerPlugin = createLoggerPlugin() - const notificationPlugin = createNotificationPlugin() + const loggerPlugin = logger() + const notificationPlugin = notification() @@ - const loggerPlugin = createLoggerPlugin() + const loggerPlugin = logger()Also applies to: 1800-1803
packages/docs/src/.vitepress/theme/custom.css (1)
27-32: Mermaid SVGs: prevent clipping and ensure full-width scalingTo avoid edge/marker clipping and guarantee responsive fill, add width and overflow controls.
.mermaid > svg { display: block; margin: 0 auto; + width: 100%; height: auto; max-width: 100%; + overflow: visible; }packages/docs/src/.vitepress/config.ts (2)
80-96: Prefer extensionless sidebar linksVitePress routes resolve without “.md”. Dropping extensions keeps URLs stable if file types change.
- { text: 'Plugin System Introduction', link: '/guide/plugin/introduction.md' }, + { text: 'Plugin System Introduction', link: '/guide/plugin/introduction' }, @@ - link: '/guide/plugin/getting-started.md' + link: '/guide/plugin/getting-started' @@ - { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle.md' }, - { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies.md' }, - { text: 'Plugin Decorators', link: '/guide/plugin/decorators.md' }, - { text: 'Plugin Extensions', link: '/guide/plugin/extensions.md' }, - { text: 'Plugin Type System', link: '/guide/plugin/type-system.md' }, - { text: 'Plugin Testing', link: '/guide/plugin/testing.md' }, - { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines.md' }, - { text: 'Plugin List', link: '/guide/plugin/list.md' } + { text: 'Plugin Lifecycle', link: '/guide/plugin/lifecycle' }, + { text: 'Plugin Dependencies', link: '/guide/plugin/dependencies' }, + { text: 'Plugin Decorators', link: '/guide/plugin/decorators' }, + { text: 'Plugin Extensions', link: '/guide/plugin/extensions' }, + { text: 'Plugin Type System', link: '/guide/plugin/type-system' }, + { text: 'Plugin Testing', link: '/guide/plugin/testing' }, + { text: 'Plugin Development Guidelines', link: '/guide/plugin/guidelines' }, + { text: 'Plugin List', link: '/guide/plugin/list' }
138-160: Avoid hard‑coded pnpm store paths in aliases; resolve from module IDsThese absolute .pnpm paths are brittle across machines/CI. Use require.resolve instead.
+import { createRequire } from 'node:module' @@ - vite: { + vite: { @@ - resolve: { - alias: - process.env.NODE_ENV !== 'production' - ? { - debug: path.resolve( - __dirname, - '../../../../node_modules/.pnpm/debug@4.4.1/node_modules/debug/src/browser.js' - ), - '@braintree/sanitize-url': path.resolve( - __dirname, - '../../../../node_modules/.pnpm/@braintree+sanitize-url@7.1.1/node_modules/@braintree/sanitize-url/dist/index.js' - ), - dayjs: path.resolve( - __dirname, - '../../../../node_modules/.pnpm/dayjs@1.11.18/node_modules/dayjs/esm/index.js' - ) - } - : undefined - }, + resolve: { + alias: (() => { + if (process.env.NODE_ENV === 'production') return undefined + const require = createRequire(import.meta.url) + return { + debug: require.resolve('debug/src/browser.js'), + '@braintree/sanitize-url': require.resolve('@braintree/sanitize-url/dist/index.js'), + dayjs: require.resolve('dayjs/esm/index.js') + } + })() + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
packages/docs/src/.vitepress/config.ts(3 hunks)packages/docs/src/.vitepress/theme/custom.css(1 hunks)packages/docs/src/guide/advanced/command-hooks.md(1 hunks)packages/docs/src/guide/advanced/context-extensions.md(1 hunks)packages/docs/src/guide/advanced/custom-rendering.md(1 hunks)packages/docs/src/guide/advanced/docs-gen.md(17 hunks)packages/docs/src/guide/essentials/composable.md(2 hunks)packages/docs/src/guide/essentials/declarative.md(1 hunks)packages/docs/src/guide/essentials/getting-started.md(7 hunks)packages/docs/src/guide/essentials/lazy-async.md(1 hunks)packages/docs/src/guide/essentials/plugin-system.md(1 hunks)packages/docs/src/guide/introduction/what-is-gunshi.md(2 hunks)packages/docs/src/guide/plugin/decorators.md(1 hunks)packages/docs/src/guide/plugin/dependencies.md(1 hunks)packages/docs/src/guide/plugin/extensions.md(1 hunks)packages/docs/src/guide/plugin/getting-started.md(1 hunks)packages/docs/src/guide/plugin/guidelines.md(1 hunks)packages/docs/src/guide/plugin/introduction.md(1 hunks)packages/docs/src/guide/plugin/lifecycle.md(1 hunks)packages/docs/src/guide/plugin/list.md(1 hunks)packages/docs/src/guide/plugin/testing.md(1 hunks)packages/docs/src/guide/plugin/type-system.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/docs/src/guide/plugin/type-system.md
- packages/docs/src/guide/plugin/getting-started.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/docs/src/guide/advanced/context-extensions.md
- packages/docs/src/guide/plugin/introduction.md
- packages/docs/src/guide/advanced/command-hooks.md
- packages/docs/src/guide/essentials/getting-started.md
- packages/docs/src/guide/essentials/declarative.md
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Applied to files:
packages/docs/src/guide/introduction/what-is-gunshi.md
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Applied to files:
packages/docs/src/guide/plugin/testing.md
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Mock external dependencies when needed in tests
Applied to files:
packages/docs/src/guide/plugin/testing.md
🪛 markdownlint-cli2 (0.18.1)
packages/docs/src/guide/plugin/guidelines.md
9-9: Blank line inside blockquote
(MD028, no-blanks-blockquote)
packages/docs/src/guide/advanced/custom-rendering.md
107-107: Blank line inside blockquote
(MD028, no-blanks-blockquote)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/docs/src/guide/essentials/composable.md
107-107: Blank line inside blockquote
(MD028, no-blanks-blockquote)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/docs/src/guide/essentials/plugin-system.md
192-192: Blank line inside blockquote
(MD028, no-blanks-blockquote)
packages/docs/src/guide/plugin/dependencies.md
63-63: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
236-236: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
266-266: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
packages/docs/src/guide/plugin/testing.md
9-9: Blank line inside blockquote
(MD028, no-blanks-blockquote)
12-12: Blank line inside blockquote
(MD028, no-blanks-blockquote)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test on Node.js 20
🔇 Additional comments (28)
packages/docs/src/guide/essentials/lazy-async.md (5)
197-215: Node.js flags: fix historical notes (not a rename) and scope of transform-typesThe text still implies a rename. It should state that
--experimental-transform-typeswas introduced in v22.7.0 and implies strip-types; v22.6.0 used--experimental-strip-types. Keep the production guidance unchanged.> - **Node.js with experimental support**: When running Node.js v22.6.0+ with the `--experimental-strip-types` flag (renamed to `--experimental-transform-types` in v22.7.0+) +> - **Node.js with experimental support**: On Node.js v22.6.0 use `--experimental-strip-types`. In v22.7.0+, `--experimental-transform-types` was added and implies strip-types for features that require transformation (e.g., `enum`).
218-220: Good addition: default type stripping in Node v23.6+Accurately captures that erasable TS runs without flags from v23.6.0, while transforms still need
--experimental-transform-types. LGTM.
61-68: Runtime guard for optional commandName looks goodClear check with a tight subCommands construction. Matches the earlier explanation.
284-296: Type extraction pattern is solid
NonNullable<typeof def.args>+GunshiParams<{ args: … }>provides precise ctx typing. Nice.Also applies to: 287-296
112-117: Helpful clarification aboutcommandNamevsnameAccurately explains the JS function
namereservation. Good tip.packages/docs/src/guide/plugin/dependencies.md (3)
38-58: Diagram and bullets now correctly show Auth → Cache.Mermaid edges and the bullet list are aligned with the complete example. Nice correction.
63-63: Fix MD036: don’t use emphasis as a heading.Replace the bold line with a proper subheading (or plain text).
Apply:
-**Loading order: Logger → Cache → Auth → API** +#### Loading order +Logger → Cache → Auth → API
99-101: Confirm admonition syntax renders in your docs toolchain.
> [!WARNING]is not standard Markdown. If you’re on VitePress/Markdown‑It, prefer::: warning … :::. Adjust if needed.packages/docs/src/guide/advanced/docs-gen.md (2)
295-296: Verify import/export alignment for../src/commands.Past feedback flagged a mismatch between named imports
{ mainCommand, subCommands }and actual exports. Re-check the current repo structure or adjust the example to avoid broken imports.
391-392: Good hardening: switched toexecFileSyncwith args.Safer than
execSyncstrings; resolves earlier injection/quoting concern.Also applies to: 504-509
packages/docs/src/guide/essentials/plugin-system.md (2)
144-167: Access i18n extension via exported ID constant for type safety.Import and use i18nId instead of hardcoded 'g:i18n'.
-import i18n, { defineI18n, resolveKey } from '@gunshi/plugin-i18n' +import i18n, { defineI18n, resolveKey, i18nId } from '@gunshi/plugin-i18n' ... - const locale = ctx.extensions['g:i18n'].locale + const locale = ctx.extensions[i18nId].locale ... - const t = ctx.extensions['g:i18n'].translate + const t = ctx.extensions[i18nId].translateAlso applies to: 168-177, 313-333, 334-357
168-176: Normalize LANG to BCP‑47 (e.g., en-US) and strip encoding.Prevents locale mismatches like en_US.UTF-8.
- i18n({ - locale: process.env.LANG || 'en-US' - }) + i18n({ + locale: (process.env.LANG || 'en-US').replace('_', '-').split('.')[0] + })Also applies to: 339-356
packages/docs/src/guide/plugin/decorators.md (2)
90-96: Fix comment: third decorator is innermost, not “executes first”.Align with reduceRight wrapper semantics and output below.
- // Registered third (executes first!) + // Registered third (innermost wrapper; executes after A and B)
463-469: Correct renderer chain example to show function composition, not value piping.Each decorator receives baseRenderer (a function) and returns a new renderer.
-// Actual execution flow for renderer decorators -const base = await baseRenderer(ctx) // Returns "" -const afterRenderer = await rendererDecorator(base, ctx) // Doesn't call base, returns full usage -const afterCustomA = await customADecorator(afterRenderer, ctx) // Adds "Enhanced by Plugin A" -const final = await customBDecorator(afterCustomA, ctx) // Adds "Styled by Plugin B" +// Composition model for renderer decorators +const baseRenderer = async ctx => "" // Base +const r1 = ctx => rendererDecorator(baseRenderer, ctx) // Provided by @gunshi/plugin-renderer +const r2 = ctx => customADecorator(r1, ctx) // Wraps r1 +const r3 = ctx => customBDecoraror(r2, ctx) // Wraps r2 +const final = await r3(ctx)packages/docs/src/guide/plugin/guidelines.md (6)
115-121: Use namespaced IDs consistently in examples.Avoid plain 'logger' IDs to reduce copy‑paste collisions.
-import { plugin } from 'gunshi/plugin' +import { plugin, namespacedId } from 'gunshi/plugin' ... - return plugin({ - id: 'logger', + return plugin({ + id: namespacedId('logger'),
158-159: Fix relative link to “Plugin Type System”.-For detailed type system usage, see [Plugin Type System](./type-system.md). +For detailed type system usage, see [Plugin Type System](../advanced/type-system.md).
319-326: Avoid process.exit() in signal handlers; let Node exit naturally after cleanup.This preserves other handlers/finally blocks.
- const cleanup = async () => { - await ctx.extensions.database.disconnect() - process.exit(0) - } + const cleanup = async () => { + try { await ctx.extensions.database.disconnect() } catch {} + }
456-472: Harden path validation; current checks are bypassable (Windows, symlinks).Resolve against a sandbox base and verify containment; perform case‑insensitive extension check.
-extension: () => ({ - readFile: async (path: string) => { - // Prevent path traversal - if (path.includes('..') || path.startsWith('/')) { - throw new Error('Invalid file path') - } - - // Validate file extension - const allowed = ['.json', '.yaml', '.yml'] - if (!allowed.some(ext => path.endsWith(ext))) { - throw new Error('Unsupported file type') - } - - return await fs.readFile(path, 'utf-8') - } -}) +extension: () => ({ + readFile: async (relPath: string) => { + const base = new URL('./data/', import.meta.url) + const fileUrl = new URL(relPath, base) + const basePath = fileURLToPath(base) + const targetPath = fileURLToPath(fileUrl) + if (!targetPath.startsWith(basePath)) { + throw new Error('Invalid file path') + } + const allowed = ['.json', '.yaml', '.yml'] + if (!allowed.some(ext => targetPath.toLowerCase().endsWith(ext))) { + throw new Error('Unsupported file type') + } + return await fs.readFile(targetPath, 'utf-8') + } +})
307-311: Don’t use 'exit' for async cleanup; use 'beforeExit' and await.'exit' won’t await promises.
-onExtension: ctx => { - process.once('exit', () => ctx.extensions.myPlugin.cleanup()) -} +onExtension: ctx => { + process.once('beforeExit', async () => { + try { await ctx.extensions.myPlugin.cleanup() } catch { /* noop */ } + }) +}
877-888: Pin typedoc-plugin-markdown to >=4.1.0 <4.3.0 (TypeDoc 0.26 compatible).4.1.x and 4.2.x are compatible with TypeDoc 0.26; latest patch in that sub-range is 4.2.1. Change package.json to avoid pulling 4.3+.
- "typedoc": "^0.26.0", - "typedoc-plugin-markdown": "^4.0.0" + "typedoc": "^0.26.0", + "typedoc-plugin-markdown": ">=4.1.0 <4.3.0"packages/docs/src/guide/introduction/what-is-gunshi.md (1)
47-49: Use VitePress callouts instead of GitHub-style [!TIP].Ensures proper rendering in VitePress.
-> [!TIP] -> Start with the **Setup** guide to install Gunshi in your project, then proceed through the **Essentials** section in order. Each chapter builds upon previous concepts. +::: tip +Start with the **Setup** guide to install Gunshi in your project, then proceed through the **Essentials** section in order. Each chapter builds upon previous concepts. +:::packages/docs/src/guide/plugin/extensions.md (1)
45-46: Do not change — existing plugin-local link is correct.packages/docs/src/guide/plugin/type-system.md exists, so the current Plugin Type System link in packages/docs/src/guide/plugin/extensions.md (lines 45–46) resolves correctly; do not change to ../advanced/type-system.md.
Likely an incorrect or invalid review comment.
packages/docs/src/guide/plugin/lifecycle.md (2)
13-30: Mermaid diagrams: keep build support verified.This page relies on mermaid fences; ensure VitePress markdown config includes a Mermaid plugin at build time.
Run:
#!/bin/bash rg -n "markdown-it-mermaid|vitepress-plugin-mermaid|md-enhance" packages/docs/src/.vitepress/config.* packages/docs/package.json || trueAlso applies to: 225-248
190-193: Fix decorator execution order (pre-order was inverted).Correct LIFO wrapping comment to avoid misleading readers.
-// If plugins A, B, C add decorators in that order: -// Execution order: C → B → A → original command → A → B → C +// If plugins A, B, C add decorators in that order: +// Execution order: A → B → C → original command → C → B → Apackages/docs/src/.vitepress/config.ts (4)
12-17: Add rewrites for renamed/moved pages to avoid broken inbound linksGiven the new slugs, wire top‑level rewrites so old URLs keep working (incl. “custom‑usage‑generation” → “custom‑rendering”).
export default withMermaid({ title: 'Gunshi', description: 'Modern JavaScript Command-line library', lastUpdated: true, cleanUrls: true, + rewrites: { + 'guide/advanced/custom-usage-generation': 'guide/advanced/custom-rendering', + 'guide/advanced/documentation-generation': 'guide/advanced/docs-gen', + 'guide/essentials/declarative-configuration': 'guide/essentials/declarative', + 'guide/essentials/auto-usage-generation': 'guide/essentials/auto-usage' + },
67-76: Advanced section LGTM; add redirect for removed “custom‑usage‑generation”Nav structure reads well. Keep the rewrite for the removed page to preserve backlinks.
164-165: Confirmed — mermaidPlugin.class is correctvitepress-plugin-mermaid exposes a top-level mermaidPlugin option and accepts mermaidPlugin: { class: "mermaid my-class" }, so the config is valid as-is.
55-60: Add redirects for legacy slugs or confirm hosting rewrites
- packages/docs/src/.vitepress/config.ts uses the new slugs (line 55: Declarative → /guide/essentials/declarative; line 59: Auto Usage → /guide/essentials/auto-usage).
- Repo search found no occurrences of the old slugs and no redirect/rewrites config (_redirects, netlify.toml, vercel.json, redirects.*). Add redirects mapping /guide/essentials/declarative-configuration → /guide/essentials/declarative and /guide/essentials/auto-usage-generation → /guide/essentials/auto-usage, or confirm your host will handle them.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/shared/src/types.ts (1)
6-12: Avoid runtime value imports in packages/shared/src/types.ts — use type-only typeof import()constants.ts does not import from types (no runtime cycle detected), but packages/shared/src/types.ts currently imports constants as runtime values (lines 6–12) while those identifiers are used only in type positions. Remove the value import and reference them with type-only typeof import(...) to avoid accidental runtime dependency and ESM
.tsspecifier pitfalls.Apply this diff:
-import { - ARG_PREFIX, - BUILT_IN_KEY_SEPARATOR, - BUILT_IN_PREFIX, - COMMAND_BUILTIN_RESOURCE_KEYS, - COMMON_ARGS -} from './constants.ts'Then update the type references elsewhere in this file:
-export type GenerateNamespacedKey< - Key extends string, - Prefixed extends string = typeof BUILT_IN_PREFIX -> = `${Prefixed}${typeof BUILT_IN_KEY_SEPARATOR}${Key}` +export type GenerateNamespacedKey< + Key extends string, + Prefixed extends string = typeof import('./constants')['BUILT_IN_PREFIX'] +> = `${Prefixed}${typeof import('./constants')['BUILT_IN_KEY_SEPARATOR']}${Key}` -export type CommandBuiltinArgsKeys = keyof typeof COMMON_ARGS +export type CommandBuiltinArgsKeys = keyof typeof import('./constants')['COMMON_ARGS'] -export type CommandBuiltinResourceKeys = (typeof COMMAND_BUILTIN_RESOURCE_KEYS)[number] +export type CommandBuiltinResourceKeys = + (typeof import('./constants')['COMMAND_BUILTIN_RESOURCE_KEYS'])[number] - K extends string = GenerateNamespacedKey< - Extract<KeyOfArgs<RemovedIndex<A>>, string>, - typeof ARG_PREFIX - > + K extends string = GenerateNamespacedKey< + Extract<KeyOfArgs<RemovedIndex<A>>, string>, + typeof import('./constants')['ARG_PREFIX'] + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/shared/src/types.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use ES modules throughout the codebase
Follow existing code style (enforced by ESLint and Prettier)
Files:
packages/shared/src/types.ts
🧠 Learnings (1)
📚 Learning: 2025-07-21T07:12:47.997Z
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Applied to files:
packages/shared/src/types.ts
🧬 Code graph analysis (1)
packages/shared/src/types.ts (1)
packages/shared/src/constants.ts (2)
COMMON_ARGS(33-44)COMMAND_BUILTIN_RESOURCE_KEYS(46-58)
🔇 Additional comments (2)
packages/shared/src/types.ts (2)
56-56: Const-array index access for union is idiomatic.
(typeof COMMAND_BUILTIN_RESOURCE_KEYS)[number]correctly produces the literal union. LGTM.
51-51: Approve:keyof typeof COMMON_ARGSis correct — no downstream code requires a wider key union.Quick scan shows uses only in packages/shared/src/types.ts and packages/shared/src/utils.ts (resolveBuiltInKey generic); no callers expect a broader string union.
Description
Linked Issues
Additional context
Summary by CodeRabbit