On Tue, Sep 15, 2015 at 06:27:49PM -0700, Junio C Hamano wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > >> +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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html