Re: [PATCH v6] refs.c: use a string_list for repack_without_refs

[Jonathan Nieder <jrnieder@gmail.com>]

Stefan Beller wrote:

> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> ---

Yep, looks good now.  Thanks for bearing with me.

[...]
> +++ b/refs.h
> @@ -163,8 +163,16 @@ extern void rollback_packed_refs(void);
[...]
> +/*
> + * Remove the refs listed in the unsorted string list 'without' from
> + * the packed-refs file. On error, packed-refs will be unchanged, the
> + * return value is nonzero, and a message about the error is written
> + * to the 'err' strbuf.
> + *
> + * The refs in 'without' may be unsorted.
> + * 'err' must not be NULL.

I think we've gone back and forth enough on this text and it's not
worth the transactional cost to tweak it further, so I'm not
suggesting a change --- just explaining how I read it for future
reference.

"may be unsorted" is confusing to me.  It sounds like the reader of
this comment (someone calling repack_without_refs) has to be prepared
for that possibility.  But we are saying the opposite --- not "be
prepared", but "don't worry about sorting 'without', since
repack_without_refs can handle it".

It's also redundant, since the paragraph above already says that
'without' is an unsorted string list.

The way I see it, there are four types that for various reasons (lack
of language-level support for subclassing, etc) are conflated into a
single struct in the string-list API:

 * sorted string list that owns its items (i.e., created with DUP)
 * sorted string list that does not own its items (i.e., created with NODUP)
 * unsorted string list that owns its items
 * unsorted string list that does not own its items

Different functions are valid to call on each type, as documented in
the comments in string-list.h.

repack_without_refs accepts all 4 types of string-list.  That's what
it means when the documentation says its argument is unsorted.

Thanks,
Jonathan