From: Junio C Hamano Date: Thu, 27 Aug 2026 14:35:38 GMT Subject: Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options Message-ID: In-Reply-To: <20260827045515.GA176544@coredump.intra.peff.net> Jeff King writes: > It took me a minute to understand why we would even want to have an > arbitrary-sized input if we are capping at 4 anyway. The answer is that > we are capping at 4 options _that the user actually specified_. But the > input can be the total set of conflicting options, which is greater. OK. > > Really we could cap at 2 if we wanted to be technically correct, but it > might annoy the user to find each pair iteratively. > > So that makes sense. Of course the follow-on question is whether any > callers actually want to pass more than 4 options. I don't see any > patches adding new calls. There isn't. While I was writing [*], I wondered if the two calls next to each other for opt3 and opt4 want to be combined to opt7. * https://lore.kernel.org/git/xmqq1pbkefh0.fsf@gitster.g/ >> -void die_for_incompatible_opt4(const char *opt1_name, int opt1, >> - const char *opt2_name, int opt2, >> - const char *opt3_name, int opt3, >> - const char *opt4_name, int opt4) > > One nice thing about foo4() without varargs is that the compiler will > tell you if you messed it up. The obvious downside being that you have > to count in order to avoid messing it up. ;) Yes. I like that and that is why the static inlines are kept to cover the most common cases. I think I can do without [1/2], by the way. - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N pairs of - die_for_incompatible_opts() will take pairs of , expects "int" to be 0 (not set), 1 (set), or EOF==-1 (sentinel). - static inline void die_for_incompatible_opt2() emulation layer will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2, opt2_name, EOF). Similarly for opt3() and opt4() variants. > Using ARRAY_SIZE() is nice, because we could in theory bump this 4 > later. Though sadly here: > >> switch (count) { >> case 4: >> die(_("options '%s', '%s', '%s', and '%s' cannot be used together"), >> - opt1_name, opt2_name, opt3_name, opt4_name); >> + options[0], options[1], options[2], options[3]); > > we still hard-code various count values. It probably would be fine to > allocate a buffer for the message, though I guess that pushes > translators into lego-land. Very true. We could switch to dynamic allocations immediately after we see option[] filled, as we are committed to die() at that point and can afford to waste cycles. That way, for die_for_incompatible_opt10() when the end-user uses 7 of them, we can fill option[4], switch to dynamic allocation to collect all 7 of them and report. The reason I chose not to is primarily because we cannot use the existing message templates in that case, hurting i18n/l10n.