Re: [PATCH] document string_list_clear

[Jonathan Nieder <jrnieder@gmail.com>]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>>> +/*
>>> + * Clears the string list, so it has zero items. All former items are freed.
>>> + * If free_util is true, all util pointers are also freed.
>>> + */
>>>  void string_list_clear(struct string_list *list, int free_util);
>>
>> The api doc says
>>
>> 	Free a string_list. The `string` pointer of the items will be freed in
>> 	case the `strdup_strings` member of the string_list is set. The second
>> 	parameter controls if the `util` pointer of the items should be freed
>> 	or not.
>>
>> One option here would be to say
>>
>> 	Free a string_list.  See Documentation/technical/api-string-list.txt
>> 	for details.
>
> If we later introduce string_list_free() that is in line with the
> distinction between "free" and "clear" discussed on this thread, the
> comment for this function needs to be fixed to "Clear the string
> list.  See $doc". and that is not much different from "See $doc"
> without the first sentence which is the function name.

I still find the term "clear" to be confusing here.  It makes me think
the function will be analagous to strbuf_reset, when it's actually
analagous to strbuf_release.

In other words, the important thing is that this frees all members of
the string_list.  There might be a clearer way to say that.  The
string_list itself may not be dynamically allocated --- I'm not sure
what it would mean to free it.

How about string_list_release?

I think I could get used to "clear" if we used it consistently though.
I suspect anything we do will be confusing, unless we make them
consistent.

> Perhaps the API doc that currently says "Free" is the only thing
> that needs fixing?

I find the API doc to be pretty clear, actually.  If someone was
confused in practice then I'd be happy to try to debug the wording.

>                     And perhaps add "See $doc" at the beginning of
> the header and remove duplicated comments we already have in the
> file?

Yes, that sounds like a good way to go about it.

Thanks,
Jonathan