Re: [PATCH 07/67] strbuf: make strbuf_complete_line more generic

On Tue, Sep 15, 2015 at 06:27:49PM -0700, Junio C Hamano wrote:

Eric Sunshine [off-list ref] writes:

quoted

quoted

+static inline void strbuf_complete(struct strbuf *sb, char term)
+{
+       if (sb->len && sb->buf[sb->len - 1] != term)
+               strbuf_addch(sb, term);
+}

Hmm, so this only adds 'term' if not already present *and* if 'sb' is
not empty, which doesn't seem to match the documentation which says
that it "ensures" termination.

[...]
So to these two plausible and different set of callers that would be
helped by this function, the behaviour Peff gives it would match
what the callers want better than your version.

Right. I think what the function is doing is the right thing (and
certainly it matches what the callers I'm changing are doing already
:) ).

But I agree the docstring is extremely misleading. I've changed it to:

diff --git a/strbuf.h b/strbuf.h
index aef2794..43f27c3 100644
--- a/strbuf.h
+++ b/strbuf.h

@@ -491,10 +491,21 @@ extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *
  */
 extern void strbuf_addstr_xml_quoted(struct strbuf *sb, const char *s);
 
+/**
+ * "Complete" the contents of `sb` by ensuring that either it ends with the
+ * character `term`, or it is empty.  This can be used, for example,
+ * to ensure that text ends with a newline, but without creating an empty
+ * blank line if there is no content in the first place.
+ */
+static inline void strbuf_complete(struct strbuf *sb, char term)
+{
+	if (sb->len && sb->buf[sb->len - 1] != term)
+		strbuf_addch(sb, term);
+}
+
 static inline void strbuf_complete_line(struct strbuf *sb)
 {
-	if (sb->len && sb->buf[sb->len - 1] != '\n')
-		strbuf_addch(sb, '\n');
+	strbuf_complete(sb, '\n');
 }
 
 extern int strbuf_branchname(struct strbuf *sb, const char *name);

It may be that we will not find other uses beyond completing slash and
newline, but at least this version should not actively mislead anybody.

-Peff