Centralize command-line parsing part 2 #2723
Conversation
command/base/command.go
Outdated
| // Change 'value' to 'string' for consistency | ||
| if example == "value" { | ||
| example = "string" | ||
| } |
There was a problem hiding this comment.
This feels a little strange; having common flags show up as -http-addr=<value> looks weird, though
There was a problem hiding this comment.
Hmm - we should probably keep the default Go behavior here for consistency. Looking at UnquoteUsage you can fix -http-addr by putting string in backquotes in the help text:
The `address` to connect to...
This will make it do -http-addr=<address> and is probably a better way to customize. While in here I realized that the description for -http-addr doesn't address UNIX sockets at all, so we need to touch that up a bit.
command/base/command.go
Outdated
| @@ -130,6 +140,9 @@ func (c *Command) Parse(args []string) error { | |||
|
|
|||
| // Help returns the help for this flagSet. | |||
| func (c *Command) Help() string { | |||
| if c.flagSet == nil { | |||
| return "" | |||
| } | |||
There was a problem hiding this comment.
I wasn't sure if there was a better solution than this for cases where Help() gets called before anything is initialized, like with the kv command and the kv tests that check for no tabs.
There was a problem hiding this comment.
Yeah this seems ok - I'd add a comment about why this is here.
api/api.go
Outdated
| @@ -437,6 +450,9 @@ func (r *request) setWriteOptions(q *WriteOptions) { | |||
| if q.Token != "" { | |||
| r.header.Set("X-Consul-Token", q.Token) | |||
| } | |||
| if q.RelayFactor != 0 { | |||
| r.header.Set("relay-factor", strconv.Itoa(int(q.RelayFactor))) | |||
There was a problem hiding this comment.
Needs to be a param set, not header.
command/base/command.go
Outdated
| @@ -130,6 +140,9 @@ func (c *Command) Parse(args []string) error { | |||
|
|
|||
| // Help returns the help for this flagSet. | |||
| func (c *Command) Help() string { | |||
| if c.flagSet == nil { | |||
| return "" | |||
| } | |||
There was a problem hiding this comment.
Yeah this seems ok - I'd add a comment about why this is here.
command/base/command.go
Outdated
| // Change 'value' to 'string' for consistency | ||
| if example == "value" { | ||
| example = "string" | ||
| } |
There was a problem hiding this comment.
Hmm - we should probably keep the default Go behavior here for consistency. Looking at UnquoteUsage you can fix -http-addr by putting string in backquotes in the help text:
The `address` to connect to...
This will make it do -http-addr=<address> and is probably a better way to customize. While in here I realized that the description for -http-addr doesn't address UNIX sockets at all, so we need to touch that up a bit.
command/event.go
Outdated
| f.StringVar(&node, "node", "", | ||
| "Regular expression to filter on node names.") | ||
| f.StringVar(&service, "service", "", | ||
| "Regular expression to filter on service instances") |
There was a problem hiding this comment.
Missing period at the end of this one. We keep an eye out for this and try to make them all match (include the period).
| @@ -101,7 +92,7 @@ func (c *EventCommand) Run(args []string) int { | |||
| } | |||
|
|
|||
| // Create and test the HTTP client | |||
| client, err := HTTPClient(*httpAddr) | |||
There was a problem hiding this comment.
Note to self to make sure we delete this HTTPClient later.
command/keyring.go
Outdated
| f.BoolVar(&listKeys, "list", false, | ||
| "List all keys currently in use within the cluster.") | ||
| f.IntVar(&relay, "relay-factor", 0, | ||
| "Added in Consul 0.7.4, setting this to a non-zero value will cause nodes "+ |
There was a problem hiding this comment.
I wonder if it makes sense to put these version notes in the actual command help, or just the web docs. If you are seeing this, then you have the right version :-)
There was a problem hiding this comment.
Yeah, it's pretty redundant here, going to remove it.
command/kv_get.go
Outdated
| "This is especially useful if you only need the key names themselves. "+ | ||
| "This option is commonly combined with the -separator option. The default "+ | ||
| "value is false.") | ||
| base64encode := f.Bool("base64", false, |
There was a problem hiding this comment.
This should be moved up to keep them in sorted order.
command/kv_get.go
Outdated
| recurse := f.Bool("recurse", false, | ||
| "Recursively look at all keys prefixed with the given path. The default "+ | ||
| "value is false.") | ||
| separator := f.String("separator", "/", |
There was a problem hiding this comment.
This ought to get a -stale option while we are in here.
There was a problem hiding this comment.
Stale is one of the common http options, are you referring to something else?
There was a problem hiding this comment.
Oh then we need to plumb it into the options below.
There was a problem hiding this comment.
Or does the API client plumb that in?
There was a problem hiding this comment.
It doesn't; it gets passed in through the QueryOptions like this:
AllowStale: c.Command.HTTPStale(),
website/source/layouts/docs.erb
Outdated
| @@ -87,6 +87,10 @@ | |||
| <a href="/docs/commands/force-leave.html">force-leave</a> | |||
| </li> | |||
|
|
|||
| <li<%= sidebar_current("docs-commands-info") %>> | |||
| <a href="/docs/commands/info.html">info</a> | |||
There was a problem hiding this comment.
Looks like the whitespace got borked here.
This converts the commands up through
kvto use the base.Command structure for parsing.Fixes #2566