Re: [PATCH 0/11] renaming argv_array

[Jacob Keller <jacob.keller@gmail.com>]

On Tue, Jul 28, 2020 at 1:25 PM Jeff King <peff@peff.net> wrote:
>
> The argv_array data type has turned out to be useful in our code base,
> but the name isn't very good. From patch 2 of this series:
>
>   The name "argv-array" isn't very good, because it describes what the
>   data type can be used for (program argument arrays), not what it
>   actually is (a dynamically-growing string array that maintains a
>   NULL-terminator invariant). This leads to people being hesitant to use
>   it for other cases where it would actually be a good fit. The existing
>   name is also clunky to use. It's overly long, and the name often leads
>   to saying things like "argv.argv" (i.e., the field names overlap with
>   variable names, since they're describing the use, not the type). Let's
>   give it a more neutral name.
>
> This has bugged me for a while, so I decided to finally fix it. It
> wasn't _too_ painful, though I'm sure there will be a little fallout
> with topics in flight.
>
> I tried to split out the mechanical bits into their own patches to make
> reviewing easier. Patches 5-7 really could be a single patch, but
> they're too big for the mailing list. I'm OK to leave them separate, or
> they could be squashed together.
>
> We could stop at patch 9 for now and allow topics in flight to catch up
> before removing the compat layers. But the struct field renaming has to
> happen as a single step, so it will be a pain whenever we do it. If
> we're going to go this route, I'd just as soon do it all now and deal
> with other topics as they get merged.
>
>   [01/11]: argv-array: use size_t for count and alloc
>   [02/11]: argv-array: rename to strvec
>   [03/11]: strvec: rename files from argv-array to strvec
>   [04/11]: quote: rename sq_dequote_to_argv_array to mention strvec
>   [05/11]: strvec: convert builtin/ callers away from argv_array name
>   [06/11]: strvec: convert more callers away from argv_array name
>   [07/11]: strvec: convert remaining callers away from argv_array name
>   [08/11]: strvec: fix indentation in renamed calls
>   [09/11]: strvec: update documention to avoid argv_array
>   [10/11]: strvec: drop argv_array compatibility layer
>   [11/11]: strvec: rename struct fields
>

I like this. It definitely helps to name the API after what it does.

One thing I thought I would see but I guess we simply don't have one
is a technical doc that details the strvec. I guess we just never had
one for argv_array? Probably worth adding one at some point.

Thanks,
Jake