Skip to content
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

send and receive cmd output #54

Closed
raedah opened this issue Dec 12, 2018 · 7 comments
Closed

send and receive cmd output #54

raedah opened this issue Dec 12, 2018 · 7 comments
Projects

Comments

@raedah
Copy link
Member

raedah commented Dec 12, 2018

[x@rose dcrcli]$ ./dcrcli send
Destination Address:
invalid address

Destination Address:
invalid address

Correct msg would be 'You did not specify an address.' and then just exit.

Otherwise, "That is not a valid address. Try again."
Do not have unneeded blank lines in the output when re-asking the question.

@raedah
Copy link
Member Author

raedah commented Dec 12, 2018

Similiar for amount

Amount (DCR):
error parsing amount: strconv.ParseFloat: parsing "": invalid syntax

You did not specify an amount.' and then just exit.
"That is not a valid amount. Try again."

@raedah raedah changed the title invalid address send cmd output Dec 12, 2018
@raedah
Copy link
Member Author

raedah commented Dec 12, 2018

On success the output looks like this..

Amount (DCR): 2.123
Wallet Passphrase:                                      Result | Hash
                                            |
 The transaction was published successfully | b5f844895a4e5b4e496929be0356e8435a8c30976d6bd59a29539fddd37fc9d7

better would be

Amount (DCR): 2.123
Wallet Passphrase:
You are about to send 2.123 DCR to TsgQwgjtgG7KB68pJwyUAq54uZ6GW1V73dJ
Are you sure? (y/n): y
sent txid b5f844895a4e5b4e496929be0356e8435a8c30976d6bd59a29539fddd37fc9d7

@raedah raedah changed the title send cmd output send and receive cmd output Dec 12, 2018
@raedah
Copy link
Member Author

raedah commented Dec 12, 2018

$ ./dcrcli receive
                             Address | QR Code
                                     |
 TsVhcKRQbTGwpVUX1TQJKMPDyTknKRBfkVW |

[Giant QR code]

better would be

$ ./dcrcli receive
TsVhcKRQbTGwpVUX1TQJKMPDyTknKRBfkVW
[QR code]

Lets make the qr code smaller. Smallest usable would be preferred.

@raedah raedah added this to ToDo in godcr board Dec 13, 2018
@Jujhar
Copy link
Contributor

Jujhar commented Dec 15, 2018

I am working on this.

@itswisdomagain itswisdomagain moved this from ToDo to In Progress in godcr board Dec 15, 2018
@itswisdomagain
Copy link
Contributor

@Jujhar, one good way to fix this cli output message across the cli package is to do the following:

  1. Currently each command handler has this signature
    handler func(walletrpcclient *walletrpcclient.Client, commandArgs []string) (*response, error)

  2. The *response returned from executing each command is printed to stdOut, if there's an error, the error is instead printed to stdErr

If we change the signature of the command handler to
handler func(walletrpcclient *walletrpcclient.Client, commandArgs []string) error so that each command can only return an error if there's one, and then each command prints it's own message after successful execution.

That way, each command handler can print it's success message in a way different from others and mustn't be forced to use the same pattern.

What do you think @Jujhar?

@Jujhar
Copy link
Contributor

Jujhar commented Dec 15, 2018

I was just playing around with this thinking about returning &response{} as null with that there is no need to remove all the return nils but this one seems better as returning nothing is better then always returning nil for each of the errors.

@Jujhar
Copy link
Contributor

Jujhar commented Dec 17, 2018

Also there is a bit of an error with that signature related to the code in commands.go file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants