Code review comment for lp:~dave-cheney/gnuflag/001-parse-more-gnu-args

Revision history for this message
Roger Peppe (rogpeppe) wrote :

thanks for working on this - it needed doing!

a few thoughts below.

https://codereview.appspot.com/6588060/diff/1/flag.go
File flag.go (right):

https://codereview.appspot.com/6588060/diff/1/flag.go#newcode729
flag.go:729: if len(v) > 1 && len(v[1]) > 0 {
this seems a bit odd to me. i think it's perfectly ok to have an empty
*value*, but an empty flag is wrong.

i think we need to return an error here if there's an empty flag. if we
store the entire string following the flag (including the leading '=')
inside f.procFlag, then parseGnuFlagArg can know that it's got that
style of flag, and process accordingly.

that way we allow the following cases:
--foo=false
-x=true
--bar=

but not
--=x

i think -= should probably be an error too.

https://codereview.appspot.com/6588060/diff/1/flag_test.go
File flag_test.go (right):

https://codereview.appspot.com/6588060/diff/1/flag_test.go#newcode127
flag_test.go:127: "--format=json",
more test cases here please.

https://codereview.appspot.com/6588060/

« Back to merge proposal