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

[Eric Sunshine <sunshine@sunshineco.com>]

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@sunshineco.com> 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.