Subject: Re: [PATCH v8 4/6] notes.c: introduce '--separator=<paragraph-break>' option

From: Junio C Hamano <[email-masked]>
Date: Thu, 27 Apr 2023 11:21:16 -0700

Teng Long <[email-masked]> writes:

> Junio C Hamano <[email-masked]> writes:
>
>>> +	while (d->msg_nr) {
>>> +		--d->msg_nr;
>>> +		strbuf_release(&d->messages[d->msg_nr]->buf);
>>> +		free(d->messages[d->msg_nr]);
>>> +	}
>>
>>        while (d->msg_nr--) {
>>                strbuf_relesae(...);
>>                free(d->messages[d->msg_nr]);
>>        }
>
> Will fix or optimize.

Not a huge thing and I would be very surprised if you saw a
different machine code output.  The original stood out like a sore
thumb at me, primarily because we rarely if ever do pre-decrement or
pre-increment in the void context in our own code.

>>Interesting.  
>>
>>I would have expected that where we add to d->messages[] array a new
>>note_msg structure and set its .stripspace member, we already knew
>>if the associated strbuf needs to be stripspace'd and instead of
>>maintaining the .stripspace member for each note_msg, we can just
>>have it contain only a string (not even a strbuf).
>
> That's because the "-C" option, it will not do stripspace for the
> whole note, but support to use together with "-m" or "-F"(they will
> do stripspace). So, for the consistent result with backforward
> compatibility, we have to get the information about whether to do
> stripspace when concating each message.

OK.

Thanks.