Re: [PATCH 2/2] send-email: add --header-cmd option

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 25, 2023, 16:29 UTC
Message-ID: <xmqqcz3s3oz7.fsf@gitster.g>
In-Reply-To: <87y1mgortc.fsf@gmail.com>

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Show 6 quoted lines

> I'm not too familiar with the email format; but I presume an empty line
> would signal the end of a message?  That'd be bad yes, but I think it
> cannot currently happen given the 'last if $line =~ /^$/;' guard at in
> execute_cmd around line 2023; it'd means headers following an empty line
> would be discarded though.  The expected use case is indeed for a user's
> script to produce RFC 2822 style headers to messages.

Yes, silently discarding the end-user input is what I meant by a disaster.

> The former (a true default with no way to turn it off other than
> redefining it), which I believe is the same behavior as for --cc-cmd or
> --to-cmd.  There are no '--no-cc-cmd' or '--no-to-cmd' options, although
> their result can be filtered via the '--no-cc' and '--no-to' options.

Yup.

> Looking in the source, options supporting '--no-' always appear to be
> boolean toggles (on/off) though, so I'm not sure how a '--no-header-cmd'
> that take a value can currently be implemented.  Perhaps it could be
> added later if there is a need?

Perhaps we can do without a configuration variable first, and perhaps the variable could be added later if there is a need and a proper way to turn it off per invocation basis.

> I've extracted such postprocessing into fold_headers and applied
> execute_cmd to it in new invoke_header_cmd subroutine.

Sounds like a good approach (without looking the actual patch).

Thanks.