-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"ipfs key list": include the hash of the key id in addition to the name #3581
Conversation
License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>
@whyrusleeping not sure about this at all, feedback welcome, fell free to just take over this pr if it will be faster. |
return | ||
} | ||
|
||
list = append(list, key+" "+pid.Pretty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an array of strings here will make the json output of the api hard to parse. Lets make the output be an array of KeyOutput
structs (which probably means defining a KeyOutputList
struct because our use of the type system in the commands lib is weird).
Lets add a -l
or --show-ids
flag to the command and then in the output marshaler, then check that for whether or not to print the keys alongside the names.
Then in the text output marshaler, use a tabwriter to make sure the columns are aligned evenly when outputting both names and keys.
CHANGELOG: |
@whyrusleeping do you think it would be better for the hash to come first. It would make it easier to parse by shell utilities if the key name has spaces in it and would also be more consistent with "ipfs ls". I don't have a strong preference either way. |
License: MIT Signed-off-by: Kevin Atkinson <k@kevina.org>
a4aadfd
to
4dbb084
Compare
Done. I listed the hash first when the "-l" option is used, but can easily swap them. |
@@ -135,6 +142,9 @@ var KeyListCmd = &cmds.Command{ | |||
Helptext: cmds.HelpText{ | |||
Tagline: "List all local keypairs", | |||
}, | |||
Options: []cmds.Option{ | |||
cmds.BoolOption("show-ids", "l", "also show key ids"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say just l
, an have it catch all long format output, I think we might have more in future.
94262d5
to
a3b38e8
Compare
License: MIT Signed-off-by: Jeromy <why@ipfs.io>
@whyrusleeping your changes look good to me. Merge when ready. Let me know if there is anything else you need from me on this. |
First stab, closes #3568