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

From: Junio C Hamano <[email-masked]>
Date: Tue, 20 Dec 2022 10:29:12 +0900

René Scharfe <[email-masked]> 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.