git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Sean Allred <allred.sean@gmail.com>,
	Git mailing list <git@vger.kernel.org>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: Should commit-msg hook receive the washed message?
Date: Sat, 6 Jul 2024 02:37:37 -0400	[thread overview]
Message-ID: <20240706063737.GF700645@coredump.intra.peff.net> (raw)
In-Reply-To: <CAPig+cTpxXNwy8MYWjcDTa5QPoq5Mod3_LZ=+F16-gF5QVbrkg@mail.gmail.com>

On Fri, Jul 05, 2024 at 05:35:25PM -0400, Eric Sunshine wrote:

> > This seems like a bug to me; is there something I'm missing? I would
> > propose adding a call to `cleanup_message` (with the appropriate
> > arguments) inside `prepare_to_commit` right before `commit-msg` is
> > invoked.
> 
> The idea of calling cleanup_message() has been discussed before[1]. My
> takeaway from reading that message is that calling cleanup_message()
> unconditionally before invoking the hook could potentially throw away
> information that the hook might want to consult. It's possible to
> imagine a workflow in which a specialized comment is inserted in a
> commit message to control/augment behavior of the hook in some
> fashion.

Yeah, looking over that earlier discussion, I think the main takeaway is
that the unsanitized version might have useful information for the hook.
I don't know of any real workflow that relies on that, but it does seem
possible that somebody has one.

> The idea you proposed in a different thread[2] of exposing
> cleanup_message() functionality as a user-facing utility which a hook
> can call on an as-needed basis may make more sense(?).

So yes, I like that approach much better. But as noted elsewhere, the
hook has to understand which cleanup mechanism is going to be used.
Which could get complicated.

It would be nice if we could just provide _both_ forms to the hook. It
looks like commit-msg just takes the filename as the first parameter.
Perhaps we could extend it by passing a second one? It does mean
sanitizing and writing out the message twice, even if the hook might not
look at it, but I doubt the overhead is all that high.

-Peff

  reply	other threads:[~2024-07-06  6:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05 20:12 Should commit-msg hook receive the washed message? Sean Allred
2024-07-05 21:35 ` Eric Sunshine
2024-07-06  6:37   ` Jeff King [this message]
  -- strict thread matches above, loose matches on Subject: below --
2024-07-05 22:07 brianmlyles

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20240706063737.GF700645@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=allred.sean@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

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