Fixed indentation issues for long commands. #286
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideAdjusts Cobra command help formatting to align long command names and internal commands, and adds tests covering long-name layout in both grouped and ungrouped help output. Flow diagram for buildGroupCommandHelp with new alignment helpersflowchart TD
A[buildGroupCommandHelp called with cmd and group] --> B[Initialize help string and empty cmds slice]
B --> C[Collect commands in group into cmds]
C --> D[Compute padding using maxColumnNameLen cmd]
D --> E[Sort cmds by name]
E --> F[Build subGroupNameList from command annotations]
F --> G[Append group.Title to help]
G --> H[Determine intend string using hasSubgroup cmd]
H --> I{For each subgroupName in subGroupNameList}
I -->|Yes| J[If multiple subgroups append subgroup header to help]
J --> K[For each command in cmds with matching SubGroupName]
K --> L[Append formatted line using rpad command name and padding]
I -->|No more subgroups| M[For each command in cmds without SubGroupName]
M --> N[Append formatted line using rpad command name and padding plus intend]
N --> O[Return help string]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
hasSubgrouphelper duplicates work already done when buildingsubGroupNameList; you can drop the extra traversal and simply derive the indent fromlen(subGroupNameList)to avoid additional allocations and iteration. - The
intendvariable appears to be a misspelling ofindent; renaming it will make the indentation logic easier to follow and less confusing. - If command names may contain non-ASCII characters,
maxColumnNameLenshould useutf8.RuneCountInStringinstead oflento keep the help columns visually aligned with multibyte characters.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `hasSubgroup` helper duplicates work already done when building `subGroupNameList`; you can drop the extra traversal and simply derive the indent from `len(subGroupNameList)` to avoid additional allocations and iteration.
- The `intend` variable appears to be a misspelling of `indent`; renaming it will make the indentation logic easier to follow and less confusing.
- If command names may contain non-ASCII characters, `maxColumnNameLen` should use `utf8.RuneCountInString` instead of `len` to keep the help columns visually aligned with multibyte characters.
## Individual Comments
### Comment 1
<location> `cmd/root.go:104` </location>
<code_context>
}
}
+ padding := maxColumnNameLen(cmd)
+
sort.Slice(cmds, func(i, j int) bool {
</code_context>
<issue_to_address>
**suggestion:** Column width calculation may misalign multi-byte (non-ASCII) command names
`maxColumnNameLen` and `rpad` rely on `len(s)`, which measures bytes, not display cells. For non-ASCII command names (e.g., emoji, CJK), this will miscompute padding and misalign columns. If such names are supported, use `utf8.RuneCountInString` or a display-width–aware library instead of byte length.
Suggested implementation:
```golang
import (
"fmt"
"unicode/utf8"
"github.com/spf13/cobra"
)
```
```golang
func maxColumnNameLen(cmd *cobra.Command) int {
max := 0
for _, c := range cmd.Commands() {
nameLen := utf8.RuneCountInString(c.Name())
if nameLen > max {
max = nameLen
}
}
return max
}
```
```golang
func rpad(s string, padding int) string {
// padding is the desired rune width of the column
runeLen := utf8.RuneCountInString(s)
if runeLen >= padding {
return s
}
// number of spaces to add to reach the target rune width
spaceCount := padding - runeLen
return s + fmt.Sprintf("%*s", spaceCount, "")
}
```
If `maxColumnNameLen` or `rpad` are defined with slightly different signatures or logic in your actual file, you should adapt the SEARCH blocks to match the exact existing implementations (e.g., different variable names or comments). The core changes are:
1. Replace all `len(<string>)` calls used for column-width computation with `utf8.RuneCountInString(<string>)`.
2. Ensure any padding calculations that depend on string length use rune counts instead of byte counts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
cmd/root.go
Outdated
| } | ||
| } | ||
|
|
||
| padding := maxColumnNameLen(cmd) |
There was a problem hiding this comment.
suggestion: Column width calculation may misalign multi-byte (non-ASCII) command names
maxColumnNameLen and rpad rely on len(s), which measures bytes, not display cells. For non-ASCII command names (e.g., emoji, CJK), this will miscompute padding and misalign columns. If such names are supported, use utf8.RuneCountInString or a display-width–aware library instead of byte length.
Suggested implementation:
import (
"fmt"
"unicode/utf8"
"github.com/spf13/cobra"
)func maxColumnNameLen(cmd *cobra.Command) int {
max := 0
for _, c := range cmd.Commands() {
nameLen := utf8.RuneCountInString(c.Name())
if nameLen > max {
max = nameLen
}
}
return max
}func rpad(s string, padding int) string {
// padding is the desired rune width of the column
runeLen := utf8.RuneCountInString(s)
if runeLen >= padding {
return s
}
// number of spaces to add to reach the target rune width
spaceCount := padding - runeLen
return s + fmt.Sprintf("%*s", spaceCount, "")
}If maxColumnNameLen or rpad are defined with slightly different signatures or logic in your actual file, you should adapt the SEARCH blocks to match the exact existing implementations (e.g., different variable names or comments). The core changes are:
- Replace all
len(<string>)calls used for column-width computation withutf8.RuneCountInString(<string>). - Ensure any padding calculations that depend on string length use rune counts instead of byte counts.
Arfey
commented
Jan 9, 2026
Arfey
commented
Jan 9, 2026
| return fmt.Sprintf(formattedString, s) | ||
| } | ||
|
|
||
| func hasSubgroup(cmd *cobra.Command) bool { |
Contributor
Author
There was a problem hiding this comment.
Call it for each group. I didn’t want to call it before buildGroupCommandHelp and pass it as a parameter. Since we only have two groups, this is probably fine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Improve CLI help output formatting to correctly align command names and descriptions, including for very long command names and grouped commands.
Bug Fixes:
Enhancements:
Tests: