-
Notifications
You must be signed in to change notification settings - Fork 812
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
CLI: add history event version and full detail options #817
CLI: add history event version and full detail options #817
Conversation
tools/cli/commands.go
Outdated
FlagPrintEventVersion = "print_event_version" | ||
FlagPrintEventVersionWithAlias = FlagPrintEventVersion + ", pev" | ||
FlagPrintFullyDetail = "print_full" | ||
FlagPrintFullyDetailWithAlias = FlagPrintFullyDetail + ", full" |
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.
Maybe change alias "full" to "pf" to align with all other Print flags?
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.
Yeah good idea.
tools/cli/commands.go
Outdated
@@ -383,13 +390,35 @@ func showHistoryHelper(c *cli.Context, wid, rid string) { | |||
table.SetBorder(false) | |||
table.SetColumnSeparator("") | |||
for _, e := range history.Events { | |||
columns := []string{} | |||
if printFully { | |||
jsonBs, err := json.Marshal(e) |
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.
Let's also recursively decode the internal byte[] like "HistoryEventToString" did
tools/cli/commands.go
Outdated
jsonBs, err := json.Marshal(e) | ||
var jsonStr string | ||
if err != nil { | ||
jsonStr = "ERROR!" |
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.
Would be better to add a little bit more info here like "Error when marshal event" so that customers won't be confused whether it is server error or CLI error.
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.
Yeah, make sense
d856ace
to
3a31aa3
Compare
|
3a31aa3
to
019ffcf
Compare
Verified in my laptop: