From: "Ævar Arnfjörð Bjarmason" <email@example.com>
To: Michael Strawbridge <firstname.lastname@example.org>
Subject: Re: [PATCH v9 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
Date: Mon, 23 Jan 2023 14:51:28 +0100 [thread overview]
Message-ID: <email@example.com> (raw)
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/t9001-send-email.sh | 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).
* 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?...
* ...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:
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)
next prev parent reply other threads:[~2023-01-23 14:03 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 ` Ævar Arnfjörð Bjarmason [this message]
2023-01-23 16:03 ` [PATCH v9 0/2] " Michael Strawbridge
2023-01-30 10:40 ` Ævar Arnfjörð Bjarmason
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:
* 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.
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 NNTP newsgroup(s).