git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/4] strbuf: migrate api-strbuf.txt documentation to strbuf.h

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Dec 12, 2014, 22:30 UTC
Message-ID
<20141212223012.GC29365@google.com>
In-Reply-To
<20141212212800.GA27451@peff.net>
Jeff King wrote:
Show 6 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/technical/api-strbuf.txt | 351 -------------------------
>  strbuf.h                               | 458 ++++++++++++++++++++++++++++-----
>  2 files changed, 390 insertions(+), 419 deletions(-)
>  delete mode 100644 Documentation/technical/api-strbuf.txt

Thanks. This looks very sensible. Some suggested tweaks below (see patch at bottom).

> +++ b/strbuf.h
[...]
> +/**
> + * * Life Cycle
[...]
> +/**
> + * Adding data to the buffer
These were headings like
	* Life Cycle
	* Adding data to the buffer

Most ended up as ' * *' after the conversion, keeping the extra '*', but 'Adding data to the buffer' didn't.

After patch 4/4 they look the same again, so it doesn't matter.
[...]
Show 13 quoted lines
>   * 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:
> + * using strlen, meaning that this is efficient to write things like:
>   *
>   * ----
>   * strbuf_addstr(sb, "immediate string");
>   * ----
Makes sense.
[...]
Show 6 quoted lines
>  /**
>   * 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.
> - */
> + * used to give a hint about the file size, to avoid reallocs.  If read fails,
> + * any partial read is undone */
Missing period at end of sentence.  "*/" should go on the next line.
[...]
Show 6 quoted lines
> +extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
> +
> +/**
> + * Set the buffer to the path of the current working directory.
> + */
> +extern int strbuf_getcwd(struct strbuf *sb);

This snuck up a few lines --- it was between getwholeline_fd and add_absolute_path before. I think it makes sense for getcwd to be next to add_absolute_path.

[...]
Show 10 quoted lines
> +extern void stripspace(struct strbuf *buf, int skip_comments);
> +
> +/**
> + * 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);

split_* were before this before. Probably launch_editor belongs with read_file and co instead, but during this conversion it makes sense to just keep the order from before.

[...]
Show 20 quoted lines
> - * `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).
> +/*
> + * 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,
> + * except for the last substring, which might be unterminated if the
[...]
Show 19 quoted lines
> + */
> +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,

This is something that the flat file format did better. Would it work to do something similar, for example by dropping the boilerplate comment before the inline functions (not shown in the patch below)?

	/**
	 * Split a string or strbuf into a list of strbufs at a specified
	 * terminator character.  [...]
	 */
	extern struct strbuf **strbuf_split_buf(const char *, size_t, int terminator, int max);
	static inline struct strbuf **strbuf_split_str(const char *str, int terminator, int max)
	{
		return strbuf_split_buf(str, strlen(str), terminator, max);
	}
	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);
	}
	static inline struct strbuf **strbuf_split(const struct strbuf *sb, int terminator)
	{
		return strbuf_split_max(sb, terminator, 0);
	}
	/**
	 * Free a list of strbufs (for example, the return values of the
	 * `strbuf_split()` functions).
	 */
	extern void strbuf_list_free(struct strbuf **);
 strbuf.h | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/strbuf.h b/strbuf.h
index 0a83f9a..108644e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -202,7 +202,7 @@ 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
@@ -368,7 +368,8 @@ 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 */
+ * any partial read is undone.
+ */
 extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);
 
 /**
@@ -384,11 +385,6 @@ 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);
 
 /**
- * Set the buffer to the path of the current working directory.
- */
-extern int strbuf_getcwd(struct strbuf *sb);
-
-/**
  * Read a line from a FILE *, overwriting the existing contents
  * of the strbuf. The second argument specifies the line
  * terminator character, typically `'\n'`.
@@ -413,6 +409,11 @@ extern int strbuf_getwholeline(struct strbuf *, FILE *, int);
 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.
@@ -425,15 +426,6 @@ extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
  */
 extern void stripspace(struct strbuf *buf, int skip_comments);
 
-/**
- * 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);
-
 static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)
 {
 	if (strip_suffix_mem(sb->buf, &sb->len, suffix)) {
@@ -495,6 +487,15 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
  */
 extern void strbuf_list_free(struct strbuf **);
 
+/**
+ * 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);
 
 /*
Previous: Junio C HamanoNext: Jeff King
Message 30 of 49 in “document string_list_clear”
  1. document string_list_clearStefan Beller, Dec 6, 2014
  2. Jonathan NiederDec 6, 2014
  3. Jeff KingDec 6, 2014
  4. Jeff KingDec 6, 2014
  5. Junio C HamanoDec 9, 2014
  6. Jeff KingDec 9, 2014
  7. Jonathan NiederDec 9, 2014
  8. Junio C HamanoDec 9, 2014
  9. Stefan BellerDec 9, 2014
  10. Jonathan NiederDec 9, 2014
  11. Jeff KingDec 9, 2014
  12. Stefan BellerDec 9, 2014
  13. Jeff KingDec 9, 2014
  14. Jonathan NiederDec 9, 2014
  15. Junio C HamanoDec 9, 2014
  16. Jeff KingDec 10, 2014
  17. Jeff KingDec 10, 2014
  18. Jonathan NiederDec 10, 2014
  19. Jeff KingDec 12, 2014
  20. Jonathan NiederDec 12, 2014
  21. Junio C HamanoDec 12, 2014
  22. Stefan BellerDec 12, 2014
  23. Jeff KingDec 12, 2014
  24. Jeff KingDec 12, 2014
  25. Jonathan NiederDec 12, 2014
  26. Jeff KingDec 12, 2014
  27. 1/4 strbuf: migrate api-strbuf.txt documentation to strbuf.hJeff King, Dec 12, 2014
  28. Jeff KingDec 12, 2014
  29. Junio C HamanoDec 12, 2014
  30. Jonathan NiederDec 12, 2014
  31. 2/4 strbuf.h: drop asciidoc list formatting from API docsJeff King, Dec 12, 2014
  32. Junio C HamanoDec 12, 2014
  33. Jonathan NiederDec 12, 2014
  34. 3/4 strbuf.h: format asciidoc code blocks as 4-space indentJeff King, Dec 12, 2014
  35. Jonathan NiederDec 12, 2014
  36. Michael HaggertyDec 14, 2014
  37. 4/4 strbuf.h: reorganize api function grouping headersJeff King, Dec 12, 2014
  38. Jonathan NiederDec 12, 2014
  39. Stefan BellerDec 12, 2014
  40. Michael HaggertyDec 10, 2014
  41. Jonathan NiederDec 10, 2014
  42. Junio C HamanoDec 10, 2014
  43. Jonathan NiederDec 10, 2014
  44. Junio C HamanoDec 10, 2014
  45. Jonathan NiederDec 10, 2014
  46. Jonathan NiederDec 9, 2014
  47. Stefan BellerDec 9, 2014
  48. Jonathan NiederDec 9, 2014
  49. Jonathan NiederDec 9, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.