cmd/geth: fix passing datadir flag to attach subcommand #15517
Merged
Conversation
This file contains 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
tsarpaul
reviewed
Nov 18, 2017
cmd/geth/consolecmd.go
Outdated
| @@ -48,13 +48,17 @@ See https://github.com/ethereum/go-ethereum/wiki/Javascipt-Console.`, | |||
| Name: "attach", | |||
| Usage: "Start an interactive JavaScript environment (connect to node)", | |||
| ArgsUsage: "[endpoint]", | |||
| Flags: append(consoleFlags, utils.DataDirFlag), | |||
| Flags: append(consoleFlags), | |||
There was a problem hiding this comment.
You could remove the append here.
MaximilianMeister
force-pushed
the
usage-ipcpath
branch
from
f8a0714 to
af75892
Compare
|
Perhaps we should fix the subcommand to accept |
MaximilianMeister
force-pushed
the
usage-ipcpath
branch
from
af75892 to
7f998c1
Compare
@karalabe is this the right way to fix it? i just assumed that the ipc endpoint will always be called |
MaximilianMeister
changed the title
lion22
reviewed
Nov 23, 2017
| @@ -112,7 +113,11 @@ func localConsole(ctx *cli.Context) error { | |||
| // console to it. | |||
| func remoteConsole(ctx *cli.Context) error { | |||
| // Attach to a remotely running geth instance and start the JavaScript console | |||
| client, err := dialRPC(ctx.Args().First()) | |||
There was a problem hiding this comment.
конечная точка : = ctx. Args (). Первый ()
- если ctx. GlobalIsSet (utils. DataDirFlag . Name ) {
- endpoint = fmt. Sprintf ( " % s /geth.ipc " , ctx. GlobalString (utils. DataDirFlag . Name ))
+} - клиент , err : = dialRPC (конечная точка)
fjl
requested changes
Nov 27, 2017
cmd/geth/consolecmd.go
Outdated
| @@ -112,7 +113,11 @@ func localConsole(ctx *cli.Context) error { | |||
| // console to it. | |||
| func remoteConsole(ctx *cli.Context) error { | |||
| // Attach to a remotely running geth instance and start the JavaScript console | |||
| client, err := dialRPC(ctx.Args().First()) | |||
| endpoint := ctx.Args().First() | |||
| if ctx.GlobalIsSet(utils.DataDirFlag.Name) { | |||
There was a problem hiding this comment.
This is fine, but please use --datadir only if the first argument is not set.
There was a problem hiding this comment.
@fjl done, thanks
MaximilianMeister
force-pushed
the
usage-ipcpath
branch
from
7f998c1 to
348dc79
Compare
fjl
requested changes
Nov 27, 2017
cmd/geth/consolecmd.go
Outdated
| @@ -112,7 +113,11 @@ func localConsole(ctx *cli.Context) error { | |||
| // console to it. | |||
| func remoteConsole(ctx *cli.Context) error { | |||
| // Attach to a remotely running geth instance and start the JavaScript console | |||
| client, err := dialRPC(ctx.Args().First()) | |||
| endpoint := ctx.Args().First() | |||
| if (endpoint == "") && (ctx.GlobalIsSet(utils.DataDirFlag.Name)) { | |||
There was a problem hiding this comment.
Please remove unnecessary parens here. You can write endpoint == "" && ctx.GlobalIsSet(utils.DataDirFlag.Name).
There was a problem hiding this comment.
@fjl sure, fixed
the datadir flag wasn't respected, instead you had to provide an additional argument, which was unclear from a usage point of view Signed-off-by: Maximilian Meister <[email protected]>
MaximilianMeister
force-pushed
the
usage-ipcpath
branch
from
348dc79 to
afab235
Compare
fjl
approved these changes
Nov 28, 2017
This was referenced Dec 1, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
the datadir flag wasn't respected, instead you had to provide an
additional argument, which was unclear from a usage point of view
Signed-off-by: Maximilian Meister [email protected]
I tried the following on a private network: