mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: Michael Strawbridge <>
Subject: Re: [PATCH v9 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
Date: Mon, 30 Jan 2023 11:40:14 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Mon, Jan 23 2023, Michael Strawbridge wrote:

> On 2023-01-23 08:51, Ævar Arnfjörð Bjarmason wrote:
  * Aside from that, shouldn't we have a new "validate-headers" or
>>    whatever hook, instead of assuming that we can add another argument
>>    to existing users?...
> While it's true we could (and I don't have a super strong opinion here),
> I suppose I was foreseeing the potential that a user may want to have
> logic that requires both the email headers and contents.  For example,
> only checking contents for a specific mailing list.  If we split the
> hooks, a user would then need to figure out how to have them coordinate.


>>  * ...except can we do it safely? Now, it seems to me like you have
>>    potential correctness issues here. We call format_2822_time() to make
>>    the headers, but that's based on "$time", which we save away earlier.
>>    But for the rest (e.g. "Message-Id" are we sure that we're giving the
>>    hook the same headers as the one we actually end up sending?
>>    But regardless of that, something that would bypass this entire
>>    stdin/potential correctness etc. problem is if we just pass an offset
>>    to the the, i.e. currently we have a "validate" which gets the
>>    contents, if we had a "validate-raw" or whatever we could just pass:
> I think there might be a part missing here: "problem is if we just pass
> an offset to the ___."  So there's a chance I may not fully grasp your
> suggestion.

Sorry, a byte offset into the file to indicate the boundary between the
headers and the content.

>> 	<headers>
>> 	\n\n
>> 	<content>
>>    Where the current "validate" just gets "content", no? We could then
>>    either pass the offset to the "\n\n", or just trust that such a hook
>>    knows to find the "\n\n".
>>    I also think that would be more generally usable, as the tiny
>>    addition of some exit code interpretation would allow us to say "I
>>    got this, and consider this sent", which would also satisfy some who
>>    have wanted e.g. a way to intrecept it before it invokes "sendmail"
>>    (I remember a recent thread about that in relation to using "mutt" to
>>    send it directly)
> Are you suggesting to simply add the header to the current
> sendemail-validate hook?

No, I'm saying that we currently don't pass them at all, and your patch
adds another argument to a file with the headers.

That *may* break some existing users if they're only expecting the
current argument(s) (although that's probably unlikely), more
importantly we're now doing extra work for all existing hook users, for
the benefit of only some new users.

So I'm suggesting having some opt-in mechanism for the new semantics,
both to preserve the existing semantics for existing users, and for
current and new users avoid writing out the file etc. when we don't need

Which we could do with a config variable,
e.g. hooks."sendemail-validate".includeHeaders=true, or just by having a
new "sendemail-validate-raw" (or whatever we'd call it).

I think it's fine to enforce that if such a new hook exists we'd take it
over the "sendemail-validate" (if any), i.e. we wouldn't need to support

      reply	other threads:[~2023-01-30 10:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-20  1:24 [PATCH v9 0/2] send-email: expose header information to git-send-email's sendemail-validate hook Michael Strawbridge
2023-01-20  1:24 ` [PATCH v9 1/2] send-email: refactor header generation functions Michael Strawbridge
2023-01-20  1:24 ` [PATCH v9 2/2] send-email: expose header information to git-send-email's sendemail-validate hook Michael Strawbridge
2023-01-20 13:07   ` Luben Tuikov
2023-01-20 14:25     ` Michael Strawbridge
2023-04-19 17:01       ` Junio C Hamano
2023-04-19 18:05         ` Luben Tuikov
2023-01-23 13:51 ` [PATCH v9 0/2] " Ævar Arnfjörð Bjarmason
2023-01-23 16:03   ` Michael Strawbridge
2023-01-30 10:40     ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).