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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2023, 17:47 UTC
Message-ID: <xmqqa5yv26ry.fsf@gitster.g>
In-Reply-To: <ef40e0efc34ce6b6ee5b9b67bdde3dae02652cec.1682429602.git.dyroneteng@gmail.com>

Teng Long <dyroneteng@gmail.com> writes:

> From: Teng Long <dyroneteng@gmail.com>
>
> This commit rename "insert_separator" to "append_separator" and also

"This commit rename" -> "Rename".

Show 7 quoted lines

> 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?

Show 12 quoted lines

> +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.