Subject: Re: [PATCH v7 2/3] maintenance: `git maintenance run` learned `--scheduler=<scheduler>`

From: Junio C Hamano <[email-masked]>
Date: Tue, 06 Jul 2021 13:52:12 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Fri, Jul 02 2021, Lénaïc Huard wrote:
>
>> + *     ┏━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
>> + *     ┃ Input ┃                     Output                      ┃
>> + *     ┃ *cmd  ┃ return code │       *cmd        │ *is_available ┃
>> + *     ┣━━━━━━━╋━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━━━━━┿━━━━━━━━━━━━━━━┫
>> + *     ┃ "foo" ┃    false    │ "foo" (unchanged) │  (unchanged)  ┃
>> + *     ┗━━━━━━━┻━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━━━━━┷━━━━━━━━━━━━━━━┛
>
> I wonder if we have developers for whom the non-ASCII here is an issue.

I do have an issue myself ;-) but I can survive.  I do not know
about others.

> This sort of code is much more pleseant to read and work with if you use
> strbuf_split_buf(). This isn't performance sensitive, so a few more
> allocations is fine.

Please do not encourage use of strbuf_split_buf().  It is a
misdesigned API as it rarely is justifyable to have an array, each
element of which can be independently tweaked by being strbuf.  We
are not implementing a text editor after all ;-)

A helper function that takes a string and returns a strvec would be
a good fit, though.

>> +#ifdef __APPLE__
>> +	return 1;
>> +#else
>> +	return 0;
>> +#endif
>> +}
>
> I see this is partially a pre-existing thing in the file, but we have an
> __APPLE__ already in cache.h. Perhaps define a iLAUNCHCTL_AVAILABLE
> there. See e.g. 62e5ee81a39 (read-cache.c: remove #ifdef NO_PTHREADS,
> 2018-11-03).

Excellent suggestion.