[PATCH 01/12] Fixes to parse_options

[Pekka Paalanen]

On Tue, 19 Aug 2014 12:12:28 -0700
Bill Spitzak <spitzak at gmail.com> wrote:

> 
> On 08/19/2014 02:18 AM, Pekka Paalanen wrote:
> 
> >> Multiple single-letter booleans in one switch allowed, ie
> >> -xyz is the same as -x -y -z. For wayland modules they all have
> >> to belong to the same module.
> >
> > This was not accepted before, right? It is confusing that one needs to
> > know that they are from the same module, so I'd rather just not accept
> > combined single letter booleans at all.
> 
> It may have been suggested by somebody else? And then rejected? I'll 
> remove this, it's not a big deal.
> 
> >> Previous version could use text after the null at the end of
> >> an argv entry. Now requires the = sign.
> >
> > What does this mean? I mean, how could one have ever used the old
> > behaviour? Nothing comes to my mind, so I suppose that is ok.
> 
> The old code basically did *(string+strlen(string)+1) if the string did 
> not have an = sign in it, passing that pointer to the argument parser.
> Probably this always fails parsing without a segfault but technically it 
> is wrong, it could segfault, or it could get some text at that point 
> that actually parses, producing an unexpected value.

Aah, maybe that was hack to try and use the argv[i+1] as the value if
argv[i] was the option?

Looks like 'weston --width 500' won't work at the moment anyway, and
I'm not sure if it ever was meant to, so I think requiring the '=' is
ok.

> > There are some style issues. First is the patch subject, it should be
> > prefixed with the component the patch is touching. To get a feeling
> > what the components are, look at the git-log of the file your are
> > changing, and there should be a pattern.
> 
> I see, there is a "name:" at the start of the description line. I missed 
> that, I was only adding the git repository if it was not weston to 
> [PATCH repository]. Also looks like things like V2 should be between the 
> square brackets.
> 
> > Though, I see that option-parser.c doesn't really have any pattern. Oh
> > well, "shared:" should do then.
> 
> Or the demo programs, or startup.
> >
> > More style issues below.
> 
> Will try to address these.

Thanks,
pq