add --format=json style gnu args parsing

add --format=json style gnu args parsing



https://code.launchpad.net/~dave-cheney/gnuflag/001-parse-more-gnu-args/+merge/127419

(do not edit description out of merge proposal)

[dave_cheney.net, 2012-10-02 06:43:49]

Please take a look.

[dave_cheney.net, 2012-10-02 06:57:28]

Please take a look.

[rog, 2012-10-02 07:10:01]

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.

[dave_cheney.net, 2012-10-02 11:15:32]

Thank you for your comments. I have a slight worry about scope creep, we only
have to support --format=json to fix the current bug in juju.

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 {
On 2012/10/02 07:10:01, rog wrote:
> 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

yup, will fix.

> -x=true

I don't think this is valid gnuarg

> --bar=
> 
> but not
> --=x

good point, this has no name.

> i think -= should probably be an error too.

I think it is an error in the same way -x=true is an error

[rog, 2012-10-02 13:01:58]

See alternative CL at https://codereview.appspot.com/6598052/

On 2 October 2012 12:15,  <dave@cheney.net> wrote:
> Thank you for your comments. I have a slight worry about scope creep, we
> only have to support --format=json to fix the current bug in juju.
>
>
>
> 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 {
> On 2012/10/02 07:10:01, rog wrote:
>>
>> 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
>
>
> yup, will fix.
>
>> -x=true
>
>
> I don't think this is valid gnuarg

Hmm, that's a good point. I should disallow = for single-letter flags.

>> --bar=
>
>
>> but not
>> --=x
>
>
> good point, this has no name.
>
>
>> i think -= should probably be an error too.
>
>
> I think it is an error in the same way -x=true is an error

actually, -x=true is not necessarily an error with GNU flag parsing - it
specifies that -x has the argument "=true".

-=true is always an error though, as is -=.