Re: [PATCH v4 2/5] promisor-remote: allow a server to advertise more fields

[Christian Couder <christian.couder@gmail.com>]

On Mon, Jun 23, 2025 at 10:04 PM Justin Tobler <jltobler@gmail.com> wrote:
>
> On 25/06/11 03:45PM, Christian Couder wrote:

> > +static char *fields_from_config(struct string_list *fields_list, const char *config_key)
> > +{
> > +     char *fields = NULL;
> > +
> > +     if (!git_config_get_string(config_key, &fields) && *fields) {
> > +             string_list_split_in_place(fields_list, fields, ", ", -1);
> > +             string_list_remove_empty_items(fields_list, 0);
>
> Ok, in this version we now filter out empty entries from the
> string_list. Previously if fields were specified with both a comma and
> SP character (i.e. "partialCloneFilter, token"), an empty entry would be
> parsed in the middle and lead to a warning message.
>
> This change is good because it would be pretty natural for a user to
> specify the config with both. It might be nice to leave a comment
> explaining why we do this though as it may be confusing without context.

Yeah, I have added some comments, so it looks like this in v5:

        /* Split on any comma or space character */
        string_list_split_in_place(fields_list, fields, ", ", -1);
        /*
         * Remove empty items that might result from trailing
         * commas, or from items being separated by both
         * commas and spaces.
         */
        string_list_remove_empty_items(fields_list, 0);

Thanks.