Skip to content
This repository was archived by the owner on Jul 15, 2018. It is now read-only.

fix console output#119

Merged
zramsay merged 2 commits intocobra-not-urfavefrom
93-fix-console-output
Oct 26, 2017
Merged

fix console output#119
zramsay merged 2 commits intocobra-not-urfavefrom
93-fix-console-output

Conversation

@zramsay
Copy link
Contributor

@zramsay zramsay commented Oct 23, 2017

func cmdConsole(cmd *cobra.Command, args []string) {

for {
fmt.Printf("\n> ")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the extra line is not necessary IMO and makes for a less nice experience because we end up with double line breaks

if !rsp.Code.IsOK() {
fmt.Printf("-> code: %s\n", rsp.Code.String())
}
// Always print the status code.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how the docs had it. it's a good verification that things worked

if cmd.Use != "commit" {
fmt.Printf("-> data: %s\n", rsp.Data)
}
fmt.Printf("-> data.hex: 0x%X\n", rsp.Data)
Copy link
Contributor Author

@zramsay zramsay Oct 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append 0x because that's how the docs are. maybe it got removed for a reason but IMO with is nicer

-> data.hex: 0x7B2273697A65223A317D

> commit
-> data: I��\ͬޮ�(����h����
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhmm, ya this was the test.

Copy link
Contributor

@ebuchman ebuchman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@zramsay zramsay merged commit b5bdb6e into cobra-not-urfave Oct 26, 2017
@zramsay zramsay deleted the 93-fix-console-output branch October 26, 2017 11:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants