Hi,
On Sat, Jan 30, 2021 at 5:37 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "Hariom Verma via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > } else if (!match_placeholder_bool_arg(*arg, "only", arg, &opts->only_trailers) &&
> > !match_placeholder_bool_arg(*arg, "unfold", arg, &opts->unfold) &&
> > !match_placeholder_bool_arg(*arg, "keyonly", arg, &opts->key_only) &&
> > - !match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only))
> > - break;
> > + !match_placeholder_bool_arg(*arg, "valueonly", arg, &opts->value_only)) {
> > + size_t invalid_arg_len = strcspn(*arg, ",)");
> > + *invalid_arg = xstrndup(*arg, invalid_arg_len);
> > + return 1;
> > + }
> > }
> > return 0;
> > }
>
> Would the existing caller be OK with this change?
Yes, I made sure of that.
> It used to be that this parsing code simply _ignored_ unrecognised
> trailer keyword because the very original just did a "break" and
> fell though, but now because this returns non-zero, it causes the
> caller rewritten in the patch [v2 1/3] to "goto trailer_out".
>
> It is not clear from your proposed log message if this would result
> in behaviour change, and if so if that behaviour change was intended.
>
> I suspect that the behaviour change the code implements may be OK,
> but the log message needs to discuss why it is OK.
I should have included that change in behaviour in the commit message.
Will change that too.
Thanks,
Hariom Verma
