On Tue, Jul 28, 2020 at 1:25 PM Jeff King <peff@xxxxxxxx> 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
