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

[Junio C Hamano <gitster@pobox.com>]

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.

A free-standing variable could use "int" or "unsigned" as the base
language C does not differentiate different enums as separate types,
but let's not go there, as -Wenum-compare and other warnings do give
us opportunity to do better than that.