feat: export fctl commands as json #69
Conversation
|
""" WalkthroughA new Go file introduces a command-line tool that exports the structure and metadata of a Cobra-based CLI application to a JSON file. The tool defines data structures for commands and flags, recursively traverses the command hierarchy, collects relevant metadata, and writes the information to "inventory.json". Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Dav-14
force-pushed
the
feat/export_fctl_command_as_json_
branch
from
531459e to
20c8403
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
tools/commandusage/export.go (3)
13-15: Consider making the output filename configurable.The hardcoded filename limits flexibility. Consider making this configurable via a command-line flag or environment variable for better usability.
-var ( - file = "inventory.json" -) +var ( + outputFile = flag.String("output", "inventory.json", "Output JSON file") +)
17-24: Review field redundancy in Flag struct.Both
DescriptionandUsagefields are populated with the same value (f.Usage) in theflagSetToFlagsfunction (lines 48, 51). Consider if both fields are necessary or if they should serve different purposes.If they serve the same purpose, consider removing one:
type Flag struct { Name string `json:"name"` Description string `json:"description"` Deprecated string `json:"deprecated,omitempty"` - Usage string `json:"usage"` DefaultValue string `json:"default_value,omitempty"` Type string `json:"type,omitempty"` }
26-37: Unused SubCommands field in DocCommand struct.The
SubCommandsfield is defined but never populated in the code. The current implementation writes each command as a separate JSON object rather than building a nested structure. Consider removing this field if not needed, or document why it's reserved for future use.If not needed, remove the unused field:
type DocCommand struct { Name string `json:"name"` Usage string `json:"usage"` Description string `json:"description"` Deprecated string `json:"deprecated,omitempty"` Aliases []string `json:"aliases,omitempty"` Flags []Flag `json:"flags,omitempty"` GlobalFlags []Flag `json:"global_flags,omitempty"` - - SubCommands []DocCommand `json:"subcommands,omitempty"` }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tools/commandusage/inventory.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
tools/commandusage/export.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tools/commandusage/export.go (1)
cmd/root.go (1)
NewRootCommand(39-92)
🪛 GitHub Actions: Default
tools/commandusage/export.go
[error] 120-120: gosec: Expect WriteFile permissions to be 0600 or less (G306) - os.WriteFile called with 0644 permissions
⏰ 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: Cursor Bugbot
🔇 Additional comments (2)
tools/commandusage/export.go (2)
1-11: LGTM!The package declaration and imports are clean and appropriate for this command-line tool.
39-57: LGTM!The function correctly converts pflag.FlagSet to Flag structs, properly handles hidden flags, and extracts all relevant metadata.
| //go:generate rm -rf docs | ||
| //go:generate go run ./ | ||
| func main() { | ||
| _, err := os.Stat(file) | ||
| if os.IsNotExist(err) { | ||
| if err := os.WriteFile(file, []byte("[]"), 0644); err != nil { | ||
| panic(err) | ||
| } | ||
| } else { | ||
| os.Remove(file) | ||
| } | ||
|
|
||
| file, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| defer file.Close() | ||
|
|
||
| visitSubCommands(cmd.NewRootCommand(), file) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address variable shadowing and JSON format issues.
Multiple issues in the main function:
- Variable name shadowing between global
filestring and localfile*os.File - The JSON output will be invalid (starts with "[]" then appends raw JSON objects)
- File handling logic is unnecessarily complex
Consider this approach:
func main() {
- _, err := os.Stat(file)
- if os.IsNotExist(err) {
- if err := os.WriteFile(file, []byte("[]"), 0600); err != nil {
- panic(err)
- }
- } else {
- os.Remove(file)
- }
-
- file, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
- if err != nil {
+ outputFile, err := os.OpenFile(file, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
+ if err != nil {
panic(err)
}
- defer file.Close()
+ defer outputFile.Close()
- visitSubCommands(cmd.NewRootCommand(), file)
+ // Write opening bracket for JSON array
+ outputFile.WriteString("[\n")
+ visitSubCommands(cmd.NewRootCommand(), outputFile)
+ // Write closing bracket
+ outputFile.WriteString("\n]")
}You'll also need to modify writeDocs to handle proper JSON array formatting with commas.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Default
[error] 120-120: gosec: Expect WriteFile permissions to be 0600 or less (G306) - os.WriteFile called with 0644 permissions
🤖 Prompt for AI Agents
In tools/commandusage/export.go around lines 115 to 134, fix the variable
shadowing by renaming the local file handle variable to avoid clashing with the
global file string. Refactor the file handling logic to open the file once in
write mode, truncating it initially instead of writing "[]" and then appending.
Modify the JSON writing logic in writeDocs to correctly format the output as a
valid JSON array by writing the opening bracket once, then writing each JSON
object separated by commas, and finally writing the closing bracket, ensuring
the entire output is a well-formed JSON array.
20c8403 to
3043815
Compare
Dav-14
force-pushed
the
feat/export_fctl_command_as_json_
branch
from
3043815 to
3570ba6
Compare
| } | ||
|
|
||
| //go:generate rm -rf docs | ||
| //go:generate go run ./ |
There was a problem hiding this comment.
why run the binary on generate?
| } | ||
|
|
||
| commands := visitSubCommands(cmd.NewRootCommand()) | ||
| file, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600) |
There was a problem hiding this comment.
Instead of remove the file with os.Remove then opening a new fil with os.OpenFile, you can use os.Create which truncates the file if it already exists.
No description provided.