Repository navigation
Conversation
StringMapArgs is the only exported named-argument type without a
Command.{Type}Arg accessor, while Args() documents that named arguments
are read through those accessors. Add Command.StringMapArgs, a test, and
the regenerated godoc snapshots.
This branch has not been deployed
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 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.
What type of PR is this?
What this PR does / why we need it:
StringMapArgsis the only exported named-argument type inargs.gowithout a matchingCommand.{Type}Argaccessor.args.godeclares:while every other argument type can be read back through
cmd.FloatArg,cmd.IntArg,cmd.StringArg, ... There is nocmd.StringMapArgs.Command.Valueonly looks at flags, and the doc comment onCommand.Args()says values consumed by named arguments "should be retrieved via the command.{Type}Arg(name) functions instead". Without this accessor the parsed value of aStringMapArgsargument can only be reached by iteratingcmd.Argumentsand type-assertingGet(), or throughDestination— the documented retrieval path does not exist for this one type.This adds the accessor the same way
StringArg/StringArgsare built (arg[map[string]string](name, c)), returning nil for an unknown name. Parsing behaviour is unchanged.Which issue(s) this PR fixes:
No issue; reported from source read. This is an API-completion fix for the accessor promised by the
Args()doc comment, not a bug fix for a reported failure.Special notes for your reviewer:
StringMapArgs. The alias is singular (ArgumentBase) but carries theArgssuffix, so the accessor follows the type name for discoverability; happy to rename toStringMapArgif you prefer.godoc-current.txtandtestdata/godoc-v3.x.txtwere regenerated withmake generate+make v3approve;make v3diffexits 0.Testing:
go test ./...andgo vet ./...pass, including the newTestArgsStringMapType. It runs a command with&cli.StringMapArgs{Name: "ma", Destination: &mval}over"a=1,b=2"and checks theDestination,cmd.StringMapArgs("ma"), and the nil result for an unknown name.args.gohunk makes the test fail at compile time withcmd.StringMapArgs undefined (type *Command has no field or method StringMapArgs), which is the gap this PR fills.Release Notes