Re: [PATCH 3/5] parseopt: use boolean type for a simple flag

>
> On 03/11/2025 05:19, Junio C Hamano wrote:
> > "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> >
> >>              is_optional = skip_prefix(value, ":(optional)", &value);
> >>              if (!value)
> >> -                    is_optional = 0;
> >> +                    is_optional = false;
> >
> > Whether it is spelled 0 or false, I do not think this makes any
> > sense.  skip_prefix() either touches &value to point at the
> > substring in value that comes after ":(optional)", or it does not
> > touch it at all, so there is no way value can be NULL here (and we
> > know value is not NULL before we call skip_prefix()).
> >
> > Shouldn't you be removing the entire "if value is NULL, it is not
> > optional" thing instead?  That is exactly what Phillip pointed out
> > in his review.
>
> Looking at this again I wonder if the intention was to error out if
> there wasn't a filename after the ":(optional)" prefix which I think
> would be a reasonable thing to do but that's not what this code actually
> does.
>
> Thanks
>
> Phillip