On Tue, May 27, 2025, at 10:24, Phillip Wood wrote:

quoted

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?

quoted

quoted

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.