[PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[Jeff King]

This is a re-roll of this series:

  http://thread.gmane.org/gmane.comp.version-control.git/260922/focus=261374

from early December to move the strbuf documentation into the header
file. I've incorporated all of the minor formatting feedback and
suggestions from Jonathan and Stefan in response to that series.

Patch 2 is new from Stefan, and just fixes the comment blocks that
already existed in strbuf.h to be consistent.

Patches 3-5 are the same as the old 2-4 (modulo a header I missed in the
original of 5).

Both Junio and Jonathan noted that tighter grouping of related functions
can be more readable in some places. Patches 6 and 7 adjust those two
spots. This moves us farther from a single comment describing each
function, which may make extracting the documentation later harder. But
I think it makes reading the in-header documentation much more pleasant.

  [1/7]: strbuf.h: integrate api-strbuf.txt documentation
  [2/7]: strbuf.h: unify documentation comments beginnings
  [3/7]: strbuf.h: drop asciidoc list formatting from API docs
  [4/7]: strbuf.h: format asciidoc code blocks as 4-space indent
  [5/7]: strbuf.h: reorganize api function grouping headers
  [6/7]: strbuf.h: drop boilerplate descriptions of strbuf_split_*
  [7/7]: strbuf.h: group documentation for trim functions

The two things I _didn't_ do are:

  1. Read through for possible places to use `backticks` or other
     punctuation to make the source more readable. I think this is a
     good goal, but it's largely orthogonal, so we can build it on top
     just as easily (and also, it's tedious; I'm happy to work towards
     that goal slowly as I touch or read specific pieces of
     documentation).

  2. We do not consistently use parameter names in declarations. They
     have no meaning to the compiler, of course, but they are a good way
     of making the documentation more readable, since it gives us a
     concrete name to refer to (and also some declarations are simply
     confusing, such as when there are two ints next to each other;
     which is which?).  This is in the same boat as (1). I think it's a
     good direction, but I did not make a pass.  Patches welcome. :)

And of course the elephant in the room is the other dozen or more
api-*.txt files. I'd propose to do this strbuf.h series (and possible
follow-ons mentioned above) and stop there for a bit. That will let us
form a more coherent opinion on whether we like this system in practice,
how it ages as functions are changed and added, etc. That might affect
how or if we end up converting other files.

It does leave us in an inconsistent state (some documentation is in
Documentation/technical, and some is in the headers), but I think that
is largely where we're at today. IMHO this is a strict improvement
because at least the logical chunk of "strbuf" is now in a single place.

-Peff

[PATCH 1/7] strbuf.h: integrate api-strbuf.txt documentation

[Jeff King]

Some of strbuf is documented as comments above functions,
and some is separate in Documentation/technical/api-strbuf.txt.
This makes it annoying to find the appropriate documentation.
We'd rather have it all in one place, which means all in the
text document, or all in the header.

Let's choose the header as that place. Even though the
formatting is not quite as pretty, this keeps the
documentation close to the related code.  The hope is that
this makes it easier to find what you want (human-readable
comments are right next to the C declarations), and easier
for writers to keep the documentation up to date.

This is more or less a straight import of the text from
api-strbuf.txt into C comments, complete with asciidoc
formatting. The exceptions are:

 1. All comments created in this way are started with "/**"
    to indicate they are part of the API documentation. This
    may help later with extracting the text to pretty-print
    it.

 2. Function descriptions do not repeat the function name,
    as it is available in the context directly below.  So:

      `strbuf_add`::

          Add data of given length to the buffer.

    from api-strbuf.txt becomes:

      /**
       * Add data of given length to the buffer.
       */
      void strbuf_add(struct strbuf *sb, const void *, size_t);

    As a result, any block-continuation required in asciidoc
    for that list item was dropped in favor of straight
    blank-line paragraph (since it is not necessary when we
    are not in a list item).

 3. There is minor re-wording to integrate existing comments
    and api-strbuf text. In each case, I took whichever
    version was more descriptive, and eliminated any
    redundancies. In one case, for strbuf_addstr, the api
    documentation gave its inline definition; I eliminated
    this as redundant with the actual definition, which can
    be seen directly below the comment.

 4. The functions in the header file are re-ordered to match
    the ordering of the API documentation, under the
    assumption that more thought went into the grouping
    there.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/technical/api-strbuf.txt | 351 -------------------------
 strbuf.h                               | 457 ++++++++++++++++++++++++++++-----
 2 files changed, 390 insertions(+), 418 deletions(-)
 delete mode 100644 Documentation/technical/api-strbuf.txt

diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
deleted file mode 100644
index cca6543..0000000
--- a/Documentation/technical/api-strbuf.txt
+++ /dev/null
@@ -1,351 +0,0 @@
-strbuf API
-==========
-
-strbuf's are meant to be used with all the usual C string and memory
-APIs. Given that the length of the buffer is known, it's often better to
-use the mem* functions than a str* one (memchr vs. strchr e.g.).
-Though, one has to be careful about the fact that str* functions often
-stop on NULs and that strbufs may have embedded NULs.
-
-A strbuf is NUL terminated for convenience, but no function in the
-strbuf API actually relies on the string being free of NULs.
-
-strbufs have some invariants that are very important to keep in mind:
-
-. The `buf` member is never NULL, so it can be used in any usual C
-string operations safely. strbuf's _have_ to be initialized either by
-`strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
-+
-Do *not* assume anything on what `buf` really is (e.g. if it is
-allocated memory or not), use `strbuf_detach()` to unwrap a memory
-buffer from its strbuf shell in a safe way. That is the sole supported
-way. This will give you a malloced buffer that you can later `free()`.
-+
-However, it is totally safe to modify anything in the string pointed by
-the `buf` member, between the indices `0` and `len-1` (inclusive).
-
-. The `buf` member is a byte array that has at least `len + 1` bytes
-  allocated. The extra byte is used to store a `'\0'`, allowing the
-  `buf` member to be a valid C-string. Every strbuf function ensure this
-  invariant is preserved.
-+
-NOTE: It is OK to "play" with the buffer directly if you work it this
-      way:
-+
-----
-strbuf_grow(sb, SOME_SIZE); <1>
-strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
-----
-<1> Here, the memory array starting at `sb->buf`, and of length
-`strbuf_avail(sb)` is all yours, and you can be sure that
-`strbuf_avail(sb)` is at least `SOME_SIZE`.
-+
-NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
-+
-Doing so is safe, though if it has to be done in many places, adding the
-missing API to the strbuf module is the way to go.
-+
-WARNING: Do _not_ assume that the area that is yours is of size `alloc
-- 1` even if it's true in the current implementation. Alloc is somehow a
-"private" member that should not be messed with. Use `strbuf_avail()`
-instead.
-
-Data structures
----------------
-
-* `struct strbuf`
-
-This is the string buffer structure. The `len` member can be used to
-determine the current length of the string, and `buf` member provides
-access to the string itself.
-
-Functions
----------
-
-* Life cycle
-
-`strbuf_init`::
-
-	Initialize the structure. The second parameter can be zero or a bigger
-	number to allocate memory, in case you want to prevent further reallocs.
-
-`strbuf_release`::
-
-	Release a string buffer and the memory it used. You should not use the
-	string buffer after using this function, unless you initialize it again.
-
-`strbuf_detach`::
-
-	Detach the string from the strbuf and returns it; you now own the
-	storage the string occupies and it is your responsibility from then on
-	to release it with `free(3)` when you are done with it.
-
-`strbuf_attach`::
-
-	Attach a string to a buffer. You should specify the string to attach,
-	the current length of the string and the amount of allocated memory.
-	The amount must be larger than the string length, because the string you
-	pass is supposed to be a NUL-terminated string.  This string _must_ be
-	malloc()ed, and after attaching, the pointer cannot be relied upon
-	anymore, and neither be free()d directly.
-
-`strbuf_swap`::
-
-	Swap the contents of two string buffers.
-
-* Related to the size of the buffer
-
-`strbuf_avail`::
-
-	Determine the amount of allocated but unused memory.
-
-`strbuf_grow`::
-
-	Ensure that at least this amount of unused memory is available after
-	`len`. This is used when you know a typical size for what you will add
-	and want to avoid repetitive automatic resizing of the underlying buffer.
-	This is never a needed operation, but can be critical for performance in
-	some cases.
-
-`strbuf_setlen`::
-
-	Set the length of the buffer to a given value. This function does *not*
-	allocate new memory, so you should not perform a `strbuf_setlen()` to a
-	length that is larger than `len + strbuf_avail()`. `strbuf_setlen()` is
-	just meant as a 'please fix invariants from this strbuf I just messed
-	with'.
-
-`strbuf_reset`::
-
-	Empty the buffer by setting the size of it to zero.
-
-* Related to the contents of the buffer
-
-`strbuf_trim`::
-
-	Strip whitespace from the beginning and end of a string.
-	Equivalent to performing `strbuf_rtrim()` followed by `strbuf_ltrim()`.
-
-`strbuf_rtrim`::
-
-	Strip whitespace from the end of a string.
-
-`strbuf_ltrim`::
-
-	Strip whitespace from the beginning of a string.
-
-`strbuf_reencode`::
-
-	Replace the contents of the strbuf with a reencoded form.  Returns -1
-	on error, 0 on success.
-
-`strbuf_tolower`::
-
-	Lowercase each character in the buffer using `tolower`.
-
-`strbuf_cmp`::
-
-	Compare two buffers. Returns an integer less than, equal to, or greater
-	than zero if the first buffer is found, respectively, to be less than,
-	to match, or be greater than the second buffer.
-
-* Adding data to the buffer
-
-NOTE: All of the functions in this section will grow the buffer as necessary.
-If they fail for some reason other than memory shortage and the buffer hadn't
-been allocated before (i.e. the `struct strbuf` was set to `STRBUF_INIT`),
-then they will free() it.
-
-`strbuf_addch`::
-
-	Add a single character to the buffer.
-
-`strbuf_addchars`::
-
-	Add a character the specified number of times to the buffer.
-
-`strbuf_insert`::
-
-	Insert data to the given position of the buffer. The remaining contents
-	will be shifted, not overwritten.
-
-`strbuf_remove`::
-
-	Remove given amount of data from a given position of the buffer.
-
-`strbuf_splice`::
-
-	Remove the bytes between `pos..pos+len` and replace it with the given
-	data.
-
-`strbuf_add_commented_lines`::
-
-	Add a NUL-terminated string to the buffer. Each line will be prepended
-	by a comment character and a blank.
-
-`strbuf_add`::
-
-	Add data of given length to the buffer.
-
-`strbuf_addstr`::
-
-Add a NUL-terminated string to the buffer.
-+
-NOTE: This function will *always* be implemented as an inline or a macro
-that expands to:
-+
-----
-strbuf_add(..., s, strlen(s));
-----
-+
-Meaning that this is efficient to write things like:
-+
-----
-strbuf_addstr(sb, "immediate string");
-----
-
-`strbuf_addbuf`::
-
-	Copy the contents of another buffer at the end of the current one.
-
-`strbuf_adddup`::
-
-	Copy part of the buffer from a given position till a given length to the
-	end of the buffer.
-
-`strbuf_expand`::
-
-	This function can be used to expand a format string containing
-	placeholders. To that end, it parses the string and calls the specified
-	function for every percent sign found.
-+
-The callback function is given a pointer to the character after the `%`
-and a pointer to the struct strbuf.  It is expected to add the expanded
-version of the placeholder to the strbuf, e.g. to add a newline
-character if the letter `n` appears after a `%`.  The function returns
-the length of the placeholder recognized and `strbuf_expand()` skips
-over it.
-+
-The format `%%` is automatically expanded to a single `%` as a quoting
-mechanism; callers do not need to handle the `%` placeholder themselves,
-and the callback function will not be invoked for this placeholder.
-+
-All other characters (non-percent and not skipped ones) are copied
-verbatim to the strbuf.  If the callback returned zero, meaning that the
-placeholder is unknown, then the percent sign is copied, too.
-+
-In order to facilitate caching and to make it possible to give
-parameters to the callback, `strbuf_expand()` passes a context pointer,
-which can be used by the programmer of the callback as she sees fit.
-
-`strbuf_expand_dict_cb`::
-
-	Used as callback for `strbuf_expand()`, expects an array of
-	struct strbuf_expand_dict_entry as context, i.e. pairs of
-	placeholder and replacement string.  The array needs to be
-	terminated by an entry with placeholder set to NULL.
-
-`strbuf_addbuf_percentquote`::
-
-	Append the contents of one strbuf to another, quoting any
-	percent signs ("%") into double-percents ("%%") in the
-	destination. This is useful for literal data to be fed to either
-	strbuf_expand or to the *printf family of functions.
-
-`strbuf_humanise_bytes`::
-
-	Append the given byte size as a human-readable string (i.e. 12.23 KiB,
-	3.50 MiB).
-
-`strbuf_addf`::
-
-	Add a formatted string to the buffer.
-
-`strbuf_commented_addf`::
-
-	Add a formatted string prepended by a comment character and a
-	blank to the buffer.
-
-`strbuf_fread`::
-
-	Read a given size of data from a FILE* pointer to the buffer.
-+
-NOTE: The buffer is rewound if the read fails. If -1 is returned,
-`errno` must be consulted, like you would do for `read(3)`.
-`strbuf_read()`, `strbuf_read_file()` and `strbuf_getline()` has the
-same behaviour as well.
-
-`strbuf_read`::
-
-	Read the contents of a given file descriptor. The third argument can be
-	used to give a hint about the file size, to avoid reallocs.
-
-`strbuf_read_file`::
-
-	Read the contents of a file, specified by its path. The third argument
-	can be used to give a hint about the file size, to avoid reallocs.
-
-`strbuf_readlink`::
-
-	Read the target of a symbolic link, specified by its path.  The third
-	argument can be used to give a hint about the size, to avoid reallocs.
-
-`strbuf_getline`::
-
-	Read a line from a FILE *, overwriting the existing contents
-	of the strbuf. The second argument specifies the line
-	terminator character, typically `'\n'`.
-	Reading stops after the terminator or at EOF.  The terminator
-	is removed from the buffer before returning.  Returns 0 unless
-	there was nothing left before EOF, in which case it returns `EOF`.
-
-`strbuf_getwholeline`::
-
-	Like `strbuf_getline`, but keeps the trailing terminator (if
-	any) in the buffer.
-
-`strbuf_getwholeline_fd`::
-
-	Like `strbuf_getwholeline`, but operates on a file descriptor.
-	It reads one character at a time, so it is very slow.  Do not
-	use it unless you need the correct position in the file
-	descriptor.
-
-`strbuf_getcwd`::
-
-	Set the buffer to the path of the current working directory.
-
-`strbuf_add_absolute_path`
-
-	Add a path to a buffer, converting a relative path to an
-	absolute one in the process.  Symbolic links are not
-	resolved.
-
-`stripspace`::
-
-	Strip whitespace from a buffer. The second parameter controls if
-	comments are considered contents to be removed or not.
-
-`strbuf_split_buf`::
-`strbuf_split_str`::
-`strbuf_split_max`::
-`strbuf_split`::
-
-	Split a string or strbuf into a list of strbufs at a specified
-	terminator character.  The returned substrings include the
-	terminator characters.  Some of these functions take a `max`
-	parameter, which, if positive, limits the output to that
-	number of substrings.
-
-`strbuf_list_free`::
-
-	Free a list of strbufs (for example, the return values of the
-	`strbuf_split()` functions).
-
-`launch_editor`::
-
-	Launch the user preferred editor to edit a file and fill the buffer
-	with the file's contents upon the user completing their editing. The
-	third argument can be used to set the environment which the editor is
-	run in. If the buffer is NULL the editor is launched as usual but the
-	file's contents are not read into the buffer upon completion.
diff --git a/strbuf.h b/strbuf.h
index 652b6c4..b4050de 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,22 +1,117 @@
 #ifndef STRBUF_H
 #define STRBUF_H
 
-/* See Documentation/technical/api-strbuf.txt */
+/**
+ * strbuf's are meant to be used with all the usual C string and memory
+ * APIs. Given that the length of the buffer is known, it's often better to
+ * use the mem* functions than a str* one (memchr vs. strchr e.g.).
+ * Though, one has to be careful about the fact that str* functions often
+ * stop on NULs and that strbufs may have embedded NULs.
+ *
+ * A strbuf is NUL terminated for convenience, but no function in the
+ * strbuf API actually relies on the string being free of NULs.
+ *
+ * strbufs have some invariants that are very important to keep in mind:
+ *
+ * . The `buf` member is never NULL, so it can be used in any usual C
+ * string operations safely. strbuf's _have_ to be initialized either by
+ * `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
+ * +
+ * Do *not* assume anything on what `buf` really is (e.g. if it is
+ * allocated memory or not), use `strbuf_detach()` to unwrap a memory
+ * buffer from its strbuf shell in a safe way. That is the sole supported
+ * way. This will give you a malloced buffer that you can later `free()`.
+ * +
+ * However, it is totally safe to modify anything in the string pointed by
+ * the `buf` member, between the indices `0` and `len-1` (inclusive).
+ *
+ * . The `buf` member is a byte array that has at least `len + 1` bytes
+ *   allocated. The extra byte is used to store a `'\0'`, allowing the
+ *   `buf` member to be a valid C-string. Every strbuf function ensure this
+ *   invariant is preserved.
+ * +
+ * NOTE: It is OK to "play" with the buffer directly if you work it this
+ *       way:
+ * +
+ * ----
+ * strbuf_grow(sb, SOME_SIZE); <1>
+ * strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
+ * ----
+ * <1> Here, the memory array starting at `sb->buf`, and of length
+ * `strbuf_avail(sb)` is all yours, and you can be sure that
+ * `strbuf_avail(sb)` is at least `SOME_SIZE`.
+ * +
+ * NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
+ * +
+ * Doing so is safe, though if it has to be done in many places, adding the
+ * missing API to the strbuf module is the way to go.
+ * +
+ * WARNING: Do _not_ assume that the area that is yours is of size `alloc
+ * - 1` even if it's true in the current implementation. Alloc is somehow a
+ * "private" member that should not be messed with. Use `strbuf_avail()`
+ * instead.
+ */
 
-extern char strbuf_slopbuf[];
+/**
+ * Data Structures
+ * ---------------
+ */
+
+/**
+ * This is the string buffer structure. The `len` member can be used to
+ * determine the current length of the string, and `buf` member provides
+ * access to the string itself.
+ */
 struct strbuf {
 	size_t alloc;
 	size_t len;
 	char *buf;
 };
 
+extern char strbuf_slopbuf[];
 #define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
 
-/*----- strbuf life cycle -----*/
+/**
+ * Functions
+ * ---------
+ */
+
+/**
+ * * Life Cycle
+ */
+
+/**
+ * Initialize the structure. The second parameter can be zero or a bigger
+ * number to allocate memory, in case you want to prevent further reallocs.
+ */
 extern void strbuf_init(struct strbuf *, size_t);
+
+/**
+ * Release a string buffer and the memory it used. You should not use the
+ * string buffer after using this function, unless you initialize it again.
+ */
 extern void strbuf_release(struct strbuf *);
+
+/**
+ * Detach the string from the strbuf and returns it; you now own the
+ * storage the string occupies and it is your responsibility from then on
+ * to release it with `free(3)` when you are done with it.
+ */
 extern char *strbuf_detach(struct strbuf *, size_t *);
+
+/**
+ * Attach a string to a buffer. You should specify the string to attach,
+ * the current length of the string and the amount of allocated memory.
+ * The amount must be larger than the string length, because the string you
+ * pass is supposed to be a NUL-terminated string.  This string _must_ be
+ * malloc()ed, and after attaching, the pointer cannot be relied upon
+ * anymore, and neither be free()d directly.
+ */
 extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);
+
+/**
+ * Swap the contents of two string buffers.
+ */
 static inline void strbuf_swap(struct strbuf *a, struct strbuf *b)
 {
 	struct strbuf tmp = *a;
@@ -24,14 +119,35 @@ static inline void strbuf_swap(struct strbuf *a, struct strbuf *b)
 	*b = tmp;
 }
 
-/*----- strbuf size related -----*/
+
+/**
+ * * Related to the size of the buffer
+ */
+
+/**
+ * Determine the amount of allocated but unused memory.
+ */
 static inline size_t strbuf_avail(const struct strbuf *sb)
 {
 	return sb->alloc ? sb->alloc - sb->len - 1 : 0;
 }
 
+/**
+ * Ensure that at least this amount of unused memory is available after
+ * `len`. This is used when you know a typical size for what you will add
+ * and want to avoid repetitive automatic resizing of the underlying buffer.
+ * This is never a needed operation, but can be critical for performance in
+ * some cases.
+ */
 extern void strbuf_grow(struct strbuf *, size_t);
 
+/**
+ * Set the length of the buffer to a given value. This function does *not*
+ * allocate new memory, so you should not perform a `strbuf_setlen()` to a
+ * length that is larger than `len + strbuf_avail()`. `strbuf_setlen()` is
+ * just meant as a 'please fix invariants from this strbuf I just messed
+ * with'.
+ */
 static inline void strbuf_setlen(struct strbuf *sb, size_t len)
 {
 	if (len > (sb->alloc ? sb->alloc - 1 : 0))
@@ -39,16 +155,277 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)
 	sb->len = len;
 	sb->buf[len] = '\0';
 }
+
+/**
+ * Empty the buffer by setting the size of it to zero.
+ */
 #define strbuf_reset(sb)  strbuf_setlen(sb, 0)
 
-/*----- content related -----*/
+
+/**
+ * * Related to the contents of the buffer
+ */
+
+/**
+ * Strip whitespace from the beginning and end of a string.
+ * Equivalent to performing `strbuf_rtrim()` followed by `strbuf_ltrim()`.
+ */
 extern void strbuf_trim(struct strbuf *);
+
+/**
+ * Strip whitespace from the end of a string.
+ */
 extern void strbuf_rtrim(struct strbuf *);
+
+/**
+ * Strip whitespace from the beginning of a string.
+ */
 extern void strbuf_ltrim(struct strbuf *);
+
+/**
+ * Replace the contents of the strbuf with a reencoded form.  Returns -1
+ * on error, 0 on success.
+ */
 extern int strbuf_reencode(struct strbuf *sb, const char *from, const char *to);
+
+/**
+ * Lowercase each character in the buffer using `tolower`.
+ */
 extern void strbuf_tolower(struct strbuf *sb);
+
+/**
+ * Compare two buffers. Returns an integer less than, equal to, or greater
+ * than zero if the first buffer is found, respectively, to be less than,
+ * to match, or be greater than the second buffer.
+ */
 extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
+
+/**
+ * * Adding data to the buffer
+ *
+ * NOTE: All of the functions in this section will grow the buffer as
+ * necessary.  If they fail for some reason other than memory shortage and the
+ * buffer hadn't been allocated before (i.e. the `struct strbuf` was set to
+ * `STRBUF_INIT`), then they will free() it.
+ */
+
+/**
+ * Add a single character to the buffer.
+ */
+static inline void strbuf_addch(struct strbuf *sb, int c)
+{
+	strbuf_grow(sb, 1);
+	sb->buf[sb->len++] = c;
+	sb->buf[sb->len] = '\0';
+}
+
+/**
+ * Add a character the specified number of times to the buffer.
+ */
+extern void strbuf_addchars(struct strbuf *sb, int c, size_t n);
+
+/**
+ * Insert data to the given position of the buffer. The remaining contents
+ * will be shifted, not overwritten.
+ */
+extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);
+
+/**
+ * Remove given amount of data from a given position of the buffer.
+ */
+extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
+
+/**
+ * Remove the bytes between `pos..pos+len` and replace it with the given
+ * data.
+ */
+extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
+			  const void *, size_t);
+
+/**
+ * Add a NUL-terminated string to the buffer. Each line will be prepended
+ * by a comment character and a blank.
+ */
+extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size);
+
+
+/**
+ * Add data of given length to the buffer.
+ */
+extern void strbuf_add(struct strbuf *, const void *, size_t);
+
+/**
+ * Add a NUL-terminated string to the buffer.
+ *
+ * NOTE: This function will *always* be implemented as an inline or a macro
+ * using strlen, meaning that this is efficient to write things like:
+ *
+ * ----
+ * strbuf_addstr(sb, "immediate string");
+ * ----
+ *
+ */
+static inline void strbuf_addstr(struct strbuf *sb, const char *s)
+{
+	strbuf_add(sb, s, strlen(s));
+}
+
+/**
+ * Copy the contents of another buffer at the end of the current one.
+ */
+static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2)
+{
+	strbuf_grow(sb, sb2->len);
+	strbuf_add(sb, sb2->buf, sb2->len);
+}
+
+/**
+ * Copy part of the buffer from a given position till a given length to the
+ * end of the buffer.
+ */
+extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
+
+/**
+ * This function can be used to expand a format string containing
+ * placeholders. To that end, it parses the string and calls the specified
+ * function for every percent sign found.
+ *
+ * The callback function is given a pointer to the character after the `%`
+ * and a pointer to the struct strbuf.  It is expected to add the expanded
+ * version of the placeholder to the strbuf, e.g. to add a newline
+ * character if the letter `n` appears after a `%`.  The function returns
+ * the length of the placeholder recognized and `strbuf_expand()` skips
+ * over it.
+ *
+ * The format `%%` is automatically expanded to a single `%` as a quoting
+ * mechanism; callers do not need to handle the `%` placeholder themselves,
+ * and the callback function will not be invoked for this placeholder.
+ *
+ * All other characters (non-percent and not skipped ones) are copied
+ * verbatim to the strbuf.  If the callback returned zero, meaning that the
+ * placeholder is unknown, then the percent sign is copied, too.
+ *
+ * In order to facilitate caching and to make it possible to give
+ * parameters to the callback, `strbuf_expand()` passes a context pointer,
+ * which can be used by the programmer of the callback as she sees fit.
+ */
+typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
+extern void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn, void *context);
+
+/**
+ * Used as callback for `strbuf_expand()`, expects an array of
+ * struct strbuf_expand_dict_entry as context, i.e. pairs of
+ * placeholder and replacement string.  The array needs to be
+ * terminated by an entry with placeholder set to NULL.
+ */
+struct strbuf_expand_dict_entry {
+	const char *placeholder;
+	const char *value;
+};
+extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);
+
+/**
+ * Append the contents of one strbuf to another, quoting any
+ * percent signs ("%") into double-percents ("%%") in the
+ * destination. This is useful for literal data to be fed to either
+ * strbuf_expand or to the *printf family of functions.
+ */
+extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src);
+
+/**
+ * Append the given byte size as a human-readable string (i.e. 12.23 KiB,
+ * 3.50 MiB).
+ */
+extern void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes);
+
+/**
+ * Add a formatted string to the buffer.
+ */
+__attribute__((format (printf,2,3)))
+extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
+
+/**
+ * Add a formatted string prepended by a comment character and a
+ * blank to the buffer.
+ */
+__attribute__((format (printf, 2, 3)))
+extern void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
+
+__attribute__((format (printf,2,0)))
+extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
+
+/**
+ * Read a given size of data from a FILE* pointer to the buffer.
+ *
+ * NOTE: The buffer is rewound if the read fails. If -1 is returned,
+ * `errno` must be consulted, like you would do for `read(3)`.
+ * `strbuf_read()`, `strbuf_read_file()` and `strbuf_getline()` has the
+ * same behaviour as well.
+ */
+extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
+
+/**
+ * Read the contents of a given file descriptor. The third argument can be
+ * used to give a hint about the file size, to avoid reallocs.  If read fails,
+ * any partial read is undone.
+ */
+extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
+
+/**
+ * Read the contents of a file, specified by its path. The third argument
+ * can be used to give a hint about the file size, to avoid reallocs.
+ */
+extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
+
+/**
+ * Read the target of a symbolic link, specified by its path.  The third
+ * argument can be used to give a hint about the size, to avoid reallocs.
+ */
+extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
+
+/**
+ * Read a line from a FILE *, overwriting the existing contents
+ * of the strbuf. The second argument specifies the line
+ * terminator character, typically `'\n'`.
+ * Reading stops after the terminator or at EOF.  The terminator
+ * is removed from the buffer before returning.  Returns 0 unless
+ * there was nothing left before EOF, in which case it returns `EOF`.
+ */
+extern int strbuf_getline(struct strbuf *, FILE *, int);
+
+/**
+ * Like `strbuf_getline`, but keeps the trailing terminator (if
+ * any) in the buffer.
+ */
+extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
+
+/**
+ * Like `strbuf_getwholeline`, but operates on a file descriptor.
+ * It reads one character at a time, so it is very slow.  Do not
+ * use it unless you need the correct position in the file
+ * descriptor.
+ */
+extern int strbuf_getwholeline_fd(struct strbuf *, int, int);
+
+/**
+ * Set the buffer to the path of the current working directory.
+ */
+extern int strbuf_getcwd(struct strbuf *sb);
+
+/**
+ * Add a path to a buffer, converting a relative path to an
+ * absolute one in the process.  Symbolic links are not
+ * resolved.
+ */
+extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
+
+/**
+ * Strip whitespace from a buffer. The second parameter controls if
+ * comments are considered contents to be removed or not.
+ */
+extern void stripspace(struct strbuf *buf, int skip_comments);
+
 static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)
 {
 	if (strip_suffix_mem(sb->buf, &sb->len, suffix)) {
@@ -110,51 +487,14 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
  */
 extern void strbuf_list_free(struct strbuf **);
 
-/*----- add data in your buffer -----*/
-static inline void strbuf_addch(struct strbuf *sb, int c)
-{
-	strbuf_grow(sb, 1);
-	sb->buf[sb->len++] = c;
-	sb->buf[sb->len] = '\0';
-}
-
-extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);
-extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);
-
-/* splice pos..pos+len with given data */
-extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
-                          const void *, size_t);
-
-extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf, size_t size);
-
-extern void strbuf_add(struct strbuf *, const void *, size_t);
-static inline void strbuf_addstr(struct strbuf *sb, const char *s)
-{
-	strbuf_add(sb, s, strlen(s));
-}
-static inline void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2)
-{
-	strbuf_grow(sb, sb2->len);
-	strbuf_add(sb, sb2->buf, sb2->len);
-}
-extern void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len);
-extern void strbuf_addchars(struct strbuf *sb, int c, size_t n);
-
-typedef size_t (*expand_fn_t) (struct strbuf *sb, const char *placeholder, void *context);
-extern void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn, void *context);
-struct strbuf_expand_dict_entry {
-	const char *placeholder;
-	const char *value;
-};
-extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder, void *context);
-extern void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src);
-
-__attribute__((format (printf,2,3)))
-extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
-__attribute__((format (printf, 2, 3)))
-extern void strbuf_commented_addf(struct strbuf *sb, const char *fmt, ...);
-__attribute__((format (printf,2,0)))
-extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
+/**
+ * Launch the user preferred editor to edit a file and fill the buffer
+ * with the file's contents upon the user completing their editing. The
+ * third argument can be used to set the environment which the editor is
+ * run in. If the buffer is NULL the editor is launched as usual but the
+ * file's contents are not read into the buffer upon completion.
+ */
+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);
 
@@ -170,28 +510,11 @@ static inline void strbuf_complete_line(struct strbuf *sb)
 		strbuf_addch(sb, '\n');
 }
 
-extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
-/* XXX: if read fails, any partial read is undone */
-extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
-extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
-extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
-extern int strbuf_getcwd(struct strbuf *sb);
-
-extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
-extern int strbuf_getline(struct strbuf *, FILE *, int);
-extern int strbuf_getwholeline_fd(struct strbuf *, int, int);
-
-extern void stripspace(struct strbuf *buf, int skip_comments);
-extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
-
 extern int strbuf_branchname(struct strbuf *sb, const char *name);
 extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
 
 extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
 				    int reserved);
-extern void strbuf_humanise_bytes(struct strbuf *buf, off_t bytes);
-
-extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
 
 __attribute__((format (printf,1,2)))
 extern int printf_ln(const char *fmt, ...);
-- 
2.2.1.425.g441bb3c

[PATCH 2/7] strbuf.h: unify documentation comments beginnings

[Jeff King]

From: Stefan Beller <sbeller@google.com>

The prior patch uses "/**" to denote "documentation"
comments that we pulled from api-strbuf.txt. Let's use a
consistent style for similar comments that were already in
strbuf.h.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Jeff King <peff@peff.net>
---
This adds one spot that was missed in the original. I also rewrote the
commit message, as I found the original hard to parse.

 strbuf.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index b4050de..fd57e45 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -435,7 +435,7 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)
 		return 0;
 }
 
-/*
+/**
  * Split str (of length slen) at the specified terminator character.
  * Return a null-terminated array of pointers to strbuf objects
  * holding the substrings.  The substrings include the terminator,
@@ -451,7 +451,7 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)
 extern struct strbuf **strbuf_split_buf(const char *, size_t,
 					int terminator, int max);
 
-/*
+/**
  * Split a NUL-terminated string at the specified terminator
  * character.  See strbuf_split_buf() for more information.
  */
@@ -461,7 +461,7 @@ static inline struct strbuf **strbuf_split_str(const char *str,
 	return strbuf_split_buf(str, strlen(str), terminator, max);
 }
 
-/*
+/**
  * Split a strbuf at the specified terminator character.  See
  * strbuf_split_buf() for more information.
  */
@@ -471,7 +471,7 @@ static inline struct strbuf **strbuf_split_max(const struct strbuf *sb,
 	return strbuf_split_buf(sb->buf, sb->len, terminator, max);
 }
 
-/*
+/**
  * Split a strbuf at the specified terminator character.  See
  * strbuf_split_buf() for more information.
  */
@@ -481,7 +481,7 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
 	return strbuf_split_max(sb, terminator, 0);
 }
 
-/*
+/**
  * Free a NULL-terminated list of strbufs (for example, the return
  * values of the strbuf_split*() functions).
  */
@@ -498,7 +498,7 @@ extern int launch_editor(const char *path, struct strbuf *buffer, const char *co
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);
 
-/*
+/**
  * Append s to sb, with the characters '<', '>', '&' and '"' converted
  * into XML entities.
  */
@@ -523,7 +523,7 @@ extern int fprintf_ln(FILE *fp, const char *fmt, ...);
 
 char *xstrdup_tolower(const char *);
 
-/*
+/**
  * Create a newly allocated string using printf format. You can do this easily
  * with a strbuf, but this provides a shortcut to save a few lines.
  */
-- 
2.2.1.425.g441bb3c

[PATCH 3/7] strbuf.h: drop asciidoc list formatting from API docs

[Jeff King]

Using a hanging indent is much more readable. This means we
won't format as asciidoc anymore, but since we don't have a
working system for extracting these comments anyway, it's
probably more important to just make the source readable.

Signed-off-by: Jeff King <peff@peff.net>
---
 strbuf.h | 74 ++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index fd57e45..ab5ff27 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -13,44 +13,44 @@
  *
  * strbufs have some invariants that are very important to keep in mind:
  *
- * . The `buf` member is never NULL, so it can be used in any usual C
- * string operations safely. strbuf's _have_ to be initialized either by
- * `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
- * +
- * Do *not* assume anything on what `buf` really is (e.g. if it is
- * allocated memory or not), use `strbuf_detach()` to unwrap a memory
- * buffer from its strbuf shell in a safe way. That is the sole supported
- * way. This will give you a malloced buffer that you can later `free()`.
- * +
- * However, it is totally safe to modify anything in the string pointed by
- * the `buf` member, between the indices `0` and `len-1` (inclusive).
+ *  - The `buf` member is never NULL, so it can be used in any usual C
+ *    string operations safely. strbuf's _have_ to be initialized either by
+ *    `strbuf_init()` or by `= STRBUF_INIT` before the invariants, though.
  *
- * . The `buf` member is a byte array that has at least `len + 1` bytes
- *   allocated. The extra byte is used to store a `'\0'`, allowing the
- *   `buf` member to be a valid C-string. Every strbuf function ensure this
- *   invariant is preserved.
- * +
- * NOTE: It is OK to "play" with the buffer directly if you work it this
- *       way:
- * +
- * ----
- * strbuf_grow(sb, SOME_SIZE); <1>
- * strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
- * ----
- * <1> Here, the memory array starting at `sb->buf`, and of length
- * `strbuf_avail(sb)` is all yours, and you can be sure that
- * `strbuf_avail(sb)` is at least `SOME_SIZE`.
- * +
- * NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
- * +
- * Doing so is safe, though if it has to be done in many places, adding the
- * missing API to the strbuf module is the way to go.
- * +
- * WARNING: Do _not_ assume that the area that is yours is of size `alloc
- * - 1` even if it's true in the current implementation. Alloc is somehow a
- * "private" member that should not be messed with. Use `strbuf_avail()`
- * instead.
- */
+ *    Do *not* assume anything on what `buf` really is (e.g. if it is
+ *    allocated memory or not), use `strbuf_detach()` to unwrap a memory
+ *    buffer from its strbuf shell in a safe way. That is the sole supported
+ *    way. This will give you a malloced buffer that you can later `free()`.
+ *
+ *    However, it is totally safe to modify anything in the string pointed by
+ *    the `buf` member, between the indices `0` and `len-1` (inclusive).
+ *
+ *  - The `buf` member is a byte array that has at least `len + 1` bytes
+ *    allocated. The extra byte is used to store a `'\0'`, allowing the
+ *    `buf` member to be a valid C-string. Every strbuf function ensure this
+ *    invariant is preserved.
+ *
+ *    NOTE: It is OK to "play" with the buffer directly if you work it this
+ *    way:
+ *
+ *    ----
+ *    strbuf_grow(sb, SOME_SIZE); <1>
+ *    strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
+ *    ----
+ *    <1> Here, the memory array starting at `sb->buf`, and of length
+ *    `strbuf_avail(sb)` is all yours, and you can be sure that
+ *    `strbuf_avail(sb)` is at least `SOME_SIZE`.
+ *
+ *    NOTE: `SOME_OTHER_SIZE` must be smaller or equal to `strbuf_avail(sb)`.
+ *
+ *    Doing so is safe, though if it has to be done in many places, adding the
+ *    missing API to the strbuf module is the way to go.
+ *
+ *    WARNING: Do _not_ assume that the area that is yours is of size `alloc
+ *    - 1` even if it's true in the current implementation. Alloc is somehow a
+ *    "private" member that should not be messed with. Use `strbuf_avail()`
+ *    instead.
+*/
 
 /**
  * Data Structures
-- 
2.2.1.425.g441bb3c

[PATCH 4/7] strbuf.h: format asciidoc code blocks as 4-space indent

[Jeff King]

This is much easier to read when the whole thing is stuffed
inside a comment block. And there is precedent for this
convention in markdown (and just in general ascii text).

Signed-off-by: Jeff King <peff@peff.net>
---
 strbuf.h | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index ab5ff27..caa4dad 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -33,10 +33,9 @@
  *    NOTE: It is OK to "play" with the buffer directly if you work it this
  *    way:
  *
- *    ----
- *    strbuf_grow(sb, SOME_SIZE); <1>
- *    strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
- *    ----
+ *        strbuf_grow(sb, SOME_SIZE); <1>
+ *        strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);
+ *
  *    <1> Here, the memory array starting at `sb->buf`, and of length
  *    `strbuf_avail(sb)` is all yours, and you can be sure that
  *    `strbuf_avail(sb)` is at least `SOME_SIZE`.
@@ -261,9 +260,7 @@ extern void strbuf_add(struct strbuf *, const void *, size_t);
  * NOTE: This function will *always* be implemented as an inline or a macro
  * using strlen, meaning that this is efficient to write things like:
  *
- * ----
- * strbuf_addstr(sb, "immediate string");
- * ----
+ *     strbuf_addstr(sb, "immediate string");
  *
  */
 static inline void strbuf_addstr(struct strbuf *sb, const char *s)
-- 
2.2.1.425.g441bb3c

[PATCH 5/7] strbuf.h: reorganize api function grouping headers

[Jeff King]

The original API doc had something like:

    Functions
    ---------

    * Life cycle

      ... some life-cycle functions ...

    * Related to the contents of the buffer

      ... functions related to contents ....

    etc

This grouping can be hard to read in the comment sources,
given the "*" in the comment lines, and the amount of text
between each section.

Instead, let's make a flat list of groupings, and underline
each as a section header. That makes them stand out, and
eliminates the weird half-phrase of "Related to...". Like:

    Functions related to the contents of the buffer
    -----------------------------------------------

Signed-off-by: Jeff King <peff@peff.net>
---
 strbuf.h | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index caa4dad..6fa7156 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -71,12 +71,8 @@ extern char strbuf_slopbuf[];
 #define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
 
 /**
- * Functions
- * ---------
- */
-
-/**
- * * Life Cycle
+ * Life Cycle Functions
+ * --------------------
  */
 
 /**
@@ -120,7 +116,8 @@ static inline void strbuf_swap(struct strbuf *a, struct strbuf *b)
 
 
 /**
- * * Related to the size of the buffer
+ * Functions related to the size of the buffer
+ * -------------------------------------------
  */
 
 /**
@@ -162,7 +159,8 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)
 
 
 /**
- * * Related to the contents of the buffer
+ * Functions related to the contents of the buffer
+ * -----------------------------------------------
  */
 
 /**
@@ -201,7 +199,8 @@ extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
 
 
 /**
- * * Adding data to the buffer
+ * Adding data to the buffer
+ * -------------------------
  *
  * NOTE: All of the functions in this section will grow the buffer as
  * necessary.  If they fail for some reason other than memory shortage and the
-- 
2.2.1.425.g441bb3c

[PATCH 6/7] strbuf.h: drop boilerplate descriptions of strbuf_split_*

[Jeff King]

The description of strbuf_split_buf says most of what
needs to be said for all of the split variants that take
strings, raw memory, etc. We have a boilerplate comment
above each that points to the first. This boilerplate
ends up making it harder to read, because it spaces out the
functions, which could otherwise be read as a group.

Let's drop the boilerplate completely, and mention the
variants in the top comment. This is perhaps slightly worse
for a hypothetical system which pulls the documentation for
each function out of the comment immediately preceding it.
But such a system does not yet exist, and anyway, the end
result of extracting the boilerplate comments would not lead
to a very easy-to-read result.  We would do better in the
long run to teach the extraction system about groups of
related functions.

Signed-off-by: Jeff King <peff@peff.net>
---
 strbuf.h | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index 6fa7156..61c9c73 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -441,36 +441,29 @@ static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)
  * substring containing everything following the (max-1)th terminator
  * character).
  *
+ * The most generic form is `strbuf_split_buf`, which takes an arbitrary
+ * pointer/len buffer. The `_str` variant takes a NUL-terminated string,
+ * the `_max` variant takes a strbuf, and just `strbuf_split` is a convenience
+ * wrapper to drop the `max` parameter.
+ *
  * For lighter-weight alternatives, see string_list_split() and
  * string_list_split_in_place().
  */
 extern struct strbuf **strbuf_split_buf(const char *, size_t,
 					int terminator, int max);
 
-/**
- * Split a NUL-terminated string at the specified terminator
- * character.  See strbuf_split_buf() for more information.
- */
 static inline struct strbuf **strbuf_split_str(const char *str,
 					       int terminator, int max)
 {
 	return strbuf_split_buf(str, strlen(str), terminator, max);
 }
 
-/**
- * Split a strbuf at the specified terminator character.  See
- * strbuf_split_buf() for more information.
- */
 static inline struct strbuf **strbuf_split_max(const struct strbuf *sb,
 						int terminator, int max)
 {
 	return strbuf_split_buf(sb->buf, sb->len, terminator, max);
 }
 
-/**
- * Split a strbuf at the specified terminator character.  See
- * strbuf_split_buf() for more information.
- */
 static inline struct strbuf **strbuf_split(const struct strbuf *sb,
 					   int terminator)
 {
-- 
2.2.1.425.g441bb3c

[PATCH 7/7] strbuf.h: group documentation for trim functions

[Jeff King]

The relationship between these makes more sense if you read
them as a group, which can help people who are looking for
the right function. Let's give them a single comment.

Signed-off-by: Jeff King <peff@peff.net>
---
 strbuf.h | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index 61c9c73..1883494 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -164,19 +164,11 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)
  */
 
 /**
- * Strip whitespace from the beginning and end of a string.
- * Equivalent to performing `strbuf_rtrim()` followed by `strbuf_ltrim()`.
+ * Strip whitespace from the beginning (`ltrim`), end (`rtrim`), or both side
+ * (`trim`) of a string.
  */
 extern void strbuf_trim(struct strbuf *);
-
-/**
- * Strip whitespace from the end of a string.
- */
 extern void strbuf_rtrim(struct strbuf *);
-
-/**
- * Strip whitespace from the beginning of a string.
- */
 extern void strbuf_ltrim(struct strbuf *);
 
 /**
-- 
2.2.1.425.g441bb3c

Re: [PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> This is a re-roll of this series:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/260922/focus=261374
>
> from early December to move the strbuf documentation into the header
> file.
>
> And of course the elephant in the room is the other dozen or more
> api-*.txt files. I'd propose to do this strbuf.h series (and possible
> follow-ons mentioned above) and stop there for a bit. That will let us
> form a more coherent opinion on whether we like this system in practice,
> how it ages as functions are changed and added, etc. That might affect
> how or if we end up converting other files.
>
> It does leave us in an inconsistent state (some documentation is in
> Documentation/technical, and some is in the headers), but I think that
> is largely where we're at today. IMHO this is a strict improvement
> because at least the logical chunk of "strbuf" is now in a single place.

Is there a general concensus on the direction?

I am inclined to merge this to 'next', if there is a general
understanding that we will try to make the headers _the_ single
source of truth of the API by (1) not adding to api-*.txt without
describing new things in the headers and (2) moving things from
api-*.txt to corresponding headers when clarifying, fixing or
updating the API.

Re: [PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[Jeff King]

On Thu, Feb 12, 2015 at 03:01:18PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > This is a re-roll of this series:
> >
> >   http://thread.gmane.org/gmane.comp.version-control.git/260922/focus=261374
> >
> > from early December to move the strbuf documentation into the header
> > file.
> >
> > And of course the elephant in the room is the other dozen or more
> > api-*.txt files. I'd propose to do this strbuf.h series (and possible
> > follow-ons mentioned above) and stop there for a bit. That will let us
> > form a more coherent opinion on whether we like this system in practice,
> > how it ages as functions are changed and added, etc. That might affect
> > how or if we end up converting other files.
> >
> > It does leave us in an inconsistent state (some documentation is in
> > Documentation/technical, and some is in the headers), but I think that
> > is largely where we're at today. IMHO this is a strict improvement
> > because at least the logical chunk of "strbuf" is now in a single place.
> 
> Is there a general concensus on the direction?
> 
> I am inclined to merge this to 'next', if there is a general
> understanding that we will try to make the headers _the_ single
> source of truth of the API by (1) not adding to api-*.txt without
> describing new things in the headers and (2) moving things from
> api-*.txt to corresponding headers when clarifying, fixing or
> updating the API.

I'm fine with that (unsurprisingly), but I would like to hear an "OK"
from Jonathan before going ahead.

-Peff

Re: [PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Is there a general concensus on the direction?
>> 
>> I am inclined to merge this to 'next', if there is a general
>> understanding that we will try to make the headers _the_ single
>> source of truth of the API by (1) not adding to api-*.txt without
>> describing new things in the headers and (2) moving things from
>> api-*.txt to corresponding headers when clarifying, fixing or
>> updating the API.
>
> I'm fine with that (unsurprisingly), but I would like to hear an "OK"
> from Jonathan before going ahead.

OK.  Jonathan?

Re: [PATCH 0/7] migrate api-strbuf.txt into strbuf.h

[Jonathan Nieder]

Jeff King wrote:
> On Thu, Feb 12, 2015 at 03:01:18PM -0800, Junio C Hamano wrote:

>> I am inclined to merge this to 'next', if there is a general
>> understanding that we will try to make the headers _the_ single
>> source of truth of the API by (1) not adding to api-*.txt without
>> describing new things in the headers and (2) moving things from
>> api-*.txt to corresponding headers when clarifying, fixing or
>> updating the API.
>
> I'm fine with that (unsurprisingly), but I would like to hear an "OK"
> from Jonathan before going ahead.

Sorry for the slow reply.  Sounds good to me.  I'd prefer for the
in-between state to last as short a period as possible, but I realize
that preference isn't all that meaningful in the absence of patches. :)

Jonathan