Re: [PATCH 3/3] string_list: Remove string_list_insert_at_index from its API

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

Stefan Beller <sbeller@google.com> writes:

> This function behaves the same as string_list_insert, just the
> starting indexes for searching, where to insert into the list is also
> a parameter. So if you have knowledge on where to search for the string
> to be inserted, you may have a speed up version of string_list_insert.

The last half-sentence I am having trouble parsing.  If you have
that knowledge where the item should go, you can insert it at the
right location faster than calling string_list_insert()?

That sounds like a useful feature to me.

But if nobody uses that feature, there is no point keeping it, no
matter how useful it may sound.  So,... I am not sure.

The function would be error prone if used on a string-list that is
accessed via string_list_insert() API, which makes it only useful if
the user of the API is in full control of the sort order, so I do
not mind removing it and won't be saying "that is useful, we should
keep it and use it more."

But your log message confuses me....

>
> As we're not using this function throughout the codebase, get rid of it.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  string-list.c | 8 +-------
>  string-list.h | 2 --
>  2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/string-list.c b/string-list.c
> index c5aa076..9584fa6 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -59,13 +59,7 @@ static int add_entry(int insert_at, struct string_list *list, const char *string
>  
>  struct string_list_item *string_list_insert(struct string_list *list, const char *string)
>  {
> -	return string_list_insert_at_index(list, -1, string);
> -}
> -
> -struct string_list_item *string_list_insert_at_index(struct string_list *list,
> -						     int insert_at, const char *string)
> -{
> -	int index = add_entry(insert_at, list, string);
> +	int index = add_entry(-1, list, string);
>  
>  	if (index < 0)
>  		index = -1 - index;
> diff --git a/string-list.h b/string-list.h
> index 40ffe0c..ee9b100 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -61,8 +61,6 @@ int string_list_find_insert_index(const struct string_list *list, const char *st
>   * Returns the string_list_item, the string is part of.
>   */
>  struct string_list_item *string_list_insert(struct string_list *list, const char *string);
> -struct string_list_item *string_list_insert_at_index(struct string_list *list,
> -						     int insert_at, const char *string);
>  
>  /*
>   * Checks if the given string is part of a sorted list. If it is part of the list,