Re: [PATCH v2 3/8] string-list: return index directly when inserting an existing element

[shejialuo <shejialuo@gmail.com>]

On Mon, May 19, 2025 at 03:58:13AM -0400, Jeff King wrote:
> On Sun, May 18, 2025 at 11:57:15PM +0800, shejialuo wrote:
> 
> > When inserting an existing element, "add_entry" would convert "index"
> > value to "-1-index" to indicate the caller that this element is in the
> > list already.
> > 
> > However, in "string_list_insert", we would simply convert this to the
> > original positive index without any further action. Let's directly
> > return the index as we don't care about whether the element is in the
> > list by using "add_entry".
> > 
> > In the future, if we want to let "add_entry" tell the caller, we may add
> > "int *exact_match" parameter to "add_entry" instead of converting the
> > index to negative to indicate.
> 
> I assumed this was in the same boat as the previous change: something we
> used to use and now don't. But I don't think we ever did. The "-1-index"
> pattern goes all the way back to the beginning of the code.
> 
> It does match how other functions like string_list_find_insert_index()
> behave. But I think that pattern doesn't make much sense for
> add_entry(). After the function returns we know we've either found
> something or added it, so the positive index will always point to a
> matching entry.
> 
> So I think your patches are correct, but I was curious how we got to
> this state.

It seems that we create this in a long time ago. In 8fd2cb4069 (Extract
helper bits from c-merge-recursive work, 2006-07-25), we introduce the
"path-list.c", at that time, we have the code already.

> 
> -Peff

Thanks,
Jialuo