Re: [PATCH v2 1/8] pack-objects: use standard option incompatibility functions

From: Taylor Blau <me@ttaylorr.com>
Date: Apr 15, 2025, 22:27 UTC
Message-ID: <Z/7dVa+RnEqayEMf@nand.local>
In-Reply-To: <xmqqo6wxkxy2.fsf@gitster.g>

On Tue, Apr 15, 2025 at 12:48:53PM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> Taylor Blau <me@ttaylorr.com> 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