add output json format to keys add command #29
Conversation
WalkthroughThe changes in the pull request enhance the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
cmd/opinitd/key.go (3)
44-49: LGTM: New types for JSON output structure.The new types
keyJsonOutputandkeyJsonOutputElemare well-structured for the JSON output feature. The use of a map forkeyJsonOutputallows for potential future expansion to multiple keys.Consider adding a comment to explain the purpose of these types, especially if they might be used in other parts of the codebase in the future.
151-169: LGTM: Implementation of output format handling.The new logic for handling different output formats is well-implemented. It correctly uses a switch statement to differentiate between JSON and plain text output, and properly marshals the data for JSON output.
Consider adding an
else ifclause for the "plain" format explicitly, rather than using it as the default case. This would make the code more robust against potential future additions of new output formats.switch outputFormat { case "json": // ... (existing JSON handling code) +case "plain": + output = fmt.Sprintf("%s: %s\n%s", account.Name, addrString, mnemonic) default: - output = fmt.Sprintf("%s: %s\n%s", account.Name, addrString, mnemonic) + return fmt.Errorf("unsupported output format: %s", outputFormat) }
Line range hint
108-120: LGTM: Implementation of mnemonic recovery from file.The new functionality to read a mnemonic from a file enhances the key recovery feature. The implementation is correct, with proper error handling and whitespace trimming.
Consider adding a check for the file size before reading its entire content to prevent potential issues with very large files:
if mnemonicSrc != "" { file, err := os.Open(mnemonicSrc) if err != nil { return err } + fileInfo, err := file.Stat() + if err != nil { + file.Close() + return err + } + if fileInfo.Size() > 1024 { // 1KB limit, adjust as needed + file.Close() + return fmt.Errorf("mnemonic file too large: %d bytes", fileInfo.Size()) + } bz, err := io.ReadAll(file) file.Close() if err != nil { return err } mnemonic = strings.TrimSpace(string(bz)) } else { // ... (existing code for manual input) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- cmd/opinitd/key.go (4 hunks)
🔇 Additional comments (4)
cmd/opinitd/key.go (4)
21-21: LGTM: New import and constant for JSON output.The addition of the
encoding/jsonimport and theflagOutputconstant are appropriate for implementing the new JSON output feature.Also applies to: 41-41
81-82: LGTM: Updated command examples.The new examples for the
keys addcommand clearly demonstrate the usage of the--recoverflag with a source file and the new--output jsonoption. These additions improve the command's documentation and usability.
176-176: LGTM: New flag for output format.The addition of the
--outputflag is correct and consistent with the new functionality. The default value "plain" maintains backwards compatibility, and the description clearly indicates the available options.
Line range hint
1-420: Overall assessment: Well-implemented feature additions.The changes to this file successfully implement JSON output capability for the
keys addcommand and enhance the mnemonic recovery feature. The new code is well-integrated, maintains existing error handling patterns, and follows the established coding style. These additions improve the flexibility and usability of the key management commands without disrupting existing functionality.To ensure the changes don't introduce any regressions, please run the following verification script:
This script tests the basic functionality of the
keys addcommand with both default and JSON output, as well as the new mnemonic recovery from file feature.
Summary by CodeRabbit
New Features
--outputflag for thekeys addcommand, allowing users to choose between "plain" or "json" output formats.Bug Fixes