Junio, thanks for pointing out, why my patch doesn't make sense here. Do we have similar filters somewhere in place already, so I could have a look at the code architecture, the api, and how the user would operate that? The way you're proposing, doesn't sound as if a hook would be the right thing for such filtering. The one big thing I liked over the first patch in this thread was the 'maintainability', i.e. if it were a hook, I could set that up and forget about it. No need to change my behavior in using git, but still I have the desired postprocessed results. A command line option for such filtering is inconvenient because of having it type all the time and not just having it setup and forgetting about it. Sure we can have a command line option to specify such a filter, but I'd be more interested in having an option in maybe the git config file like [format-patch] external-commit-message-stream-processor = $(GIT_DIR)/../foo/bar.sh The command line option would rather only be used to override that config in non-default cases. Thanks for your thoughts, Stefan On Mon, Nov 17, 2014 at 11:06 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Stefan Beller <sbeller@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html