Subject: Re: [PATCH v3 1/3] worktree: refactor infer_backlink() to use *strbuf

From: Junio C Hamano <[email-masked]>
Date: Thu, 10 Oct 2024 12:14:55 -0700

Caleb White <[email-masked]> writes:

>> Question here: should we use `strbuf_reset` here? I want to know the
>> reason why you design this. Is the caller's responsibility to clear the
>> "inferred" when calling this function?
>
> Yes we should, sure it is the caller's responsibility but this just helps
> prevent bugs. There's plenty of functions that reset the strbuf that's
> passed to the function before modifying it.

If the API says that it _appends_ what it found in the supplied
strbuf, then it is the caller's responsibility to ensure that the
strbuf being passed is empty, if the caller wants the resulting
strbuf to hold only what the function found.

If the API says it _returns_ what it found in the supplied strbuf,
then the function is responsible to discard what the supplied strbuf
originally has.

As an API design, if it is rarely useful for callers to be able to
append, then the latter is simpler to use, but there are many cases
where the "append" semantics is useful (see strbuf.c to find
examples).

In this particular case, it is rarely useful for the caller to get
the result appended to a string it already has before calling this
function, so unconditionally discarding what the caller had, without
telling the caller that it is responsible for passing an empty buffer,
is the right thing, I wuld think.

Thanks.