Re: [PATCH] am: don't pass strvec to apply_parse_options()

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 20, 2022, 01:29 UTC
Message-ID: <xmqqa63izwuv.fsf@gitster.g>
In-Reply-To: <91c5120d-bbd4-8aa5-a205-d1f5387a7f19@web.de>

René Scharfe <l.s.r@web.de> writes:

> Depends on the specifics.  Perhaps .bar is redundant and could be
> inferred from .foo.  Or they could be replaced by an enum.
> ...
> Or normalize the struct to avoid dependencies between fields.

I think the example Peff brought up was mostly from the revisions API, where there are two vastly different traversal behaviour (i.e. limited and !limited). With .limited bit set, we first fully enumerate the potential commits, before showing a single entry in the output, and without, we can stream the output. In general, we prefer !limited traversal because that would give us better interactive latency, but some options only can work with the limited bit set (e.g. --ancestry-path, --cherry).

We _could_ probably have do without the .limited bit and instead wrote each and every check we currently make to the revs->limited bit as something like

#define is_limited(revs) (revs->cherry_mark || \
			  revs->ancestry_path || \
			  ...)

so that the "struct members" API users do not have to know that they have to set the .limit bit when they set .cherry_mark bit. There are many others even in the revisions API alone.

IOW, yes, this depends on the specifics, and "normalize" is easier said than done, unfortunately.