On Tue, Apr 15, 2025 at 12:48:53PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@xxxxxxxxxxxx> writes:
>
> > Now I can't un-see it ;-). Even though it's not a correctness issue as
> > you note, the whole thing leaves a bad taste in my mouth. I'll swap the
> > ordering to match the original in the next round.
>
> I do not think we can be completely faithful to the original in this
> rewrite, simply because the original is not consistent with what
> die_for_incompat() thing produces and you'd need to adjust the test
> anyway. So unless there are other things you need to reroll, I
> wouldn't worry about it too much.
Yeah, we need to adjust the test either way. I just disliked reading the
patch and seeing:
if (stdin_packs && filter_options.choice)
die(_("--stdin-packs and --filter can't be used together"));
turn into
die_for_incompatible_opt2(filter_options.choice, "--filter",
stdin_packs, "--stdin-packs");
since the check is "stdin_packs then filter_options.choice" in the
original, but "filter_options.choice then stdin_packs" in this patch.
Funny enough, the test that breaks expects output that mentions
"--filter" before "--stdin-packs" here, so preserving the order of the
check in the code reverses the order in which the incompatible arguments
appear in the die() message.
> Thanks.
Thanks,
Taylor
