This repository was archived by the owner on Jul 15, 2018. It is now read-only.
use spf13/cobra instead of urfave/cli#117
Merged
Conversation
0cdf4ef to
8939b9f
Compare
This was referenced Oct 18, 2017
Closed
8939b9f to
ec5507b
Compare
zramsay
commented
Oct 23, 2017
| Use: "console", | ||
| Short: "Start an interactive abci console for multiple commands", | ||
| Long: "", | ||
| ValidArgs: []string{"batch", "echo", "info", "set_option", "deliver_tx", "check_tx", "commit", "query"}, |
Contributor
Author
There was a problem hiding this comment.
ec5507b to
3330cb4
Compare
ebuchman
suggested changes
Oct 26, 2017
Contributor
ebuchman
left a comment
There was a problem hiding this comment.
This is great other than the exit strategy. We should only use RunE and always return an error.
cmd/abci-cli/abci-cli.go
Outdated
| line, more, err := bufReader.ReadLine() | ||
| if more { | ||
| return errors.New("Input line is too long") | ||
| ifExit(errors.New("Input line is too long")) |
Contributor
There was a problem hiding this comment.
We should only use RunE and return errors here.
| args := persistentArgs(line) | ||
| app.Run(args) //cli prints error within its func call | ||
| pArgs := persistentArgs(line) | ||
| out, err := exec.Command(pArgs[0], pArgs[1:]...).Output() |
Contributor
There was a problem hiding this comment.
shelling it out? would be better if we could keep in process
cmd/abci-cli/abci-cli.go
Outdated
| func cmdCheckTx(cmd *cobra.Command, args []string) { | ||
| if len(args) != 1 { | ||
| return errors.New("Command check_tx takes 1 argument") | ||
| ifExit(errors.New("Command check_tx takes only 1 argument")) |
cmd/abci-cli/abci-cli.go
Outdated
| srv, err := server.NewServer(addrC, abci, app) | ||
| if err != nil { | ||
| logger.Error(err.Error()) | ||
| os.Exit(1) |
cmd/abci-cli/abci-cli.go
Outdated
| return []byte(s[1 : len(s)-1]), nil | ||
| } | ||
|
|
||
| func ifExit(err error) { |
Contributor
There was a problem hiding this comment.
remove this and all use of os.Exit in functions
807a4f3 to
f013ee5
Compare
ebuchman
approved these changes
Oct 27, 2017
odeke-em
added a commit
that referenced
this pull request
Dec 10, 2017
Use the single client connection at startup time for sending over commands instead of shelling out for every command. This code fixes the regression from #117 which instead used "os/exec".Command with: "abci-cli <the_command> [args...]" The purpose of this code is to restore us back to the state after cobra replace urlfave/cli. There is still a bit of work to implement Batch itself, but that should be simpler as a focused command. Fixes #133
odeke-em
added a commit
that referenced
this pull request
Dec 16, 2017
Use the single client connection at startup time for sending over commands instead of shelling out for every command. This code fixes the regression from #117 which instead used "os/exec".Command with: "abci-cli <the_command> [args...]" The purpose of this code is to restore us back to the state after cobra replace urlfave/cli. There is still a bit of work to implement Batch itself, but that should be simpler as a focused command. Fixes #133
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.