Re: [PATCHv8 1/5] string list: improve comment

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  string-list.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/string-list.h b/string-list.h
> index d3809a1..465a1f0 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -106,7 +106,7 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_
>   * list->strdup_strings must be set, as new memory needs to be
>   * allocated to hold the substrings.  If maxsplit is non-negative,
>   * then split at most maxsplit times.  Return the number of substrings
> - * appended to list.
> + * appended to list. The list may be non-empty already.

I personally find that the original comment is clear enough, though.

When somebody says "resulting elements of the split are appended to
the list" without saying either:

  a. "the list MUST be empty in the beginning", or
  b. "the list will be emptied first before the split result are appended",

wouldn't it be natural to take it as "you can append them to any
list, be it empty or not, and they are _appended_, not replaced"?

So while this is not incorrect per-se, I am not sure if it adds much
value.  If somebody needs this additional clarification, because the
original did not say a. above, she would certainly need more
clarification because the original did not say b. above, either.

"The list may be non-empty already", but she would keep wondering if
the existing contents would be discarded before the result gets
appended to it.

You may say "No, there won't be such a confusion, because we say
'append'; empty and then append is 'replace'".  But then the same
logic would say "There cannot be a requirement for the list to be
empty in the first place, because we say 'append'".

So...