Subject: Re: [PATCH v8 5/6] notes.c: append separator instead of insert by pos

From: Junio C Hamano <[email-masked]>
Date: Tue, 25 Apr 2023 10:47:29 -0700

Teng Long <[email-masked]> writes:

> From: Teng Long <[email-masked]>
>
> This commit rename "insert_separator" to "append_separator" and also

"This commit rename" -> "Rename".

> remove the "postion" argument, this serves two purpose:
>
> The first is that when specifying more than one "-m" ( like "-F", etc)
> to "git notes add" or "git notes append", the order of them matters,
> which means we need to append the each separator and message in turn,
> so we don't have to make the caller specify the position, the "append"
> operation is enough and clear.

OK.

> The second is that when we execute the "git notes append" subcommand
> , we need to combine the "prev_note" and "current_note" to get the

Funny placement of comma?

> +static void append_separator(struct strbuf *message)
>  {
>  	if (!separator)
> -		strbuf_insertstr(message, pos, "\n");
> +		strbuf_insertstr(message, message->len, "\n");
>  	else if (separator[strlen(separator) - 1] == '\n')
> -		strbuf_insertstr(message, pos, separator);
> +		strbuf_insertstr(message, message->len, separator);
>  	else
> -		strbuf_insertf(message, pos, "%s%s", separator, "\n");
> +		strbuf_insertf(message, message->len, "%s%s", separator, "\n");
>  }

Is it still needed to use strbuf_insert*() variants when the only
thing being done is to append at the end?  Use of strbuf_add*()
would let you drop message->len from the arguments.