Re: [PATCH v2 2/5] pack-objects: refactor `read_packs_list_from_stdin()` to use `strmap`

[Taylor Blau <me@ttaylorr.com>]

On Thu, Mar 26, 2026 at 03:11:06PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Thu, Mar 26, 2026 at 04:40:00PM -0400, Derrick Stolee wrote:
> >> On 3/25/2026 7:51 PM, Taylor Blau wrote:
> >>
> >> > -static void read_packs_list_from_stdin(struct rev_info *revs)
> >> > +struct stdin_pack_info {
> >> > +	struct packed_git *p;
> >> > +	enum {
> >> > +		STDIN_PACK_INCLUDE = (1<<0),
> >> > +		STDIN_PACK_EXCLUDE_CLOSED = (1<<1),
> >> > +	} kind;
> >> > +};
> >>
> >> I kind of wish this enum wasn't anonymous. And it matters later.
> >> Let's call this 'enum pack_input_kind' for now.
> >
> > Hmm. I don't feel strongly about this, but I'm not sure I follow the
> > reasoning here. The enum is truly only meant to be used within the
> > context of a stdin_pack_info struct, so it felt natural to keep it
> > anonymous above.
>
> The only thing that makes it beneficial to have a name is if a
> code like this one ...
>
> >> 	const char *key = buf.buf;
> >> 	enum pack_input_kind kind = STDIN_PACK_INCLUDE;
> >>
> >> 	if (*key == '^') {
> >> 		key++;
> >> 		kind |= STDIN_PACK_EXCLUDE_CLOSED;
> >> 	}
>
> ... that uses the type to define its own variable outside the
> context of the struct needs to be written, right?
>
> If these STDIN_PACK_* constants would ever appear _only_ within the
> context of talking about the .kind member of the stdin_pack_info
> struct and cannot possibly appear anywhere else, then there is no
> point naming the enum.

Yup, I agree. I'm inclined to leave the enum anonymous for now, since
the only place we would need a name for it is the suggestion Stolee made
above, which I think does not correctly handle an edge case where packs
are specified multiple times.

As it is currently, the enum is only used in the context of the .kind
member of the stdin_pack_info struct, and we don't ever declare a
int/unsigned variable to hold the kind outside of that context (which
would be gross ;-)).

Thanks,
Taylor