Web lists-archives.com

Re: [PATCH v1] convert log_ref_write_fd() to use strbuf




Jeff King <peff@xxxxxxxx> writes:

>> -	while (buf < cp && isspace(cp[-1]))
>> -		cp--;
>> -	*cp++ = '\n';
>> -	return cp - buf;
>> +	strbuf_rtrim(sb);
>
> Using rtrim is a nice reduction in complexity. A pure translation would
> include a final strbuf_addch(sb, '\n'). It looks like you moved that to
> the caller. There's only one, so that's OK now, but it may affect topics
> in flight (and I do in fact have an old topic that calls it).
>
> But I think it's OK, as the change in function signature means that any
> callers will need updated anyway. So there's little risk of a silent
> mis-merge.

It is interesting that we came to a slightly different conclusion,
after doing pretty much the same analysis ;-).  Unless Ben has a
plan to use a version that trims the trailing LF elsewhere, there is
no point changing what the function does, especially because the
existing and only caller does want the terminating LF at the end.