{"thread":{"id":"37972","subject":"[PATCH] Introduce a hook to run after formatting patches","startedAt":"2014-11-15T00:47:40Z","lastAt":"2014-11-21T04:31:57Z","messageCount":12,"participants":["Stefan Beller","Philip Oakley","Junio C Hamano","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"251927","messageId":"1416012460-4459-1-git-send-email-sbeller@google.com","threadId":"37972","inReplyTo":null,"subject":"[PATCH] Introduce a hook to run after formatting patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-15T00:47:40Z","receivedAt":"2014-11-15T00:47:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This comes in handy if you want to post-process formatted patches.\nOne examplary use case would be removing ChangeIds, which are used\nin Gerrit, a program sitting on top of Git, used for tracking\ndifferent versions of a patch.\n\nAnother use case would be checking if all your commits are signed off,\nor have another kind of property.\n\nSo in my case a hook like the following will help a lot.\n\n\t# Run with on formatted patches. The first argument is the filename to the patch.\n\tsed --in-place '/^Change-Id:/d' $1\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n Hi Git people,\n I haven't sent a patch for some time now, but I intend to change that\n soon, as I'll be overtaking the transactions series from Ronnie Sahlberg.\n\n The patch series I intend to overtake has been reviewed on this list\n as well as https://code-review.googlesource.com/#/q/project:git\n using Gerrit. Gerrit uses Change-Ids, which I want to reliably \n remove before sending them on the list. And for reliability \n you better trust a machine than a human like me.\n\n Documentation/githooks.txt |  9 +++++++++\n builtin/log.c              | 17 +++++++++++++----\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 9ef2469..b4f06a9 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -69,6 +69,15 @@ and is invoked after the patch is applied and a commit is made.\n This hook is meant primarily for notification, and cannot affect\n the outcome of 'git am'.\n \n+post-format-patch\n+~~~~~~~~~~~~\n+\n+This hook is called after format-patch created a patch and it is \n+invoked with the filename of the patch as the first parameter.\n+\n+This hook can be used to alter the created patch, such as removing\n+or adding Sign-Offs or similar information.\n+\n pre-commit\n ~~~~~~~~~~\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex 734aab3..863fcef 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -761,7 +761,8 @@ static const char *output_directory = NULL;\n static int outdir_offset;\n \n static int reopen_stdout(struct commit *commit, const char *subject,\n-\t\t\t struct rev_info *rev, int quiet)\n+\t\t\t struct rev_info *rev, int quiet,\n+\t\t\t struct strbuf *choosen_filename)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(rev->patch_suffix) + 1;\n@@ -788,6 +789,11 @@ static int reopen_stdout(struct commit *commit, const char *subject,\n \tif (freopen(filename.buf, \"w\", stdout) == NULL)\n \t\treturn error(_(\"Cannot open patch file %s\"), filename.buf);\n \n+\tif (choosen_filename) {\n+\t\tstrbuf_reset(choosen_filename);\n+\t\tstrbuf_addstr(choosen_filename, filename.buf);\n+\t}\n+\n \tstrbuf_release(&filename);\n \treturn 0;\n }\n@@ -921,7 +927,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tcommitter = git_committer_info(0);\n \n \tif (!use_stdout &&\n-\t    reopen_stdout(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet))\n+\t    reopen_stdout(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet, NULL))\n \t\treturn;\n \n \tlog_write_email_headers(rev, head, &pp.subject, &pp.after_subject,\n@@ -1176,6 +1182,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf filename = STRBUF_INIT;\n \tint use_patch_format = 0;\n \tint quiet = 0;\n \tint reroll_count = -1;\n@@ -1531,7 +1538,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (!use_stdout &&\n-\t\t    reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))\n+\t\t    reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet, &filename))\n \t\t\tdie(_(\"Failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree_commit_buffer(commit);\n@@ -1552,8 +1559,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\telse\n \t\t\t\tprint_signature();\n \t\t}\n-\t\tif (!use_stdout)\n+\t\tif (!use_stdout) {\n \t\t\tfclose(stdout);\n+\t\t\trun_hook_le(NULL, \"post-format-patch\", filename.buf, NULL);\n+\t\t}\n \t}\n \tfree(list);\n \tfree(branch_name);\n-- \n2.2.0.rc1.24.g562add4\n"},{"id":"251938","messageId":"B4E02729E7DE4F90BC6F1A065A7D4D5D@PhilipOakley","threadId":"37972","inReplyTo":"1416012460-4459-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-15T08:27:58Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Stefan Beller\" <sbeller@google.com>\n> This comes in handy if you want to post-process formatted patches.\n> One examplary use case would be removing ChangeIds, which are used\n> in Gerrit, a program sitting on top of Git, used for tracking\n> different versions of a patch.\n>\n> Another use case would be checking if all your commits are signed off,\n> or have another kind of property.\n>\n> So in my case a hook like the following will help a lot.\n>\n> # Run with on formatted patches. The first argument is the filename to \n> the patch.\n> sed --in-place '/^Change-Id:/d' $1\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n> Hi Git people,\n> I haven't sent a patch for some time now, but I intend to change that\n> soon, as I'll be overtaking the transactions series from Ronnie \n> Sahlberg.\n>\n> The patch series I intend to overtake has been reviewed on this list\n> as well as https://code-review.googlesource.com/#/q/project:git\n> using Gerrit. Gerrit uses Change-Ids, which I want to reliably\n> remove before sending them on the list. And for reliability\n> you better trust a machine than a human like me.\n>\n> Documentation/githooks.txt |  9 +++++++++\n> builtin/log.c              | 17 +++++++++++++----\n> 2 files changed, 22 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 9ef2469..b4f06a9 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -69,6 +69,15 @@ and is invoked after the patch is applied and a \n> commit is made.\n> This hook is meant primarily for notification, and cannot affect\n> the outcome of 'git am'.\n>\n> +post-format-patch\n> +~~~~~~~~~~~~\n> +\n> +This hook is called after format-patch created a patch and it is\n> +invoked with the filename of the patch as the first parameter.\n> +\n> +This hook can be used to alter the created patch, such as removing\n> +or adding Sign-Offs or similar information.\n\nsurely   s/adding/checking/  as described in the commit message.\nWe wouldn't want unthinking sign-offs being applied;-)\n\n> +\n> pre-commit\n> ~~~~~~~~~~\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 734aab3..863fcef 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -761,7 +761,8 @@ static const char *output_directory = NULL;\n> static int outdir_offset;\n>\n> static int reopen_stdout(struct commit *commit, const char *subject,\n> - struct rev_info *rev, int quiet)\n> + struct rev_info *rev, int quiet,\n> + struct strbuf *choosen_filename)\n> {\n>  struct strbuf filename = STRBUF_INIT;\n>  int suffix_len = strlen(rev->patch_suffix) + 1;\n> @@ -788,6 +789,11 @@ static int reopen_stdout(struct commit *commit, \n> const char *subject,\n>  if (freopen(filename.buf, \"w\", stdout) == NULL)\n>  return error(_(\"Cannot open patch file %s\"), filename.buf);\n>\n> + if (choosen_filename) {\n> + strbuf_reset(choosen_filename);\n> + strbuf_addstr(choosen_filename, filename.buf);\n> + }\n> +\n>  strbuf_release(&filename);\n>  return 0;\n> }\n> @@ -921,7 +927,7 @@ static void make_cover_letter(struct rev_info \n> *rev, int use_stdout,\n>  committer = git_committer_info(0);\n>\n>  if (!use_stdout &&\n> -     reopen_stdout(NULL, rev->numbered_files ? NULL : \"cover-letter\", \n> rev, quiet))\n> +     reopen_stdout(NULL, rev->numbered_files ? NULL : \"cover-letter\", \n> rev, quiet, NULL))\n>  return;\n>\n>  log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,\n> @@ -1176,6 +1182,7 @@ int cmd_format_patch(int argc, const char \n> **argv, const char *prefix)\n>  const char *in_reply_to = NULL;\n>  struct patch_ids ids;\n>  struct strbuf buf = STRBUF_INIT;\n> + struct strbuf filename = STRBUF_INIT;\n>  int use_patch_format = 0;\n>  int quiet = 0;\n>  int reroll_count = -1;\n> @@ -1531,7 +1538,7 @@ int cmd_format_patch(int argc, const char \n> **argv, const char *prefix)\n>  }\n>\n>  if (!use_stdout &&\n> -     reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, \n> quiet))\n> +     reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, \n> quiet, &filename))\n>  die(_(\"Failed to create output files\"));\n>  shown = log_tree_commit(&rev, commit);\n>  free_commit_buffer(commit);\n> @@ -1552,8 +1559,10 @@ int cmd_format_patch(int argc, const char \n> **argv, const char *prefix)\n>  else\n>  print_signature();\n>  }\n> - if (!use_stdout)\n> + if (!use_stdout) {\n>  fclose(stdout);\n> + run_hook_le(NULL, \"post-format-patch\", filename.buf, NULL);\n> + }\n>  }\n>  free(list);\n>  free(branch_name);\n> -- \n> 2.2.0.rc1.24.g562add4\n>\n> --\nPhilip \n"},{"id":"251965","messageId":"xmqqzjbryonp.fsf@gitster.dls.corp.google.com","threadId":"37972","inReplyTo":"1416012460-4459-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-16T18:40:42Z","receivedAt":"2014-11-16T18:40:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +post-format-patch\n> +~~~~~~~~~~~~\n> +\n> +This hook is called after format-patch created a patch and it is \n> +invoked with the filename of the patch as the first parameter.\n\nSuch an interface would not work well with --stdout mode, would it?\n\nAnd if this only works with output generated into the files, then\n\n    $ git format-patch $range | xargs -n1 $your_post_processing_script\n\nwould do the same without any change to Git, I would imagine.\n\nSo I would have to say that I am fairly negative on this change in\nthe presented form.\n\nAn alternative design to implement this as a post-processing filter\nto work for both \"to individual files\" and \"to standard output\nstream\" output filter may be possible, but even in that case I am\nnot sure if it is worth the churn.\n\nIn general I'd look at post-anything hook that works locally with a\ngreat suspicion, so that may partly be where my comment above is\ncoming from.  I dunno.\n"},{"id":"251995","messageId":"CAGZ79kZsK=hkSOCnR75eDYauOt-biCD6_v0eH3Ls_Krb_UAAvQ@mail.gmail.com","threadId":"37972","inReplyTo":"xmqqzjbryonp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-17T19:01:44Z","receivedAt":"2014-11-17T19:01:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Nov 16, 2014 at 10:40 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> +post-format-patch\n>> +~~~~~~~~~~~~\n>> +\n>> +This hook is called after format-patch created a patch and it is\n>> +invoked with the filename of the patch as the first parameter.\n>\n> Such an interface would not work well with --stdout mode, would it?\n>\n> And if this only works with output generated into the files, then\n>\n>     $ git format-patch $range | xargs -n1 $your_post_processing_script\n>\n> would do the same without any change to Git, I would imagine.\n\nOk I'll try to use these commands.\n\n>\n> So I would have to say that I am fairly negative on this change in\n> the presented form.\n\nOk, I definitely did not expect this patch to be accepted the way it is,\nbut rather was just proposing an idea. The post-format-patch.sample\nhook file would be missing for example.\n\n>\n> An alternative design to implement this as a post-processing filter\n> to work for both \"to individual files\" and \"to standard output\n> stream\" output filter may be possible, but even in that case I am\n> not sure if it is worth the churn.\n>\n> In general I'd look at post-anything hook that works locally with a\n> great suspicion, so that may partly be where my comment above is\n> coming from.  I dunno.\n>\n\nGit is very easy to use when importing from other VCS, because of all\nthe helpers like high level git-svn or low level fast-import.\n\nPatches on mailing lists can also be easily imported as a commit,\nso I think having an exporting system in place with various options\nlike custom post processing would help for some use cases.\n\nAnyway I'll just drop this patch and go with your suggestion.\n\nThanks,\nStefan\n"},{"id":"251996","messageId":"xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com","threadId":"37972","inReplyTo":"xmqqzjbryonp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-17T19:06:12Z","receivedAt":"2014-11-17T19:06:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> +post-format-patch\n>> +~~~~~~~~~~~~\n>> +\n>> +This hook is called after format-patch created a patch and it is \n>> +invoked with the filename of the patch as the first parameter.\n>\n> Such an interface would not work well with --stdout mode, would it?\n>\n> And if this only works with output generated into the files, then\n>\n>     $ git format-patch $range | xargs -n1 $your_post_processing_script\n>\n> would do the same without any change to Git, I would imagine.\n>\n> So I would have to say that I am fairly negative on this change in\n> the presented form.\n>\n> An alternative design to implement this as a post-processing filter\n> to work for both \"to individual files\" and \"to standard output\n> stream\" output filter may be possible, but even in that case I am\n> not sure if it is worth the churn.\n>\n> In general I'd look at post-anything hook that works locally with a\n> great suspicion, so that may partly be where my comment above is\n> coming from.  I dunno.\n\nAnother reason, in addition to that this only works on the already\ncreated output files, why I find this particular design distasteful\n(I am not saying that there should be an easy way to drop cruft left\nby third-party systems such as \"Change-id:\" line) is because the\nmechanism the patch adds does not attempt to take advantage of being\ninside Git, so the \"xargs -n1\" above is strictly an equivalent.  You\nhave a chance to make the life better for users, but not you are not\ndoing so.\n\nThe design of this feature could be made to allow the user to\nspecify a filter to munge _ONLY_ the log message part.  For example,\njust after logmsg_reencode() returns the proposed commit log message\nto msg in pretty.c::pretty_print_commit(), you can detect a request\nto use some external filter program and let the program munge the\nmessage.  With such a design:\n\n * The external filter your users would write does not have to worry\n   about limiting its damage only to the log message part, as it\n   will never see the patch text part; and\n\n * The same mechanism would work just as well for --stdout mode.\n\nThe former is what I mean by \"to take advantage of being inside\".\nIncidentally, it falls into #2 of \"5 valid reasons to admit a new\nhook\" [*1*].\n\n\n[Reference]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/232809/focus=71069\n"},{"id":"251998","messageId":"xmqqd28ly6p1.fsf@gitster.dls.corp.google.com","threadId":"37972","inReplyTo":"xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-17T19:20:58Z","receivedAt":"2014-11-17T19:20:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> (I am not saying that there should be an easy way to drop cruft left\n> by third-party systems such as \"Change-id:\" line) ...\n\nHeh, that was \"should not be\", but I guess it was probably obvious.\n\nSorry for the noise.\n"},{"id":"252064","messageId":"CAGZ79kZ=1qekoKfY5cwdmDp68oU-poTcc6A4Fa7u5LhwCB5HyQ@mail.gmail.com","threadId":"37972","inReplyTo":"xmqqlhn9y7dn.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-18T02:30:00Z","receivedAt":"2014-11-18T02:30:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Junio,\n\nthanks for pointing out, why my patch doesn't make sense here.\n\nDo we have similar filters somewhere in place already,\nso I could have a look at the code architecture,\nthe api, and how the user would operate that?\n\nThe way you're proposing, doesn't sound as if a hook would be the right\nthing for such filtering.\n\nThe one big thing I liked over the first patch in this thread was the\n'maintainability',\ni.e. if it were a hook, I could set that up and forget about it.\nNo need to change my behavior in using git, but still I have the\ndesired postprocessed\nresults.\nA command line option for such filtering is inconvenient because of\nhaving it type\nall the time and not just having it setup and forgetting about it.\nSure we can have a command line option to specify such a filter,\nbut I'd be more interested in having an option in maybe the git config file like\n\n[format-patch]\n  external-commit-message-stream-processor = $(GIT_DIR)/../foo/bar.sh\n\nThe command line option would rather only be used to override that\nconfig in non-default cases.\n\nThanks for your thoughts,\nStefan\n\n\n\n\n\n\n\n\n\n\n\nOn Mon, Nov 17, 2014 at 11:06 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> +post-format-patch\n>>> +~~~~~~~~~~~~\n>>> +\n>>> +This hook is called after format-patch created a patch and it is\n>>> +invoked with the filename of the patch as the first parameter.\n>>\n>> Such an interface would not work well with --stdout mode, would it?\n>>\n>> And if this only works with output generated into the files, then\n>>\n>>     $ git format-patch $range | xargs -n1 $your_post_processing_script\n>>\n>> would do the same without any change to Git, I would imagine.\n>>\n>> So I would have to say that I am fairly negative on this change in\n>> the presented form.\n>>\n>> An alternative design to implement this as a post-processing filter\n>> to work for both \"to individual files\" and \"to standard output\n>> stream\" output filter may be possible, but even in that case I am\n>> not sure if it is worth the churn.\n>>\n>> In general I'd look at post-anything hook that works locally with a\n>> great suspicion, so that may partly be where my comment above is\n>> coming from.  I dunno.\n>\n> Another reason, in addition to that this only works on the already\n> created output files, why I find this particular design distasteful\n> (I am not saying that there should be an easy way to drop cruft left\n> by third-party systems such as \"Change-id:\" line) is because the\n> mechanism the patch adds does not attempt to take advantage of being\n> inside Git, so the \"xargs -n1\" above is strictly an equivalent.  You\n> have a chance to make the life better for users, but not you are not\n> doing so.\n>\n> The design of this feature could be made to allow the user to\n> specify a filter to munge _ONLY_ the log message part.  For example,\n> just after logmsg_reencode() returns the proposed commit log message\n> to msg in pretty.c::pretty_print_commit(), you can detect a request\n> to use some external filter program and let the program munge the\n> message.  With such a design:\n>\n>  * The external filter your users would write does not have to worry\n>    about limiting its damage only to the log message part, as it\n>    will never see the patch text part; and\n>\n>  * The same mechanism would work just as well for --stdout mode.\n>\n> The former is what I mean by \"to take advantage of being inside\".\n> Incidentally, it falls into #2 of \"5 valid reasons to admit a new\n> hook\" [*1*].\n>\n>\n> [Reference]\n>\n> *1* http://thread.gmane.org/gmane.comp.version-control.git/232809/focus=71069\n"},{"id":"252078","messageId":"CAP8UFD3m9aouYKXfk-vE4AGbTxdyH6=k8ey5n-kF1OLux2Ah0g@mail.gmail.com","threadId":"37972","inReplyTo":"xmqqd28ly6p1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2014-11-18T06:40:07Z","receivedAt":"2014-11-18T06:40:07Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Nov 17, 2014 at 8:20 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> (I am not saying that there should be an easy way to drop cruft left\n>> by third-party systems such as \"Change-id:\" line) ...\n>\n> Heh, that was \"should not be\", but I guess it was probably obvious.\n>\n> Sorry for the noise.\n\nI am not sure it is very easy yet but as Change-id: ... line are\ntrailers, you can do that with git interpret-trailers.\n\nFor example:\n\n$ echo -e \"\\nChange-id: stuff\\nOther: thing\"  | git -c\ntrailer.Change-id.ifexists=replace interpret-trailers --trim-empty\n--trailer Change-id=\n>\n> Other: thing\n\nThe idea is that the above command replaces an existing \"Change-id:\nstuff\" trailer with an empty \"Change-id:\" trailer and then removes all\nthe empty trailers.\n\nBest,\nChristian.\n"},{"id":"252086","messageId":"xmqqoas4wif0.fsf@gitster.dls.corp.google.com","threadId":"37972","inReplyTo":"CAGZ79kZ=1qekoKfY5cwdmDp68oU-poTcc6A4Fa7u5LhwCB5HyQ@mail.gmail.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-18T17:02:59Z","receivedAt":"2014-11-18T17:02:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Do we have similar filters somewhere in place already,\n> so I could have a look at the code architecture,\n> the api, and how the user would operate that?\n\nThe clean/smudge filters interacts with the payload data and the end\nuser configuration in a similar way, I would say.\n\n> The way you're proposing, doesn't sound as if a hook would be the right\n> thing for such filtering.\n\nThat depends on how you define \"what a hook is\", I think.\n\n> The one big thing I liked over the first patch in this thread was\n> the 'maintainability', i.e. if it were a hook, I could set that up\n> and forget about it.  No need to change my behavior in using git,\n> but still I have the desired postprocessed results.\n\nIn the message you are responding, I said \"detect a request to use\",\nexactly because I wanted to leave it up to you to design what\nform(s) that \"request detection\" takes.  One of the forms could be\n\"a script with this $filename exists in $GIT_DIR/\", and the $filname\nmay be \"hooks/format-patch-redaction-filter\".\n\nOf course, any \"configured into repository\" solution must have a\ncommand line override, so the order you would develop this would be:\n\n (0) make the code that interracts with the filter if given by the\n     user work, without worrying about how the user specifies the\n     filter.\n\n (1) add a --command-line-option=<filter-name> to trigger the code\n     you wrote in (0) above.\n\n (2) add a --no-command-line-option to defeat the configured filter,\n     without worrying about how the user configures.  For your new\n     command line option --frotz, \"git cmd --frotz --no-frotz\"\n     should make your cmd refrain from doing frotz.\n\n (3) add configuration variable to point at a filter script,\n     e.g. \"format.redactionFilter\"; you must make sure that this is\n     disabled with \"--no-*\" you added in (2) above;\n\n (4) perhaps add support for \"hooks/format-patch-redaction-filter\",\n     if you strongly feel like it.  The user must be able to disable\n     this with the same \"--no-*\" added in (2).\n\nI'd say (4) is optional; by the time you reach (3), you already have\nthe same \"write once and forget\" capability.\n"},{"id":"293743","messageId":"20141120232628.GA30554@google.com","threadId":"37972","inReplyTo":"CAP8UFD3m9aouYKXfk-vE4AGbTxdyH6=k8ey5n-kF1OLux2Ah0g@mail.gmail.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2014-11-20T23:26:28Z","receivedAt":"2014-11-20T23:26:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Nov 18, 2014 at 07:40:07AM +0100, Christian Couder wrote:\n> On Mon, Nov 17, 2014 at 8:20 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Junio C Hamano <gitster@pobox.com> writes:\n> >\n> >> (I am not saying that there should be an easy way to drop cruft left\n> >> by third-party systems such as \"Change-id:\" line) ...\n> >\n> > Heh, that was \"should not be\", but I guess it was probably obvious.\n> >\n> > Sorry for the noise.\n> \n> I am not sure it is very easy yet but as Change-id: ... line are\n> trailers, you can do that with git interpret-trailers.\n> \n> For example:\n> \n> $ echo -e \"\\nChange-id: stuff\\nOther: thing\"  | git -c\n> trailer.Change-id.ifexists=replace interpret-trailers --trim-empty\n> --trailer Change-id=\n> >\n> > Other: thing\n> \n> The idea is that the above command replaces an existing \"Change-id:\n> stuff\" trailer with an empty \"Change-id:\" trailer and then removes all\n> the empty trailers.\n> \n\nSo I have read the man page on the trailers and it seems like the solution\nto my problem in removing parts from the commit message.\nHowever I did not find out, if it can be run automatically, whenever\ncalling format-patch\n\nMaybe all that is missing here is an option\n\n\tgit config format.enable_trailers \n?\n\nBest,\nStefan\n\n> Best,\n"},{"id":"252309","messageId":"xmqqioi91m7t.fsf@gitster.dls.corp.google.com","threadId":"37972","inReplyTo":"20141120232628.GA30554@google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-20T23:33:26Z","receivedAt":"2014-11-20T23:33:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> So I have read the man page on the trailers and it seems like the solution\n> to my problem in removing parts from the commit message.\n> However I did not find out, if it can be run automatically, whenever\n> calling format-patch\n>\n> Maybe all that is missing here is an option\n>\n> \tgit config format.enable_trailers \n> ?\n\nThe idea has been to first give a standalone text transmonger as a\nfilter for let people to try out, so that we know what kind of\nchanges are useful (e.g. \"insert s-o-b at the very end\") and make\nsure the configuration language to specify the changes is easy and\nexpressive enough, which is more or less what we have in 'master'.\n\nOnce we gain experience (and that may result in updates to what is\nin 'master'), in the second phase, we would figure out what code\npaths can make use of this text transmonger (e.g. your configuration\nvariable \"format.trailers\" to affect the format-patch code path) and\nintegrate it more tightly to the codebase.\n\nWe are not there yet.\n"},{"id":"252320","messageId":"CAP8UFD3kaFNoPdGVFwfBVZasdUJSn=04bmWoXbmRva0aiA-kyw@mail.gmail.com","threadId":"37972","inReplyTo":"xmqqioi91m7t.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Introduce a hook to run after formatting patches","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2014-11-21T04:31:57Z","receivedAt":"2014-11-21T04:31:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Nov 21, 2014 at 12:33 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> So I have read the man page on the trailers and it seems like the solution\n>> to my problem in removing parts from the commit message.\n>> However I did not find out, if it can be run automatically, whenever\n>> calling format-patch\n>>\n>> Maybe all that is missing here is an option\n>>\n>>       git config format.enable_trailers\n>> ?\n\nYeah, we could use config variables or command options or both to make\nit easier to enable it and pass it arguments.\n\nFor example we could add:\n\n1) an \"--interpret-trailers\" option to \"git commit\", \"git\nformat-patch\", \"git am\" and maybe others too, so that \"git\ninterpret-trailers\" is called (without arguments),\n2) an \"--trailer <trailer>\" option (that can be repeated) to the same\ncommands, so that \"git interpret-trailers\" is called with the\n\"--trailer <trailer>\" arguments,\n3) a \"trailer.enable_commands =  'format-patch, commit'\" config\nvariable, so that \"git interpret-trailers\" is called (without\narguments) every time one of the specified command is used,\n4) your suggested \"format.enable_trailers\" and maybe\n\"commit.enable_trailers\" and others like that,\n5) a \"trailer\" command for rebase -i todo lists.\n\nMy preference would be for 1), 2), 3) and 5).\n\n> The idea has been to first give a standalone text transmonger as a\n> filter for let people to try out, so that we know what kind of\n> changes are useful (e.g. \"insert s-o-b at the very end\") and make\n> sure the configuration language to specify the changes is easy and\n> expressive enough, which is more or less what we have in 'master'.\n>\n> Once we gain experience (and that may result in updates to what is\n> in 'master'), in the second phase, we would figure out what code\n> paths can make use of this text transmonger (e.g. your configuration\n> variable \"format.trailers\" to affect the format-patch code path) and\n> integrate it more tightly to the codebase.\n>\n> We are not there yet.\n\nYeah, \"interpret-trailers\" will be in Git 2.2 without the above\nsuggested features, but maybe, if we can get  some feedback from users\nabout which features they would use, we can aim to have some in Git\n2.3.\n\nThanks,\nChristian.\n"}]}