Re: [PATCH v3 01/19] convert: make convert_attrs() and convert structs public

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 30, 2020, 17:38 UTC
Message-ID: <xmqqd00z397m.fsf@gitster.c.googlers.com>
In-Reply-To: <CAHd-oW44tFRnpDWsQfF80gZn42mvgwZ0rT-ocQZ92zBTrQL-ZA@mail.gmail.com>

Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes:

Show 28 quoted lines

>> > +enum crlf_action {
>> > +     CRLF_UNDEFINED,
>> > +     CRLF_BINARY,
>> > +     CRLF_TEXT,
>> > +     CRLF_TEXT_INPUT,
>> > +     CRLF_TEXT_CRLF,
>> > +     CRLF_AUTO,
>> > +     CRLF_AUTO_INPUT,
>> > +     CRLF_AUTO_CRLF
>> > +};
>> > +
>> > +struct convert_driver;
>> > +
>> > +struct conv_attrs {
>> > +     struct convert_driver *drv;
>> ...
>> > +void convert_attrs(const struct index_state *istate,
>> > +                struct conv_attrs *ca, const char *path);
>> > +
>> >  /*
>> >   * Initialize the checkout metadata with the given values.  Any argument may be
>> >   * NULL if it is not applicable.  The treeish should be a commit if that is
>>
>> The new global symbols are reasonable, I would think, with a
>> possible exception of "crlf_action", which may want to also have
>> "conv" or "convert" somewhere in its name.
>
> OK. Maybe `enum crlf_conv_action`? In this case, should I also change

Either that, or "conv_crlf_action" (or even use the fully spelled "convert_" as the prefix common to the global symbols from the subsystem).

> In this case, should I also change
> the prefix of the enum values? I'm not sure if it's worth it, though,
> since there are about 52 occurrences of them.

At the use sites, these constants will be passed to, or compared with values returned by, the API functions whose names make it clear that they are from the "convert_" family, so I think it is OK to leave the values as-is, as long as there is no unrelated symbol whose name starts with "CRLF_" (and "git grep '^CRLF_'" tells me that there is not any).

Thanks.