Re: [PATCH] document string_list_clear

[Jonathan Nieder <jrnieder@gmail.com>]

Jonathan Nieder wrote:
> Stefan Beller wrote:
>> On Tue, Dec 9, 2014 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:

>>> Perhaps the API doc that currently says "Free" is the only thing
>>> that needs fixing?  And perhaps add "See $doc" at the beginning of
>>> the header and remove duplicated comments we already have in the
>>> file?
>>
>> The reason I wrote this patch originally was because I seem to forget we have
>> more than one place to document our APIs. If there are comments in the header
>> I seem to have thought it were the only place where we have documentation.
>
> How about this patch?

And another:

-- >8--
Subject: put string-list API documentation in one place

Until recently (v1.8.0-rc0~46^2~5, 2012-09-12), the string-list API
was documented in detail in Documentation/technical/api-string-list.txt
and the header file contained section markers and some short reminders
but little other documentation.

Since then, the header has acquired some more comments that are mostly
identical to the documentation from technical/.  In principle that
should help convenience, since it means one less hop for someone
reading the header to find API documentation.  In practice,
unfortunately, it is hard to remember that there is documentation in
two places, and the comprehensive documentation of some functions in
the header makes it too easy to forget that the other functions are
documented at all (and where).

Add a comment pointing to Documentation/technical/ and remove the
comments that duplicate what is written there.  Longer term, we may
want to move all of the technical docs to header files and generate
printer-ready API documentation another way, but that is a larger
change for another day.

Short reminders in the header file are still okay.

Reported-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/technical/api-string-list.txt | 12 ++++++-
 string-list.h                               | 56 ++---------------------------
 2 files changed, 14 insertions(+), 54 deletions(-)

diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt
index d51a657..a85e713 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -185,7 +185,17 @@ overwriting the delimiter characters with NULs and creating new
 string_list_items that point into the original string (the original
 string must therefore not be modified or freed while the `string_list`
 is in use).
-
++
+Examples:
++
+----
+string_list_split(l, "foo:bar:baz", ':', -1) -> ["foo", "bar", "baz"]
+string_list_split(l, "foo:bar:baz", ':', 0) -> ["foo:bar:baz"]
+string_list_split(l, "foo:bar:baz", ':', 1) -> ["foo", "bar:baz"]
+string_list_split(l, "foo:bar:", ':', -1) -> ["foo", "bar", ""]
+string_list_split(l, "", ':', -1) -> [""]
+string_list_split(l, ":", ':', -1) -> ["", ""]
+----
 
 Data structures
 ---------------
diff --git a/string-list.h b/string-list.h
index 494eb5d..e6a7841 100644
--- a/string-list.h
+++ b/string-list.h
@@ -1,6 +1,8 @@
 #ifndef STRING_LIST_H
 #define STRING_LIST_H
 
+/* See Documentation/technical/api-string-list.txt */
+
 struct string_list_item {
 	char *string;
 	void *util;
@@ -35,20 +37,9 @@ int for_each_string_list(struct string_list *list,
 #define for_each_string_list_item(item,list) \
 	for (item = (list)->items; item < (list)->items + (list)->nr; ++item)
 
-/*
- * Apply want to each item in list, retaining only the ones for which
- * the function returns true.  If free_util is true, call free() on
- * the util members of any items that have to be deleted.  Preserve
- * the order of the items that are retained.
- */
 void filter_string_list(struct string_list *list, int free_util,
 			string_list_each_func_t want, void *cb_data);
 
-/*
- * Remove any empty strings from the list.  If free_util is true, call
- * free() on the util members of any items that have to be deleted.
- * Preserve the order of the items that are retained.
- */
 void string_list_remove_empty_items(struct string_list *list, int free_util);
 
 /* Use these functions only on sorted lists: */
@@ -59,30 +50,12 @@ struct string_list_item *string_list_insert(struct string_list *list, const char
 struct string_list_item *string_list_insert_at_index(struct string_list *list,
 						     int insert_at, const char *string);
 struct string_list_item *string_list_lookup(struct string_list *list, const char *string);
-
-/*
- * Remove all but the first of consecutive entries with the same
- * string value.  If free_util is true, call free() on the util
- * members of any items that have to be deleted.
- */
 void string_list_remove_duplicates(struct string_list *sorted_list, int free_util);
 
 
 /* Use these functions only on unsorted lists: */
 
-/*
- * Add string to the end of list.  If list->strdup_string is set, then
- * string is copied; otherwise the new string_list_entry refers to the
- * input string.
- */
 struct string_list_item *string_list_append(struct string_list *list, const char *string);
-
-/*
- * Like string_list_append(), except string is never copied.  When
- * list->strdup_strings is set, this function can be used to hand
- * ownership of a malloc()ed string to list without making an extra
- * copy.
- */
 struct string_list_item *string_list_append_nodup(struct string_list *list, char *string);
 
 void sort_string_list(struct string_list *list);
@@ -92,32 +65,9 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list,
 
 void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util);
 
-/*
- * Split string into substrings on character delim and append the
- * substrings to list.  The input string is not modified.
- * 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.
- *
- * Examples:
- *   string_list_split(l, "foo:bar:baz", ':', -1) -> ["foo", "bar", "baz"]
- *   string_list_split(l, "foo:bar:baz", ':', 0) -> ["foo:bar:baz"]
- *   string_list_split(l, "foo:bar:baz", ':', 1) -> ["foo", "bar:baz"]
- *   string_list_split(l, "foo:bar:", ':', -1) -> ["foo", "bar", ""]
- *   string_list_split(l, "", ':', -1) -> [""]
- *   string_list_split(l, ":", ':', -1) -> ["", ""]
- */
 int string_list_split(struct string_list *list, const char *string,
 		      int delim, int maxsplit);
-
-/*
- * Like string_list_split(), except that string is split in-place: the
- * delimiter characters in string are overwritten with NULs, and the
- * new string_list_items point into string (which therefore must not
- * be modified or freed while the string_list is in use).
- * list->strdup_strings must *not* be set.
- */
 int string_list_split_in_place(struct string_list *list, char *string,
 			       int delim, int maxsplit);
+
 #endif /* STRING_LIST_H */
-- 
2.2.0.rc0.207.ga3a616c