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