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

On 2023-01-23 08:51, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Jan 19 2023, Michael Strawbridge wrote:
>> Thanks to Ævar for an idea to simplify these patches further.
>> Michael Strawbridge (2):
>>   send-email: refactor header generation functions
>>   send-email: expose header information to git-send-email's
>>     sendemail-validate hook
>>  Documentation/githooks.txt | 27 +++++++++--
>>  git-send-email.perl        | 95 +++++++++++++++++++++++---------------
>>  t/      | 27 ++++++++++-
>>  3 files changed, 106 insertions(+), 43 deletions(-)
> Thanks for the update. Aside from any quibbles, I still have some
> fundimental concerns about the implementation here:
>  * Other hooks take stdin, not this sort of file argument.
>    We discussed that ending in
>    but I probably shouldn't have mentioned "git hook" at all.
>    I do think though that we shouldn't expose a UX discrepancy like this
>    forever, but the ways forward out of that would seem to be to either
>    to revert a7555304546 (send-email: use 'git hook run' for
>    'sendemail-validate', 2021-12-22) & move forward from there, or to
>    wait for those patches (which I'm currentnly CI-ing).

Ok.  If we are at the point where the change is just trying to pass CI
but the main logic is there I am willing to wait some time.

>  * 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

> 	<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?

I appreciate the feedback.

  reply	other threads:[~2023-01-23 16:04 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 [this message]
2023-01-30 10:40     ` Ævar Arnfjörð Bjarmason

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).