Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
