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

On Wed, Sep 16, 2015 at 05:57:41AM -0400, Jeff King wrote:

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

quoted

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:
 
+/**
+ * "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.
+ */

Sounds better, thanks.