{"thread":{"id":"9163","subject":"[PATCH] Teach git-commit about commit message templates.","startedAt":"2007-07-23T04:17:42Z","lastAt":"2007-07-25T07:39:58Z","messageCount":11,"participants":["Steven Grimm","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"48259","messageId":"20070723041741.GA22461@midwinter.com","threadId":"9163","inReplyTo":null,"subject":"[PATCH] Teach git-commit about commit message templates.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-07-23T04:17:42Z","receivedAt":"2007-07-23T04:17:42Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"These are useful in organizations that enforce particular formats\nfor commit messages, e.g., to specify bug IDs or test plans.\nUse of the template is not enforced; it is simply used as the\ninitial content when the editor is invoked.\n\nSigned-off-by: Steven Grimm <koreth@midwinter.com>\n---\n\tThis came up at my company when someone was comparing git to\n\tsvn; we use svn's commit template feature.\n\n\tI know we can abuse the pre-commit hook and stick a template in\n\tthe SQUASH_MSG file (thanks to Dscho for that idea on IRC) but\n\tthat has the annoying side effect of using the template as the\n\tcommit message if you change your mind and quit the editor\n\twithout entering anything, rather than aborting the commit.\n\n\tI also know that git-commit is being builtin-ified, but as far\n\tas I know that's still a WIP and this will work today.\n\n\tThis isn't perfect -- what you'd ideally want is a way to avoid\n\tusing the template for purely local \"checkpoint of work in\n\tprogress\" commits while still using it for commits you intend to\n\tpublish. But obviously there's no way for git to know which is\n\twhich. For local commits you can easily just empty out the\n\teditor and type a message from scratch.\n\n\tI was surprised there wasn't already a test script for commit,\n\tbut I guess it gets tested implicitly in every other script\n\tanyway.\n\n Documentation/git-commit.txt |    8 ++++\n git-commit.sh                |   54 +++++++++++++++++++++--\n t/t7500-commit.sh            |   96 ++++++++++++++++++++++++++++++++++++++++++\n t/t7500/add-comments         |    4 ++\n t/t7500/add-content          |    3 +\n t/t7500/add-signed-off       |    3 +\n 6 files changed, 163 insertions(+), 5 deletions(-)\n create mode 100755 t/t7500-commit.sh\n create mode 100755 t/t7500/add-comments\n create mode 100755 t/t7500/add-content\n create mode 100755 t/t7500/add-signed-off\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 8e0e7e2..3f36c67 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -74,6 +74,14 @@ OPTIONS\n -m <msg>|--message=<msg>::\n \tUse the given <msg> as the commit message.\n \n+-t <file>|--template=<file>::\n+\tUse the contents of the given file as the initial version\n+\tof the commit message. The editor is invoked and you can\n+\tmake subsequent changes. If a message is specified using\n+\tthe `-m` or `-F` options, this option has no effect. The\n+\ttemplate file may also be specified using the `commit.template`\n+\tconfiguration variable.\n+\n -s|--signoff::\n \tAdd Signed-off-by line at the end of the commit message.\n \ndiff --git a/git-commit.sh b/git-commit.sh\nindex 92749df..4290ae2 100755\n--- a/git-commit.sh\n+++ b/git-commit.sh\n@@ -3,7 +3,7 @@\n # Copyright (c) 2005 Linus Torvalds\n # Copyright (c) 2006 Junio C Hamano\n \n-USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [[-i | -o] <path>...]'\n+USAGE='[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]'\n SUBDIRECTORY_OK=Yes\n . git-sh-setup\n require_work_tree\n@@ -87,6 +87,7 @@ signoff=\n force_author=\n only_include_assumed=\n untracked_files=\n+templatefile=\"`git config commit.template`\"\n while case \"$#\" in 0) break;; esac\n do\n \tcase \"$1\" in\n@@ -248,6 +249,13 @@ $1\"\n \t\tsignoff=t\n \t\tshift\n \t\t;;\n+\t-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)\n+\t\tcase \"$#\" in 1) usage ;; esac\n+\t\tshift\n+\t\ttemplatefile=\"$1\"\n+\t\tno_edit=\n+\t\tshift\n+\t\t;;\n \t-q|--q|--qu|--qui|--quie|--quiet)\n \t\tquiet=t\n \t\tshift\n@@ -321,6 +329,14 @@ t,,[1-9]*)\n \tdie \"No paths with -i does not make sense.\" ;;\n esac\n \n+if test ! -z \"$templatefile\" -a -z \"$log_given\"\n+then\n+\tif test ! -f \"$templatefile\"\n+\tthen\n+\t\tdie \"Commit template file does not exist.\"\n+\tfi\n+fi\n+\n ################################################################\n # Prepare index to have a tree to be committed\n \n@@ -454,6 +470,9 @@ then\n elif test -f \"$GIT_DIR/SQUASH_MSG\"\n then\n \tcat \"$GIT_DIR/SQUASH_MSG\"\n+elif test \"$templatefile\" != \"\"\n+then\n+\tcat \"$templatefile\"\n fi | git stripspace >\"$GIT_DIR\"/COMMIT_EDITMSG\n \n case \"$signoff\" in\n@@ -572,10 +591,35 @@ else\n fi |\n git stripspace >\"$GIT_DIR\"/COMMIT_MSG\n \n-if cnt=`grep -v -i '^Signed-off-by' \"$GIT_DIR\"/COMMIT_MSG |\n-\tgit stripspace |\n-\twc -l` &&\n-   test 0 -lt $cnt\n+# Test whether the commit message has any content we didn't supply.\n+have_commitmsg=\n+grep -v -i '^Signed-off-by' \"$GIT_DIR\"/COMMIT_MSG |\n+\tgit stripspace > \"$GIT_DIR\"/COMMIT_BAREMSG\n+\n+# Is the commit message totally empty?\n+if test -s \"$GIT_DIR\"/COMMIT_BAREMSG\n+then\n+\tif test \"$templatefile\" != \"\"\n+\tthen\n+\t\t# Test whether this is just the unaltered template.\n+\t\tif cnt=`sed -e '/^#/d' < \"$templatefile\" |\n+\t\t\tgit stripspace |\n+\t\t\tdiff \"$GIT_DIR\"/COMMIT_BAREMSG - |\n+\t\t\twc -l` &&\n+\t\t   test 0 -lt $cnt\n+\t\tthen\n+\t\t\thave_commitmsg=t\n+\t\tfi\n+\telse\n+\t\t# No template, so the content in the commit message must\n+\t\t# have come from the user.\n+\t\thave_commitmsg=t\n+\tfi\n+fi\n+\n+rm -f \"$GIT_DIR\"/COMMIT_BAREMSG\n+\n+if test \"$have_commitmsg\" = \"t\"\n then\n \tif test -z \"$TMP_INDEX\"\n \tthen\ndiff --git a/t/t7500-commit.sh b/t/t7500-commit.sh\nnew file mode 100755\nindex 0000000..f11ada8\n--- /dev/null\n+++ b/t/t7500-commit.sh\n@@ -0,0 +1,96 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Steven Grimm\n+#\n+\n+test_description='git-commit\n+\n+Tests for selected commit options.'\n+\n+. ./test-lib.sh\n+\n+commit_msg_is () {\n+\ttest \"`git log --pretty=format:%s%b -1`\" = \"$1\"\n+}\n+\n+# A sanity check to see if commit is working at all.\n+test_expect_success 'a basic commit in an empty tree should succeed' '\n+\techo content > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"initial commit\"\n+'\n+\n+test_expect_success 'nonexistent template file should return error' '\n+\techo changes >> foo &&\n+\tgit add foo &&\n+\t! git commit --template \"$PWD\"/notexist\n+'\n+\n+test_expect_success 'nonexistent template file in config should return error' '\n+\tgit config commit.template \"$PWD\"/notexist &&\n+\t! git commit &&\n+\tgit config --unset commit.template\n+'\n+\n+# From now on we'll use a template file that exists.\n+TEMPLATE=\"$PWD\"/template\n+\n+test_expect_success 'unedited template should not commit' '\n+\techo \"template line\" > \"$TEMPLATE\" &&\n+\t! git commit --template \"$TEMPLATE\"\n+'\n+\n+test_expect_success 'unedited template with comments should not commit' '\n+\techo \"# comment in template\" >> \"$TEMPLATE\" &&\n+\t! git commit --template \"$TEMPLATE\"\n+'\n+\n+test_expect_success 'a Signed-off-by line by itself should not commit' '\n+\t! GIT_EDITOR=../t7500/add-signed-off git commit --template \"$TEMPLATE\"\n+'\n+\n+test_expect_success 'adding comments to a template should not commit' '\n+\t! GIT_EDITOR=../t7500/add-comments git commit --template \"$TEMPLATE\"\n+'\n+\n+test_expect_success 'adding real content to a template should commit' '\n+\tGIT_EDITOR=../t7500/add-content git commit --template \"$TEMPLATE\" &&\n+\tcommit_msg_is \"template linecommit message\"\n+'\n+\n+test_expect_success '-t option should be short for --template' '\n+\techo \"short template\" > \"$TEMPLATE\" &&\n+\techo \"new content\" >> foo &&\n+\tgit add foo &&\n+\tGIT_EDITOR=../t7500/add-content git commit -t \"$TEMPLATE\" &&\n+\tcommit_msg_is \"short templatecommit message\"\n+'\n+\n+test_expect_success 'config-specified template should commit' '\n+\techo \"new template\" > \"$TEMPLATE\" &&\n+\tgit config commit.template \"$TEMPLATE\" &&\n+\techo \"more content\" >> foo &&\n+\tgit add foo &&\n+\tGIT_EDITOR=../t7500/add-content git commit &&\n+\tgit config --unset commit.template &&\n+\tcommit_msg_is \"new templatecommit message\"\n+'\n+\n+test_expect_success 'explicit commit message should override template' '\n+\techo \"still more content\" >> foo &&\n+\tgit add foo &&\n+\tGIT_EDITOR=../t7500/add-content git commit --template \"$TEMPLATE\" \\\n+\t\t-m \"command line msg\" &&\n+\tcommit_msg_is \"command line msg<unknown>\"\n+'\n+\n+test_expect_success 'commit message from file should override template' '\n+\techo \"content galore\" >> foo &&\n+\tgit add foo &&\n+\techo \"standard input msg\" |\n+\t\tGIT_EDITOR=../t7500/add-content git commit \\\n+\t\t\t--template \"$TEMPLATE\" --file - &&\n+\tcommit_msg_is \"standard input msg<unknown>\"\n+'\n+\n+test_done\ndiff --git a/t/t7500/add-comments b/t/t7500/add-comments\nnew file mode 100755\nindex 0000000..a72e65c\n--- /dev/null\n+++ b/t/t7500/add-comments\n@@ -0,0 +1,4 @@\n+#!/bin/sh\n+echo \"# this is a new comment\" >> \"$1\"\n+echo \"# and so is this\" >> \"$1\"\n+exit 0\ndiff --git a/t/t7500/add-content b/t/t7500/add-content\nnew file mode 100755\nindex 0000000..2fa3d86\n--- /dev/null\n+++ b/t/t7500/add-content\n@@ -0,0 +1,3 @@\n+#!/bin/sh\n+echo \"commit message\" >> \"$1\"\n+exit 0\ndiff --git a/t/t7500/add-signed-off b/t/t7500/add-signed-off\nnew file mode 100755\nindex 0000000..e1d856a\n--- /dev/null\n+++ b/t/t7500/add-signed-off\n@@ -0,0 +1,3 @@\n+#!/bin/sh\n+echo \"Signed-off-by: foo <bar@frotz>\" >> \"$1\"\n+exit 0\n-- \n1.5.3.rc2.4.g726f9\n"},{"id":"48292","messageId":"Pine.LNX.4.64.0707231059490.14781@racer.site","threadId":"9163","inReplyTo":"20070723041741.GA22461@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-23T10:04:04Z","receivedAt":"2007-07-23T10:04:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 22 Jul 2007, Steven Grimm wrote:\n\n> @@ -572,10 +591,35 @@ else\n>  fi |\n>  git stripspace >\"$GIT_DIR\"/COMMIT_MSG\n>  \n> -if cnt=`grep -v -i '^Signed-off-by' \"$GIT_DIR\"/COMMIT_MSG |\n> -\tgit stripspace |\n> -\twc -l` &&\n> -   test 0 -lt $cnt\n> +# Test whether the commit message has any content we didn't supply.\n> +have_commitmsg=\n> +grep -v -i '^Signed-off-by' \"$GIT_DIR\"/COMMIT_MSG |\n> +\tgit stripspace > \"$GIT_DIR\"/COMMIT_BAREMSG\n\nUp until here, I was with you.  But this feels very wrong.\n\nWhy not compare COMMIT_MSG to the templatefile, if there is one?  I.e.\n\ntest ! -z \"$templatefile\" && cmp \"$GIT_DIR\"/COMMIT_MSG \"$templatefile\" &&\n\tdie \"Unchanged message; will not commit\"\n\nHmm?\n\nCiao,\nDscho\n"},{"id":"48294","messageId":"46A481B4.7000502@midwinter.com","threadId":"9163","inReplyTo":"Pine.LNX.4.64.0707231059490.14781@racer.site","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-07-23T10:23:48Z","receivedAt":"2007-07-23T10:23:48Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Up until here, I was with you.  But this feels very wrong.\n>\n> Why not compare COMMIT_MSG to the templatefile, if there is one?  I.e.\n>\n> test ! -z \"$templatefile\" && cmp \"$GIT_DIR\"/COMMIT_MSG \"$templatefile\" &&\n> \tdie \"Unchanged message; will not commit\"\n>   \n\nThe template can itself have comments -- instructions or explanations of \nfields to fill in, for example -- and since comments have been stripped \nfrom COMMIT_MSG at this point, a comparison against such a template \nwould always fail. And, consistent with the current behavior, simply \nadding a Signed-off-by: line shouldn't count as supplying a commit message.\n\nI could do this test before stripping comments from COMMIT_MSG, but then \nI'd still fail the comparison if the user just deleted some comment \nlines manually, which also seems wrong to me -- the comments should be \ntotally ignored when doing this comparison, IMO. Plus that wouldn't \nignore Signed-off-by: lines.\n\nIf I'm coming at the design the wrong way, I'm of course happy to adjust \nit, but insensitivity to both comments and Signed-off-by: lines seemed \nlike the right behavior from the user's POV to me, and I didn't see a \ncleaner way to do it.\n\nThanks for looking at the patch!\n\n-Steve\n"},{"id":"48296","messageId":"Pine.LNX.4.64.0707231136530.14781@racer.site","threadId":"9163","inReplyTo":"46A481B4.7000502@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-23T10:40:33Z","receivedAt":"2007-07-23T10:40:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 23 Jul 2007, Steven Grimm wrote:\n\n> Johannes Schindelin wrote:\n> > Up until here, I was with you.  But this feels very wrong.\n> > \n> > Why not compare COMMIT_MSG to the templatefile, if there is one?  I.e.\n> > \n> > test ! -z \"$templatefile\" && cmp \"$GIT_DIR\"/COMMIT_MSG \"$templatefile\" &&\n> > \tdie \"Unchanged message; will not commit\"\n> >   \n> \n> The template can itself have comments -- instructions or explanations of\n> fields to fill in, for example -- and since comments have been stripped \n> from COMMIT_MSG at this point, a comparison against such a template \n> would always fail.\n\nAh, I missed that.  But IIRC your patch does not wrap that logic behind \ntest ! -z \"$templatefile\", right?  So this is my modified suggestion:\n\ntest ! -z \"$templatefile\" && {\n\tgrep -vie '^Signed-off-by:' < \"$GIT_DIR\"/COMMIT_MSG\" > \"$GIT_DIR\"/tmp1\n\tgrep -ve '^#' < \"$templatefile\" > \"$GIT_DIR\"/tmp1\n\ttrap 'rm \"$GIT_DIR\"/tmp[12]' 0\n\tcmp \"$GIT_DIR\"/tmp[12] &&\n\tdie \"Unchanged message; will not commit\"\n}\n\nCiao,\nDscho\n"},{"id":"48299","messageId":"46A48949.1020501@midwinter.com","threadId":"9163","inReplyTo":"Pine.LNX.4.64.0707231136530.14781@racer.site","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-07-23T10:56:09Z","receivedAt":"2007-07-23T10:56:09Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Ah, I missed that.  But IIRC your patch does not wrap that logic behind \n> test ! -z \"$templatefile\", right?  So this is my modified suggestion:\n>\n> test ! -z \"$templatefile\" && {\n> \tgrep -vie '^Signed-off-by:' < \"$GIT_DIR\"/COMMIT_MSG\" > \"$GIT_DIR\"/tmp1\n> \tgrep -ve '^#' < \"$templatefile\" > \"$GIT_DIR\"/tmp1\n> \ttrap 'rm \"$GIT_DIR\"/tmp[12]' 0\n> \tcmp \"$GIT_DIR\"/tmp[12] &&\n> \tdie \"Unchanged message; will not commit\"\n> }\n>   \n\nSo you are suggesting I do this in addition to the existing git-commit \nstripping of Signed-off-by: lines? I can certainly do that, but I didn't \nwant to make two passes over the commit message doing exactly the same \nstripping.\n\nHmm, maybe I should outline my understanding of the current (unpatched) \nbehavior and what I want it to do. Currently:\n\n* Strip off all comment lines (happens when COMMIT_MSG is created)\n* Strip off all Signed-off-by: lines\n* Trim whitespace\n* If the result has no content (`wc -l` == 0), abort.\n\nWith the patch, my intent was:\n\n* Strip off all comment lines\n* Strip off all Signed-off-by: lines\n* Trim whitespace\n* If the result has no content (! -s file), abort.\n* If a template file was specified:\n   * Strip off all comment and Signed-off-by: lines from the template\n   * Trim whitespace from the template\n   * If the resulting trimmed template is the same as the trimmed commit \nmessage, abort.\n\nSo I guess before getting to the specifics of the code, I'll ask: does \nthe above make sense as a design? I wanted to preserve the existing \nbehavior in the absence of a template.\n\nSince the existing code is already stripping Signed-off-by: lines to \ntest for a zero-length commit message, I figured I should reuse that \nwork. However, it's no big deal to do it twice if people feel that \nresults in more readable code -- certainly no human will ever notice the \ntime it takes to re-grep the commit message.\n\n-Steve\n"},{"id":"48303","messageId":"Pine.LNX.4.64.0707231239460.14781@racer.site","threadId":"9163","inReplyTo":"46A48949.1020501@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-23T11:58:02Z","receivedAt":"2007-07-23T11:58:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 23 Jul 2007, Steven Grimm wrote:\n\n> Johannes Schindelin wrote:\n> > Ah, I missed that.  But IIRC your patch does not wrap that logic behind test\n> > ! -z \"$templatefile\", right?  So this is my modified suggestion:\n> > \n> > test ! -z \"$templatefile\" && {\n> > \tgrep -vie '^Signed-off-by:' < \"$GIT_DIR\"/COMMIT_MSG\" > \"$GIT_DIR\"/tmp1\n> > \tgrep -ve '^#' < \"$templatefile\" > \"$GIT_DIR\"/tmp1\n> > \ttrap 'rm \"$GIT_DIR\"/tmp[12]' 0\n> > \tcmp \"$GIT_DIR\"/tmp[12] &&\n> > \tdie \"Unchanged message; will not commit\"\n> > }\n> >   \n> \n> So you are suggesting I do this in addition to the existing git-commit\n> stripping of Signed-off-by: lines? I can certainly do that, but I didn't want\n> to make two passes over the commit message doing exactly the same stripping.\n> \n> Hmm, maybe I should outline my understanding of the current (unpatched)\n> behavior and what I want it to do. Currently:\n> \n> * Strip off all comment lines (happens when COMMIT_MSG is created)\n> * Strip off all Signed-off-by: lines\n> * Trim whitespace\n> * If the result has no content (`wc -l` == 0), abort.\n> \n> With the patch, my intent was:\n> \n> * Strip off all comment lines\n> * Strip off all Signed-off-by: lines\n> * Trim whitespace\n> * If the result has no content (! -s file), abort.\n> * If a template file was specified:\n>   * Strip off all comment and Signed-off-by: lines from the template\n>   * Trim whitespace from the template\n>   * If the resulting trimmed template is the same as the trimmed commit\n> message, abort.\n> \n> So I guess before getting to the specifics of the code, I'll ask: does the\n> above make sense as a design? I wanted to preserve the existing behavior in\n> the absence of a template.\n\nYes, I think that makes sense.\n\n> Since the existing code is already stripping Signed-off-by: lines to test for\n> a zero-length commit message, I figured I should reuse that work. However,\n> it's no big deal to do it twice if people feel that results in more readable\n> code -- certainly no human will ever notice the time it takes to re-grep the\n> commit message.\n\nOkay, but I was really confused by the big if thing.\n\nI'd make sure that COMMIT_BAREMSG is removed with a trap, and really try \nto just enhance the\n\n\tif cnt=`grep -v -i '^Signed-off-by' \"$GIT_DIR\"/COMMIT_MSG |\n\t        git stripspace |\n\t        wc -l` &&\n\t   test 0 -lt $cnt\n\nso that it reads something like\n\n\tif test -s \"$GIT_DIR\"/COMMIT_BAREMSG &&\n\t\ttest -z \"$templatefile\" ||\n\t\t(grep -ve '^#' < \"$templatefile\" |\n\t\t git stripspace |\n\t\t git diff --quiet - \"$GIT_DIR\"/COMMIT_BAREMSG)\n\tthen\n\t\t...\n\n(Totally untested, of course.) Hmm?\n\nCiao,\nDscho\n"},{"id":"48318","messageId":"7vfy3fkpr8.fsf@assigned-by-dhcp.cox.net","threadId":"9163","inReplyTo":"46A48949.1020501@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-23T17:12:59Z","receivedAt":"2007-07-23T17:12:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm <koreth@midwinter.com> writes:\n\n> With the patch, my intent was:\n>\n> * Strip off all comment lines\n> * Strip off all Signed-off-by: lines\n> * Trim whitespace\n> * If the result has no content (! -s file), abort.\n> * If a template file was specified:\n>   * Strip off all comment and Signed-off-by: lines from the template\n>   * Trim whitespace from the template\n>   * If the resulting trimmed template is the same as the trimmed\n> commit message, abort.\n>\n> So I guess before getting to the specifics of the code, I'll ask: does\n> the above make sense as a design? I wanted to preserve the existing\n> behavior in the absence of a template.\n\nOffhand, an \"interesting\" side effect of the above I can see is\nthat you will cry \"wolf\" if the only thing the user did is to\nadd his own Signed-off-by: line ;-)  But that is sane in the\ncontext of coming up with totally new commit log message.\n\nI am more worried about how this should interact with cases\nwhere you usually do not start the log message from scratch.\nFor example, are there cases / policies where being able to use\ntemplates to leave comments on merge commits are needed?\nSquash-commits?  Perhaps \"apply this template but only when you\nhave hand resolved a conflicting merges\"?\n\nOr even the case of amending a commit made by somebody else,\nwithout changing the tree contents, in order to make the commit\nlog message to conform to the company standard?\n"},{"id":"48448","messageId":"46A5EA2D.1030707@midwinter.com","threadId":"9163","inReplyTo":"7vfy3fkpr8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-07-24T12:01:49Z","receivedAt":"2007-07-24T12:01:49Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Offhand, an \"interesting\" side effect of the above I can see is\n> that you will cry \"wolf\" if the only thing the user did is to\n> add his own Signed-off-by: line ;-)  But that is sane in the\n> context of coming up with totally new commit log message.\n>   \n\nYes, that is intentional -- the existing git-commit code wasn't treating \nSigned-off-by: as content when it checked for a commit message, so I \nwanted to preserve that behavior.\n\n> I am more worried about how this should interact with cases\n> where you usually do not start the log message from scratch.\n> For example, are there cases / policies where being able to use\n> templates to leave comments on merge commits are needed?\n> Squash-commits?  Perhaps \"apply this template but only when you\n> have hand resolved a conflicting merges\"?\n>   \n\nThose are all valid cases. They also happen to be ones that are not \npersonally useful to me; none of the stuff in my company's commit \ntemplate would really apply to merges. I'm happy to do it if you think \nthose cases need to be handled before this can go in, but given there's \nat least one user out there who doesn't need those things, I wonder if \nthis makes more sense to hold off on until someone actually asks for it.\n\n> Or even the case of amending a commit made by somebody else,\n> without changing the tree contents, in order to make the commit\n> log message to conform to the company standard?\n>   \n\nIt's not clear to me that I can do anything useful in that case, given \nthat there's no way to tell in what way the commit message needs to be \nfixed up. If it's a case of, \"This engineer committed without using the \ntemplate at all,\" then I can see an argument for loading the template \nabove / below the existing comment, but IMO a more common case will be \nsomething like, \"The format is right, but the bug ID isn't filled in or \nis incorrect,\" in which case any template I supply would be useless and \nannoying.\n\nIs your hunch about the common case different than mine? Again, I can go \naddress this if it'll actually be useful. Maybe by always including the \ntemplate if the command-line option is used, but skipping it in these \ncases if the config option is used? Or something like that? (On the \nother hand, if you have to specify the template on the command line, \nmaybe it's easier to just load it into the editor as needed.)\n\n-Steve\n"},{"id":"48523","messageId":"7v3azdh400.fsf@assigned-by-dhcp.cox.net","threadId":"9163","inReplyTo":"46A5EA2D.1030707@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-24T21:43:27Z","receivedAt":"2007-07-24T21:43:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm <koreth@midwinter.com> writes:\n\n> Junio C Hamano wrote:\n> ...\n>> I am more worried about how this should interact with cases\n>> where you usually do not start the log message from scratch.\n>> For example, are there cases / policies where being able to use\n>> templates to leave comments on merge commits are needed?\n>> Squash-commits?  Perhaps \"apply this template but only when you\n>> have hand resolved a conflicting merges\"?\n>>\n>\n> Those are all valid cases. They also happen to be ones that are not\n> personally useful to me; none of the stuff in my company's commit\n> template would really apply to merges. I'm happy to do it if you think\n> those cases need to be handled before this can go in, but given\n> there's at least one user out there who doesn't need those things, I\n> wonder if this makes more sense to hold off on until someone actually\n> asks for it.\n> ...\n> Is your hunch about the common case different than mine? Again, I can\n> go address this if it'll actually be useful.\n\nNo, I am way more cunning and lazy than that.  I did not have\nhunch about the common case, so I just had you (and anybody else\nwho would join the thread) do necessary thinking and guessing\nfor me ;-)\n"},{"id":"48541","messageId":"46A6F0C8.7010204@midwinter.com","threadId":"9163","inReplyTo":"7v3azdh400.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2007-07-25T06:42:16Z","receivedAt":"2007-07-25T06:42:16Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> No, I am way more cunning and lazy than that.  I did not have\n> hunch about the common case, so I just had you (and anybody else\n> who would join the thread) do necessary thinking and guessing\n> for me ;-)\n>   \n\nAh, good. So it sounds like what I've got is at least an okay first cut \nfrom a functionality point of view. Dscho (or anyone else), any \nobjections to the code in v2 of my patch? I believe I addressed all the \nfeedback from v1.\n\n-Steve\n"},{"id":"48545","messageId":"7vlkd5dj8x.fsf@assigned-by-dhcp.cox.net","threadId":"9163","inReplyTo":"46A6F0C8.7010204@midwinter.com","subject":"Re: [PATCH] Teach git-commit about commit message templates.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-25T07:39:58Z","receivedAt":"2007-07-25T07:39:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Grimm <koreth@midwinter.com> writes:\n\n> Junio C Hamano wrote:\n>> No, I am way more cunning and lazy than that.  I did not have\n>> hunch about the common case, so I just had you (and anybody else\n>> who would join the thread) do necessary thinking and guessing\n>> for me ;-)\n>>\n>\n> Ah, good. So it sounds like what I've got is at least an okay first\n> cut from a functionality point of view. Dscho (or anyone else), any\n> objections to the code in v2 of my patch? I believe I addressed all\n> the feedback from v1.\n\nI have already queued it for 1.5.3-rc3.  Thanks.\n"}]}