On Tue, May 27, 2025, at 10:24, Phillip Wood wrote:
>> Do you mean doing the operation on the output buffer instead?:
>>
>> if (strbuf_read(&buf, show.out, 0) < 0)
>> die_errno(_("could not read 'show' output"));
>> /* strip trailing whitespace introduced by blank lines */
>> strbuf_stripspace(&buf, NULL);
>> strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);
>> write_or_die(fd, cbuf.buf, cbuf.len);
>>
>> I think that’s cleaner. But I don’t see how it makes the code more
>> future-proof.
>
> Because it is now stripping buf and not cbuf. If in the future we decide
> to build the message in a buffer rather than writing it piecemeal to
> disk we would change signature of this function to take an strbuf
> instead of a file descriptor and use the buffer provided by the caller
> instead of cbuf. If we were to strip cbuf then a naive conversion would
> end up stripping the buffer passed by caller, not just the output from
> "git show". Various git notes subcommands have a --stripspace option and
> calling strbuf_stripspace() on the caller provided buffer would break that.
So stripping `buf` is what I should do?
>>> Should that be " \$"? What you've got seems to work with dash but I'm
>>> not sure if it is POSIX compliant or not.
>>
>> `$` is the anchor metacharacter in this context (end of string)
>> according to Posix.
>
> Right but what does the shell do to that '$'? It is not escaped and
> inside a double quoted string.
Oh right, it’s about the shell. I’ll fix it.
Thanks for spotting.
