Re: [PATCH] Introduce a hook to run after formatting patches

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 17, 2014, 19:06 UTC
Message-ID: <xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqzjbryonp.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 27 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +post-format-patch
>> +~~~~~~~~~~~~
>> +
>> +This hook is called after format-patch created a patch and it is 
>> +invoked with the filename of the patch as the first parameter.
>
> Such an interface would not work well with --stdout mode, would it?
>
> And if this only works with output generated into the files, then
>
>     $ git format-patch $range | xargs -n1 $your_post_processing_script
>
> would do the same without any change to Git, I would imagine.
>
> So I would have to say that I am fairly negative on this change in
> the presented form.
>
> An alternative design to implement this as a post-processing filter
> to work for both "to individual files" and "to standard output
> stream" output filter may be possible, but even in that case I am
> not sure if it is worth the churn.
>
> In general I'd look at post-anything hook that works locally with a
> great suspicion, so that may partly be where my comment above is
> coming from.  I dunno.

Another reason, in addition to that this only works on the already created output files, why I find this particular design distasteful (I am not saying that there should be an easy way to drop cruft left by third-party systems such as "Change-id:" line) is because the mechanism the patch adds does not attempt to take advantage of being inside Git, so the "xargs -n1" above is strictly an equivalent. You have a chance to make the life better for users, but not you are not doing so.

The design of this feature could be made to allow the user to specify a filter to munge _ONLY_ the log message part. For example, just after logmsg_reencode() returns the proposed commit log message to msg in pretty.c::pretty_print_commit(), you can detect a request to use some external filter program and let the program munge the message. With such a design:

 * The external filter your users would write does not have to worry
   about limiting its damage only to the log message part, as it
   will never see the patch text part; and

 * The same mechanism would work just as well for --stdout mode.

The former is what I mean by "to take advantage of being inside". Incidentally, it falls into #2 of "5 valid reasons to admit a new hook" [*1*].

[Reference]

*1* http://thread.gmane.org/gmane.comp.version-control.git/232809/focus=71069