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

Stefan Beller wrote:

Signed-off-by: Ronnie Sahlberg <redacted>
Signed-off-by: Stefan Beller <redacted>
Reviewed-by: Jonathan Nieder <redacted>
---

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

[...]

[...]

+/*
+ * 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