chore: help formatting improvements #165
Conversation
|
Warning Rate limit exceeded@MihalyToth20 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughInline help formatting was extracted from Changes
Sequence Diagram(s)sequenceDiagram
participant Program
participant Utils as main-program-utils
participant FormatterRoot as customFormatHelpForRoot
participant FormatterCmd as customFormatHelp
participant Output as CLI
Program->>Utils: applyCustomHelpToAllCommands(program)
activate Utils
Utils->>Utils: iterate commands & subcommands
alt command is root
Utils->>FormatterRoot: assign root formatHelp
activate FormatterRoot
FormatterRoot-->>Utils: configured root formatter
deactivate FormatterRoot
else non-root
Utils->>FormatterCmd: assign per-command formatHelp
activate FormatterCmd
FormatterCmd-->>Utils: configured command formatter
deactivate FormatterCmd
end
Utils-->>Program: all formatters applied
deactivate Utils
Program->>Output: render styled help
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/cli/src/utils/commands/main-program-utils.ts (2)
22-45: Consider using the top-levelisDeprecatedfunction.Line 23 redefines
isDeprecatedlocally, shadowing the top-level function defined on lines 3-6. For consistency and to avoid confusion, consider using the top-level function instead.Apply this diff to use the top-level function:
function collectVisibleLeafCommands(cmd, parentPath = []) { - const isDeprecated = (c) => c.description && c.description().trim().startsWith('[DEPRECATED]'); const path = [...parentPath, cmd.name()].filter((segment) => segment !== 'xano'); let results = [];
91-91: Consider accepting thehelperparameter for consistency.While
customFormatHelpForRootdoesn't currently use the helper, Commander.js'sformatHelpsignature is(cmd, helper) => string. For consistency withcustomFormatHelp(line 47) and to follow the framework convention, consider accepting the helper parameter even if unused.Apply this diff:
-function customFormatHelpForRoot(cmd) { +function customFormatHelpForRoot(cmd, helper) { // 1. Collect all visible leaf commands with their full paths
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/modern-carpets-lose.md(1 hunks)packages/cli/src/program.ts(3 hunks)packages/cli/src/utils/commands/main-program-utils.ts(2 hunks)packages/cli/src/utils/methods/font.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cli/src/utils/commands/main-program-utils.ts (1)
packages/cli/src/utils/methods/font.ts (1)
font(36-36)
🪛 LanguageTool
.changeset/modern-carpets-lose.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ...- chore: help command styling for easy scimming
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (7)
packages/cli/src/utils/methods/font.ts (2)
10-10: LGTM!The green color variant is correctly implemented using the standard ANSI escape code.
17-17: LGTM!The boldGreen combo and its integration into the font object follow the existing pattern correctly.
Also applies to: 25-25, 32-32
packages/cli/src/program.ts (2)
16-19: LGTM!The updated imports correctly include the new
applyCustomHelpToAllCommandsutility for the refactored help formatting.
101-101: LGTM!Replacing the inline help formatter configuration with the extracted
applyCustomHelpToAllCommandsutility improves code organization and reusability.packages/cli/src/utils/commands/main-program-utils.ts (3)
47-89: LGTM!The custom help formatter for non-root commands is well-structured with clear sections (description, usage, arguments, options, subcommands, footer) and appropriate styling using the font utilities.
187-197: LGTM!The recursive application of custom help formatters is correctly implemented, distinguishing between root commands (using
customFormatHelpForRoot) and subcommands (usingcustomFormatHelp).
199-199: LGTM!The updated exports correctly expose the new public API
applyCustomHelpToAllCommandswhile keepingcollectVisibleLeafCommandsas an internal helper.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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/cli/src/utils/commands/main-program-utils.ts (1)
3-6: Remove unused function.The outer
isDeprecatedfunction is never used becausecollectVisibleLeafCommandsdefines its own version at line 23. This is dead code that should be removed to reduce confusion.Apply this diff:
-function isDeprecated(cmd) { - const desc = cmd.description ? cmd.description() : ''; - return desc.trim().startsWith('[DEPRECATED]'); -} - function getFullCommandPath(cmd) {
♻️ Duplicate comments (1)
packages/cli/src/utils/commands/main-program-utils.ts (1)
101-140: Previous duplicate "Other:" group concern has been resolved.The static "Other:" group mentioned in the previous review has been removed. The current implementation only uses the dynamic "Other:" group (lines 135-140) to collect ungrouped commands, which correctly prevents duplication.
🧹 Nitpick comments (1)
packages/cli/src/utils/commands/main-program-utils.ts (1)
152-155: Consider dynamically listing options.The options are hardcoded rather than using
helper.visibleOptions(cmd)to dynamically list registered options. While-v, --versionand-h, --helpare standard, using the helper would be more maintainable if options change.If you want to use the helper dynamically:
- output.push(font.weight.bold('Options:')); - output.push( - ` -v, --version ${font.color.gray('output the version number')}\n` + - ` -h, --help ${font.color.gray('display help for command')}\n` - ); + output.push(font.weight.bold('Options:')); + const optionsList = helper.visibleOptions(cmd); + for (const opt of optionsList) { + output.push(` ${font.color.cyan(opt.flags)}` + `\n ${opt.description || ''}\n`); + }Note: This function signature currently doesn't receive
helperas a parameter, so you'd need to update the function signature tocustomFormatHelpForRoot(cmd, helper)and the caller at line 185.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/utils/commands/main-program-utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cli/src/utils/commands/main-program-utils.ts (1)
packages/cli/src/utils/methods/font.ts (1)
font(36-36)
🔇 Additional comments (4)
packages/cli/src/utils/commands/main-program-utils.ts (4)
1-2: LGTM!The font import is necessary for the new custom help formatting functions and is correctly structured.
22-45: Note: Function shadowing will be resolved.This function redefines
isDeprecatedat line 23, which shadows the unused outer function at line 3. Once the outer function is removed (as suggested in the previous comment), this shadowing issue will be resolved.
47-89: LGTM!The custom help formatter for non-root commands is well-structured with clear sections (Banner, Usage, Arguments, Options, Subcommands, Footer) and consistent styling using the font utilities.
195-195: LGTM!The export change correctly exposes the new public API
applyCustomHelpToAllCommandswhile keepinggetFullCommandPath. The previouscollectVisibleLeafCommandsis now internal-only, which aligns with the refactoring objectives.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
Chores
New Features