{"thread":{"id":"11375","subject":"[PATCH] git-commit: add --verbatim to allow unstripped commit messages","startedAt":"2007-12-20T21:18:35Z","lastAt":"2007-12-23T03:46:38Z","messageCount":23,"participants":["Alex Riesen","Linus Torvalds","Junio C Hamano","Björn Steinbrink"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"63877","messageId":"20071220211835.GA3052@steel.home","threadId":"11375","inReplyTo":null,"subject":"[PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T21:18:35Z","receivedAt":"2007-12-20T21:18:35Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"It also implies --allow-empty.\n\nSometimes the message just have to be the way user wants it.\nFor instance, a template can contain \"#\" characters, or the message\nmust be kept as close to its original source as possible for reimport\nreasons. Or maybe the user just copied a shell script including its\ncomments into the commit message for future reference.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nI just happen to have a corporate template (for perforce messages, I\nreuse it my git mirror repo) which contains \"#\" and at least one time\nlost my bash comments in a commit.\n\n Documentation/git-commit.txt |    7 ++++++-\n builtin-commit.c             |   10 ++++++++--\n t/t7502-commit.sh            |   17 +++++++++++++++++\n 3 files changed, 31 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 4261384..862543f 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git-commit' [-a | --interactive] [-s] [-v] [-u]\n \t   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]\n \t   [--allow-empty] [--no-verify] [-e] [--author <author>]\n-\t   [--] [[-i | -o ]<file>...]\n+\t   [--verbatim] [--] [[-i | -o ]<file>...]\n \n DESCRIPTION\n -----------\n@@ -95,6 +95,11 @@ OPTIONS\n \tfrom making such a commit.  This option bypasses the safety, and\n \tis primarily for use by foreign scm interface scripts.\n \n+--verbatim::\n+\tInhibits stripping of leading and trailing spaces,\n+\tempty lines and #commentary from the commit message.\n+\tImplies --allow-empty.\n+\n -e|--edit::\n \tThe message taken from file with `-F`, command line with\n \t`-m`, and from file with `-C` are usually used as the\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0a91013..cc77ba9 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n+static int verbatim_message;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n+\tOPT_BOOLEAN(0, \"verbatim\", &verbatim_message, \"do not strip spaces and #comments from message\"),\n \n \tOPT_END()\n };\n@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n \n-\tstripspace(&sb, 0);\n+\tif (!verbatim_message)\n+\t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n \t\tstruct strbuf sob;\n@@ -512,6 +515,8 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tno_edit = 1;\n \tif (edit_flag)\n \t\tno_edit = 0;\n+\tif (verbatim_message)\n+\t\tallow_empty = 1;\n \n \tif (get_sha1(\"HEAD\", head_sha1))\n \t\tinitial_commit = 1;\n@@ -813,7 +818,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tstripspace(&sb, 1);\n+\tif (!verbatim_message)\n+\t\tstripspace(&sb, 1);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 21ac785..42f98bc 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -89,4 +89,21 @@ test_expect_success 'verbose' '\n \n '\n \n+test_expect_success 'verbatim commit messages' '\n+\n+\techo >>negative &&\n+\tgit add negative &&\n+\t{ echo;echo \"# text\";echo; } >expect &&\n+\tgit commit --verbatim -t expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n+\tdiff -u expect actual &&\n+\tgit commit --verbatim -F expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual &&\n+\tgit commit --verbatim -m \"$(cat expect)\" -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n test_done\n-- \n1.5.4.rc1.33.gbd32b\n"},{"id":"63878","messageId":"alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org","threadId":"11375","inReplyTo":"20071220211835.GA3052@steel.home","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-20T21:40:13Z","receivedAt":"2007-12-20T21:40:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 20 Dec 2007, Alex Riesen wrote:\n> \n> I just happen to have a corporate template (for perforce messages, I\n> reuse it my git mirror repo) which contains \"#\" and at least one time\n> lost my bash comments in a commit.\n\nI think that this is a real bug, but I don't think this is something that \nwe should add a flag for.\n\nBasically, I don't think we should really strip lines starting with '#' \nunless *we* added them. In particular, I don't think we should strip them \nat all unless we're running the editor.\n\nSo I think that instead of your thing, we should do somethign like the \nappended, which allows you to do things like\n\n\tgit commit -m \"# Message starting with a hash-mark\"\n\nwhich the current code makes impossible (\"empty commit message\").\n\nThat may be enough for your case, although it still does leave the \"use \neditor on a template thing\", so if that is your usage scenario, I guess we \nstill do need a flag for it.\n\nBut even if we *do* add a flag (like \"--verbatim\") you should at the \n*least* also then remove the\n\n\t\"# (Comment lines starting with '#' will not be included)\\n\"\n\nprintout! Which you didn't.\n\nSo I say NAK on this patch.\n\n> It also implies --allow-empty.\n\nI disagree with this one too.\n\nWe have had *way* too many problems with various tools generating bogus \nempty commits. I get them from stgit users (and I think this is a serious \nBUG in stgit, dammit!), but I have this memory of some other usage \nscenario that did it too. \n\nIn other words, empty commits are almost always just bogus. And dammit, if \nthey aren't bogus, you should *say* so. No \"implied\" permissions, please. \nIf you really want your commits to be empty, what's the downside of just \nadding an explicit \"--allow-empty\"?\n\n\t\tLinus\n\n---\n builtin-commit.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0a91013..4685938 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -434,7 +434,7 @@ static int message_is_empty(struct strbuf *sb, int start)\n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n-\t\tstripspace(&tmpl, 1);\n+\t\tstripspace(&tmpl, !no_edit);\n \t\tif (start + tmpl.len <= sb->len &&\n \t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n \t\t\tstart += tmpl.len;\n@@ -813,7 +813,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tstripspace(&sb, 1);\n+\tstripspace(&sb, !no_edit);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\n"},{"id":"63882","messageId":"20071220233324.GB3052@steel.home","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T23:33:24Z","receivedAt":"2007-12-20T23:33:24Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Linus Torvalds, Thu, Dec 20, 2007 22:40:13 +0100:\n> On Thu, 20 Dec 2007, Alex Riesen wrote:\n> > \n> > I just happen to have a corporate template (for perforce messages, I\n> > reuse it my git mirror repo) which contains \"#\" and at least one time\n> > lost my bash comments in a commit.\n> \n> I think that this is a real bug, but I don't think this is something that \n> we should add a flag for.\n> \n> Basically, I don't think we should really strip lines starting with '#' \n> unless *we* added them. In particular, I don't think we should strip them \n> at all unless we're running the editor.\n\nRight\n\n> That may be enough for your case, although it still does leave the \"use \n> editor on a template thing\", so if that is your usage scenario, I guess we \n> still do need a flag for it.\n\nYes, I afraid I need both. I use \"git commit -t\" almost (submission in\nperforce takes careful planning) every day. I also would like to keep\nthe empty leading and trailing lines (perforce default GUI P4Win does\nnot show them, but our scripts which check the descriptions will test\nthe description text according to template which does have trailing\nempty lines).\n\n> But even if we *do* add a flag (like \"--verbatim\") you should at the \n> *least* also then remove the\n> \n> \t\"# (Comment lines starting with '#' will not be included)\\n\"\n> \n> printout! Which you didn't.\n\nI did think about it. It wont be read, I believe (at least I ignore\nthe status listing git-commit generates today). But then, it can be\nremoved for verbatim message as no one (I think) will probably care.\nIncluding the \"Please...\" so it does not look stupid in the commit\nmessage later. Will do.\n\n> > It also implies --allow-empty.\n> \n> I disagree with this one too.\n> \n\nI agree. Will remove (I am not even sure myself, why I did that. It is\nnot even tested)\n"},{"id":"63883","messageId":"20071220233539.GC3052@steel.home","threadId":"11375","inReplyTo":"20071220233324.GB3052@steel.home","subject":"[PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T23:35:39Z","receivedAt":"2007-12-20T23:35:39Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Sometimes the message just have to be the way user wants it.\nFor instance, a template can contain \"#\" characters, or the message\nmust be kept as close to its original source as possible for reimport\nreasons. Or maybe the user just copied a shell script including its\ncomments into the commit message for future reference.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nUpdated patch. It conflicts with yours a bit. Will update it\n\n Documentation/git-commit.txt |    7 ++++++-\n builtin-commit.c             |   20 ++++++++++++++------\n t/t7502-commit.sh            |   18 ++++++++++++++++++\n 3 files changed, 38 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 4261384..862543f 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git-commit' [-a | --interactive] [-s] [-v] [-u]\n \t   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]\n \t   [--allow-empty] [--no-verify] [-e] [--author <author>]\n-\t   [--] [[-i | -o ]<file>...]\n+\t   [--verbatim] [--] [[-i | -o ]<file>...]\n \n DESCRIPTION\n -----------\n@@ -95,6 +95,11 @@ OPTIONS\n \tfrom making such a commit.  This option bypasses the safety, and\n \tis primarily for use by foreign scm interface scripts.\n \n+--verbatim::\n+\tInhibits stripping of leading and trailing spaces,\n+\tempty lines and #commentary from the commit message.\n+\tImplies --allow-empty.\n+\n -e|--edit::\n \tThe message taken from file with `-F`, command line with\n \t`-m`, and from file with `-C` are usually used as the\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0a91013..3127247 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,6 +47,7 @@ static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n+static int verbatim_message;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n@@ -88,6 +89,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n+\tOPT_BOOLEAN(0, \"verbatim\", &verbatim_message, \"do not strip spaces and #comments from message\"),\n \n \tOPT_END()\n };\n@@ -346,7 +348,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n \n-\tstripspace(&sb, 0);\n+\tif (!verbatim_message)\n+\t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n \t\tstruct strbuf sob;\n@@ -404,10 +407,11 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\t\t\"#\\n\",\n \t\t\tgit_path(\"MERGE_HEAD\"));\n \n-\tfprintf(fp,\n-\t\t\"\\n\"\n-\t\t\"# Please enter the commit message for your changes.\\n\"\n-\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\tif (!verbatim_message)\n+\t\tfprintf(fp,\n+\t\t\t\"\\n\"\n+\t\t\t\"# Please enter the commit message for your changes.\\n\"\n+\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n \tif (only_include_assumed)\n \t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n \n@@ -431,6 +435,9 @@ static int message_is_empty(struct strbuf *sb, int start)\n \tconst char *nl;\n \tint eol, i;\n \n+\tif (verbatim_message && sb->len)\n+\t\treturn 1;\n+\n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n@@ -813,7 +820,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tstripspace(&sb, 1);\n+\tif (!verbatim_message)\n+\t\tstripspace(&sb, 1);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 21ac785..d549728 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -89,4 +89,22 @@ test_expect_success 'verbose' '\n \n '\n \n+test_expect_success 'verbatim commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >expect &&\n+\tgit commit --verbatim -t expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n+\tdiff -u expect actual &&\n+\techo >>negative &&\n+\tgit commit --verbatim -F expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual &&\n+\techo >>negative &&\n+\tgit commit --verbatim -m \"$(cat expect)\" -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n test_done\n-- \n1.5.4.rc1.33.gbd32b\n"},{"id":"63884","messageId":"20071220233714.GD3052@steel.home","threadId":"11375","inReplyTo":"20071220233539.GC3052@steel.home","subject":"[PATCH] Only filter \"#\" comments from commits if the editor is used","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T23:37:14Z","receivedAt":"2007-12-20T23:37:14Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Originally-by: Linus Torvalds <torvalds@linux-foundation.org>\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n builtin-commit.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 3127247..1356d20 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -441,7 +441,7 @@ static int message_is_empty(struct strbuf *sb, int start)\n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n-\t\tstripspace(&tmpl, 1);\n+\t\tstripspace(&tmpl, !no_edit);\n \t\tif (start + tmpl.len <= sb->len &&\n \t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n \t\t\tstart += tmpl.len;\n@@ -821,7 +821,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n \tif (!verbatim_message)\n-\t\tstripspace(&sb, 1);\n+\t\tstripspace(&sb, !no_edit);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\n-- \n1.5.4.rc1.33.gbd32b\n"},{"id":"63885","messageId":"7vtzmdlyva.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"20071220233714.GD3052@steel.home","subject":"Re: [PATCH] Only filter \"#\" comments from commits if the editor is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-20T23:39:37Z","receivedAt":"2007-12-20T23:39:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"},{"id":"63886","messageId":"7vprx1lyp9.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"20071220233714.GD3052@steel.home","subject":"Re: [PATCH] Only filter \"#\" comments from commits if the editor is used","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-20T23:43:14Z","receivedAt":"2007-12-20T23:43:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I said thanks a bit too early.  That is on top of verbatim, which I have\ndoubts about putting in 1.5.4.  I on the other hand think Linus's is a\npure bugfix.\n\nWill tweak and apply, thanks.\n"},{"id":"63887","messageId":"20071220234341.GE3052@steel.home","threadId":"11375","inReplyTo":"7vtzmdlyva.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Only filter \"#\" comments from commits if the editor is used","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T23:43:41Z","receivedAt":"2007-12-20T23:43:41Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Fri, Dec 21, 2007 00:39:37 +0100:\n> Thanks.\n\nPlease hold on. The test fails.\n"},{"id":"63888","messageId":"20071220234849.GF3052@steel.home","threadId":"11375","inReplyTo":"20071220233539.GC3052@steel.home","subject":"[PATCH] Fix thinko in checking for commit message emptiness","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-20T23:48:49Z","receivedAt":"2007-12-20T23:48:49Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nSorry. It must be late. It it must have been late yesterday\nand still is today.\n\n builtin-commit.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 1356d20..eae7661 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -436,7 +436,7 @@ static int message_is_empty(struct strbuf *sb, int start)\n \tint eol, i;\n \n \tif (verbatim_message && sb->len)\n-\t\treturn 1;\n+\t\treturn 0;\n \n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n-- \n1.5.4.rc1.33.gbd32b\n"},{"id":"63889","messageId":"7vlk7plydv.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712201324270.21557@woody.linux-foundation.org","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-20T23:50:04Z","receivedAt":"2007-12-20T23:50:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I said in the log message when applying your version:\n\n    We even stripped \"#\" lines from messages given this way:\n\n        git commit -m \"# Message starting with a hash-mark\"\n\n    which was argurably a bug.\n\nBut I wonder if people are using a workflow like this:\n\n\t$ cp $company_template my_message\n        $ edit my_message\n        $ git commit -F my_message\n\nTo them, this change is a regression, as $company_template may have had\ninsn like \"# Here you describe your changes...\" and the workflow relied\non the current behaviour of stripping them.\n"},{"id":"63890","messageId":"alpine.LFD.0.9999.0712201545450.21557@woody.linux-foundation.org","threadId":"11375","inReplyTo":"20071220233324.GB3052@steel.home","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-20T23:55:18Z","receivedAt":"2007-12-20T23:55:18Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 21 Dec 2007, Alex Riesen wrote:\n> \n> Yes, I afraid I need both. I use \"git commit -t\" almost (submission in\n> perforce takes careful planning) every day. I also would like to keep\n> the empty leading and trailing lines (perforce default GUI P4Win does\n> not show them, but our scripts which check the descriptions will test\n> the description text according to template which does have trailing\n> empty lines).\n\nHmm. I think your updated patch was pretty good, although I still think it \ncould be improved a bit. In particular, thinking more about it, I think we \nhave more than an \"on/off\" switch - we really have three cases:\n\n a) strip whitespace _and_ comments\n b) strip unnecessary whitespace only\n c) leave things _totally_ alone\n\nand on top of that we also have the issue of an editor.\n\nSo my patch basically said that in the absense of an editor, we'll still \nclean up whitespace, but not comments (ie \"no_edit\" implies doing (b) \nrather than (b)), while your patch basically results in (c) regardless of \nwhether we run an editor or not.\n\nBut that still leaves one case: do we ever want to do (b) even *if* we use \nan editor? There's another possible choice: our old behaviour of (a) in \nthe presense of an editor is now gone.\n\nNow, that last choice (ie \"case (a) without an editor\") is not only \nunlikely to be anything people want to do anyway, it's also easy enough to \ndo by just using \"git stripspace -s\" on whatever non-editor thing you feed \nto \"git commit\", so I don't think we need to worry about that one.\n\nBut the \"maybe you want to run an editor, and you _do_ want case (b)\" \nsounds like a case that is not at all unlikely. I could easily see the \ncase where you want to have a template that uses '#', and *despite* that \nyou want to (a) allow the user to edit things _and_ (b) clean up \nwhitespace too.\n\nSo I'd almost suggest you make the \"--verbatim\" flag a three-way switch, \nto allow \"totally verbatim\" (leave everything in place) and a \"don't touch \ncomments\" (just fix up whitespace) mode.\n\nHmm? Does that make sense to you?\n\n\t\t\tLinus\n"},{"id":"63891","messageId":"alpine.LFD.0.9999.0712201555570.21557@woody.linux-foundation.org","threadId":"11375","inReplyTo":"7vlk7plydv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-21T00:03:26Z","receivedAt":"2007-12-21T00:03:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 20 Dec 2007, Junio C Hamano wrote:\n> \n> But I wonder if people are using a workflow like this:\n> \n> \t$ cp $company_template my_message\n>         $ edit my_message\n>         $ git commit -F my_message\n\nYes. See the email I just sent to Alex.\n\nI think that the above kind of workflow is trivial to support with the \n\"new\" semantics (ie the patch under discussion), by just rewriting that \nlast step as\n\n\t$ git stripspace -s < my_message | git-commit -F -\n\nso in that sense, we don't really \"lose\" anything, although I do agree \nthat backwards compatibility (in case somebody actually does this!) \nsuffers.\n\nBut for the case of actually using the built-in editing capability, we \ndon't have that choice, so I'd argue that regardless, we should make the \n\"--verbatim\" switch be a three-way choice between (a) not touching things \nat all, (b) touching up just whitespace, or (c) doing the full enchilada.\n\nAnd if we introduce such a flag, then I think we can make the *default* \n(in the absense of an explicit flag) be something like\n\n\tif (no_edit)\n\t\tdefault = touch_up_just_whitespace;\n\telse\n\t\tdefault = whole_enchilada\n\nand obviously it could also be a configuration option.\n\nThat way, you could always get the existing behaviour with\n\n\tgit commit --verbatim=full-enchilada -F my_message\n\n(yeah, bad name - I'm not seriously suggesting it be called \n\"full-enchilada\", and I'm also not sure the flag should be called \n\"--verbatim\" any more if it has choices, it's more likely that we should \ncall it something like \"--cleanup={verbatim,whitespace,strip}\" or \nsomething like that.\n\n\t\t\tLinus\n"},{"id":"63892","messageId":"20071221001449.GA10607@atjola.homenet","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712201545450.21557@woody.linux-foundation.org","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2007-12-21T00:14:49Z","receivedAt":"2007-12-21T00:14:49Z","isPatch":true,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2007.12.20 15:55:18 -0800, Linus Torvalds wrote:\n> \n> \n> On Fri, 21 Dec 2007, Alex Riesen wrote:\n> > \n> > Yes, I afraid I need both. I use \"git commit -t\" almost (submission in\n> > perforce takes careful planning) every day. I also would like to keep\n> > the empty leading and trailing lines (perforce default GUI P4Win does\n> > not show them, but our scripts which check the descriptions will test\n> > the description text according to template which does have trailing\n> > empty lines).\n> \n> Hmm. I think your updated patch was pretty good, although I still think it \n> could be improved a bit. In particular, thinking more about it, I think we \n> have more than an \"on/off\" switch - we really have three cases:\n> \n>  a) strip whitespace _and_ comments\n>  b) strip unnecessary whitespace only\n>  c) leave things _totally_ alone\n> \n> and on top of that we also have the issue of an editor.\n> \n> So my patch basically said that in the absense of an editor, we'll still \n> clean up whitespace, but not comments (ie \"no_edit\" implies doing (b) \n> rather than (b)), while your patch basically results in (c) regardless of \n> whether we run an editor or not.\n> \n> But that still leaves one case: do we ever want to do (b) even *if* we use \n> an editor? There's another possible choice: our old behaviour of (a) in \n> the presense of an editor is now gone.\n> \n> Now, that last choice (ie \"case (a) without an editor\") is not only \n> unlikely to be anything people want to do anyway, it's also easy enough to \n> do by just using \"git stripspace -s\" on whatever non-editor thing you feed \n> to \"git commit\", so I don't think we need to worry about that one.\n> \n> But the \"maybe you want to run an editor, and you _do_ want case (b)\" \n> sounds like a case that is not at all unlikely. I could easily see the \n> case where you want to have a template that uses '#', and *despite* that \n> you want to (a) allow the user to edit things _and_ (b) clean up \n> whitespace too.\n> \n> So I'd almost suggest you make the \"--verbatim\" flag a three-way switch, \n> to allow \"totally verbatim\" (leave everything in place) and a \"don't touch \n> comments\" (just fix up whitespace) mode.\n> \n> Hmm? Does that make sense to you?\n\nHm, this is a bit more intrusive, but should catch most cases.\n\nAt the top of the comments in the commit message template add:\n#GIT CUT HERE\n(And adjust the descriptive text)\n\nThat line hopefully being uncommon enough to not affect any existing\nstuff.\n\nIf that line is present, comment lines above it are kept, otherwise\nthey're removed. Whitespace is always fixed(?).\n\nResults:\n - git commit -m \"# Foo and bar\"\n   * keeps the comment, looks like the expected thing\n\n - git commit with editor\n   * Comments that are manually added are kept\n   * For the (probably seldom) case, that you want manually added\n     comments to be stripped, you can still remove the \"#GIT CUR HERE\"\n     line\n\n - git commit with template\n   * The existing templates probably won't have a \"#GIT CUT HERE\" line,\n     so we're backwards compatible\n   * Templates that want to keep the comments can simple get a \"#GIT CUT\n     HERE\" line at the end and Just Work, regardless of whether or not\n     you forget to pass --verbatim.\n\n\nHmm?\n\nthanks,\nBjörn\n"},{"id":"63895","messageId":"alpine.LFD.0.9999.0712201645020.21557@woody.linux-foundation.org","threadId":"11375","inReplyTo":"20071221001449.GA10607@atjola.homenet","subject":"Re: [PATCH] git-commit: add --verbatim to allow unstripped commit messages","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-21T00:49:33Z","receivedAt":"2007-12-21T00:49:33Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 21 Dec 2007, Bj?rn Steinbrink wrote:\n> \n> Hm, this is a bit more intrusive, but should catch most cases.\n> \n> At the top of the comments in the commit message template add:\n> #GIT CUT HERE\n> (And adjust the descriptive text)\n\nOuch. I'd personally hate to see something like that at the top and then \nhave to save it. I always add my text to the top of the message, and all \nthe pre-made messages for me are at the top (ie the kinds you get with \n\"git commit --amend\", where the top of the thing is the old message).\n\nThat said, I might well agree with this approach if we made the marker \nline be the *last* line of the message, ie make it be something that ends \nwith (ignoring empty whitespace at the end, of course, since those are \ninvisible in most editors):\n\n\t# Remove this line to keep all comment lines\n\nor something like that.\n\nThat still keeps the question about whitespace cleanups. But if it's just \nabout whitespace or no whitespace, then a simple \"--verbatim\" flag would \nwork.\n\n\t\t\tLinus\n"},{"id":"63937","messageId":"20071221173521.GA3494@steel.home","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712201555570.21557@woody.linux-foundation.org","subject":"[PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-21T17:35:21Z","receivedAt":"2007-12-21T17:35:21Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"The modes being default, verbatim, whitespace and strip. The default\nmode depends on if the message is being edited and will either strip\nwhitespace and comments (if editor active) or just strip the\nwhitespace (for where the message is given explicitely).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n\n---\nLinus Torvalds, Fri, Dec 21, 2007 01:03:26 +0100:\n> But for the case of actually using the built-in editing capability, we \n> don't have that choice, so I'd argue that regardless, we should make the \n> \"--verbatim\" switch be a three-way choice between (a) not touching things \n> at all, (b) touching up just whitespace, or (c) doing the full enchilada.\n> \n> And if we introduce such a flag, then I think we can make the *default* \n> (in the absense of an explicit flag) be something like\n> \n> \tif (no_edit)\n> \t\tdefault = touch_up_just_whitespace;\n> \telse\n> \t\tdefault = whole_enchilada\n> \n> and obviously it could also be a configuration option.\n\nwhich makes it four-way choice...\n\n> That way, you could always get the existing behaviour with\n> \n> \tgit commit --verbatim=full-enchilada -F my_message\n> \n> (yeah, bad name - I'm not seriously suggesting it be called \n> \"full-enchilada\", and I'm also not sure the flag should be called \n> \"--verbatim\" any more if it has choices, it's more likely that we should \n> call it something like \"--cleanup={verbatim,whitespace,strip}\" or \n> something like that.\n\n\"verbatim\", \"whitespace\", \"strip\", \"default\".\n\nThe patch is on top of my previos patches. Junio, if you wish, I can\nsquash and resend.\n\n Documentation/git-commit.txt |   15 ++++++---\n builtin-commit.c             |   69 +++++++++++++++++++++++++++++-------------\n t/t7502-commit.sh            |   28 +++++++++++++++--\n 3 files changed, 83 insertions(+), 29 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 862543f..ac2bfd5 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git-commit' [-a | --interactive] [-s] [-v] [-u]\n \t   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]\n \t   [--allow-empty] [--no-verify] [-e] [--author <author>]\n-\t   [--verbatim] [--] [[-i | -o ]<file>...]\n+\t   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]\n \n DESCRIPTION\n -----------\n@@ -95,10 +95,15 @@ OPTIONS\n \tfrom making such a commit.  This option bypasses the safety, and\n \tis primarily for use by foreign scm interface scripts.\n \n---verbatim::\n-\tInhibits stripping of leading and trailing spaces,\n-\tempty lines and #commentary from the commit message.\n-\tImplies --allow-empty.\n+--cleanup=<mode>::\n+\tThis option sets how the commit message is cleaned up.\n+\tThe  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',\n+\tand 'default'. The 'default' mode will strip leading and\n+\ttrailing empty lines and #commentary from the commit message\n+\tonly if the message is to be edited. Otherwise only whitespace\n+\tremoved. The 'verbatim' mode wont change message at all,\n+\t'whitespace' removes just leading/trailing whitespace lines\n+\tand 'strip' removes both whitespace and commentary.\n \n -e|--edit::\n \tThe message taken from file with `-F`, command line with\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex eae7661..6f98537 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,7 +47,20 @@ static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n-static int verbatim_message;\n+/*\n+ * The default commit message cleanup mode will remove the lines\n+ * beginning with # (shell comments) and leading and trailing\n+ * whitespaces (empty lines or containing only whitespaces)\n+ * if editor is used, and only the whitespaces if the message\n+ * is specified explicitly.\n+ */\n+static enum {\n+\tCLEANUP_DEFAULT,\n+\tCLEANUP_NONE,\n+\tCLEANUP_SPACE,\n+\tCLEANUP_ALL,\n+} cleanup_mode;\n+static char *cleanup_arg;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n@@ -89,7 +102,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n-\tOPT_BOOLEAN(0, \"verbatim\", &verbatim_message, \"do not strip spaces and #comments from message\"),\n+\tOPT_STRING(0, \"cleanup\", &cleanup_arg, \"default\", \"how to strip spaces and #comments from message\"),\n \n \tOPT_END()\n };\n@@ -348,7 +361,7 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n \n-\tif (!verbatim_message)\n+\tif (cleanup_mode != CLEANUP_NONE)\n \t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n@@ -397,21 +410,24 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\treturn !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n \t}\n \n-\tif (in_merge && !no_edit)\n-\t\tfprintf(fp,\n-\t\t\t\"#\\n\"\n-\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n-\t\t\t\"# If this is not correct, please remove the file\\n\"\n-\t\t\t\"#\t%s\\n\"\n-\t\t\t\"# and try again.\\n\"\n-\t\t\t\"#\\n\",\n-\t\t\tgit_path(\"MERGE_HEAD\"));\n-\n-\tif (!verbatim_message)\n-\t\tfprintf(fp,\n-\t\t\t\"\\n\"\n-\t\t\t\"# Please enter the commit message for your changes.\\n\"\n-\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\tif (!no_edit) {\n+\t\tif (in_merge)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"#\\n\"\n+\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\t\"#\t%s\\n\"\n+\t\t\t\t\"# and try again.\\n\"\n+\t\t\t\t\"#\\n\",\n+\t\t\t\tgit_path(\"MERGE_HEAD\"));\n+\t\tif (cleanup_mode != CLEANUP_NONE)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"# Please enter the commit message for your changes.\\n\");\n+\t\tif (cleanup_mode == CLEANUP_DEFAULT)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\t}\n \tif (only_include_assumed)\n \t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n \n@@ -435,7 +451,7 @@ static int message_is_empty(struct strbuf *sb, int start)\n \tconst char *nl;\n \tint eol, i;\n \n-\tif (verbatim_message && sb->len)\n+\tif (cleanup_mode == CLEANUP_NONE && sb->len)\n \t\treturn 0;\n \n \t/* See if the template is just a prefix of the message. */\n@@ -594,6 +610,16 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tonly_include_assumed = \"Explicit paths specified without -i nor -o; assuming --only paths...\";\n \t\talso = 0;\n \t}\n+\tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n+\t\tcleanup_mode = CLEANUP_DEFAULT;\n+\telse if (!strcmp(cleanup_arg, \"verbatim\"))\n+\t\tcleanup_mode = CLEANUP_NONE;\n+\telse if (!strcmp(cleanup_arg, \"whitespace\"))\n+\t\tcleanup_mode = CLEANUP_SPACE;\n+\telse if (!strcmp(cleanup_arg, \"strip\"))\n+\t\tcleanup_mode = CLEANUP_ALL;\n+\telse\n+\t\tdie(\"Invalid cleanup mode %s\", cleanup_arg);\n \n \tif (all && argc > 0)\n \t\tdie(\"Paths with -a does not make sense.\");\n@@ -820,8 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tif (!verbatim_message)\n-\t\tstripspace(&sb, !no_edit);\n+\tif (cleanup_mode != CLEANUP_NONE)\n+\t\tstripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?\n+\t\t\t   !no_edit: cleanup_mode == CLEANUP_ALL);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex d549728..6219378 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -93,15 +93,37 @@ test_expect_success 'verbatim commit messages' '\n \n \techo >>negative &&\n \t{ echo;echo \"# text\";echo; } >expect &&\n-\tgit commit --verbatim -t expect -a &&\n+\tgit commit --cleanup=verbatim -t expect -a &&\n \tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n \tdiff -u expect actual &&\n \techo >>negative &&\n-\tgit commit --verbatim -F expect -a &&\n+\tgit commit --cleanup=verbatim -F expect -a &&\n \tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n \tdiff -u expect actual &&\n \techo >>negative &&\n-\tgit commit --verbatim -m \"$(cat expect)\" -a &&\n+\tgit commit --cleanup=verbatim -m \"$(cat expect)\" -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup only whitespace from commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >text &&\n+\techo \"# text\" >expect &&\n+\tgit commit --cleanup=whitespace -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo sample;echo; } >text &&\n+\techo sample >expect &&\n+\tgit commit --cleanup=strip -F text -a &&\n \tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n \tdiff -u expect actual\n \n-- \n1.5.4.rc1.34.gf21f1\n"},{"id":"63957","messageId":"7v8x3n4tbe.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"20071221173521.GA3494@steel.home","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-21T21:43:49Z","receivedAt":"2007-12-21T21:43:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> The patch is on top of my previos patches. Junio, if you wish, I can\n> squash and resend.\n\nThat would be a sane thing to do.\n"},{"id":"63967","messageId":"20071221230851.GA3260@steel.home","threadId":"11375","inReplyTo":"7v8x3n4tbe.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-21T23:08:51Z","receivedAt":"2007-12-21T23:08:51Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Sometimes the message just have to be the way user wants it.\nFor instance, a template can contain \"#\" characters, or the message\nmust be kept as close to its original source as possible for reimport\nreasons. Or maybe the user just copied a shell script including its\ncomments into the commit message for future reference.\n\nThe cleanup modes are default, verbatim, whitespace and strip. The\ndefault mode depends on if the message is being edited and will either\nstrip whitespace and comments (if editor active) or just strip the\nwhitespace (for where the message is given explicitely).\n\nSuggested by Linus.\n\nSigned-off-by: Alex Riesen <raa@gmail.com>\n---\n\nJunio C Hamano, Fri, Dec 21, 2007 22:43:49 +0100:\n> Alex Riesen <raa.lkml@gmail.com> writes:\n> \n> > The patch is on top of my previos patches. Junio, if you wish, I can\n> > squash and resend.\n> \n> That would be a sane thing to do.\n\nDone\n\n Documentation/git-commit.txt |   12 +++++++-\n builtin-commit.c             |   69 +++++++++++++++++++++++++++++++----------\n t/t7502-commit.sh            |   40 ++++++++++++++++++++++++\n 3 files changed, 103 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 4261384..ac2bfd5 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git-commit' [-a | --interactive] [-s] [-v] [-u]\n \t   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]\n \t   [--allow-empty] [--no-verify] [-e] [--author <author>]\n-\t   [--] [[-i | -o ]<file>...]\n+\t   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]\n \n DESCRIPTION\n -----------\n@@ -95,6 +95,16 @@ OPTIONS\n \tfrom making such a commit.  This option bypasses the safety, and\n \tis primarily for use by foreign scm interface scripts.\n \n+--cleanup=<mode>::\n+\tThis option sets how the commit message is cleaned up.\n+\tThe  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',\n+\tand 'default'. The 'default' mode will strip leading and\n+\ttrailing empty lines and #commentary from the commit message\n+\tonly if the message is to be edited. Otherwise only whitespace\n+\tremoved. The 'verbatim' mode wont change message at all,\n+\t'whitespace' removes just leading/trailing whitespace lines\n+\tand 'strip' removes both whitespace and commentary.\n+\n -e|--edit::\n \tThe message taken from file with `-F`, command line with\n \t`-m`, and from file with `-C` are usually used as the\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0a91013..6f98537 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,6 +47,20 @@ static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n+/*\n+ * The default commit message cleanup mode will remove the lines\n+ * beginning with # (shell comments) and leading and trailing\n+ * whitespaces (empty lines or containing only whitespaces)\n+ * if editor is used, and only the whitespaces if the message\n+ * is specified explicitly.\n+ */\n+static enum {\n+\tCLEANUP_DEFAULT,\n+\tCLEANUP_NONE,\n+\tCLEANUP_SPACE,\n+\tCLEANUP_ALL,\n+} cleanup_mode;\n+static char *cleanup_arg;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n@@ -88,6 +102,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n+\tOPT_STRING(0, \"cleanup\", &cleanup_arg, \"default\", \"how to strip spaces and #comments from message\"),\n \n \tOPT_END()\n };\n@@ -346,7 +361,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n \n-\tstripspace(&sb, 0);\n+\tif (cleanup_mode != CLEANUP_NONE)\n+\t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n \t\tstruct strbuf sob;\n@@ -394,20 +410,24 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\treturn !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n \t}\n \n-\tif (in_merge && !no_edit)\n-\t\tfprintf(fp,\n-\t\t\t\"#\\n\"\n-\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n-\t\t\t\"# If this is not correct, please remove the file\\n\"\n-\t\t\t\"#\t%s\\n\"\n-\t\t\t\"# and try again.\\n\"\n-\t\t\t\"#\\n\",\n-\t\t\tgit_path(\"MERGE_HEAD\"));\n-\n-\tfprintf(fp,\n-\t\t\"\\n\"\n-\t\t\"# Please enter the commit message for your changes.\\n\"\n-\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\tif (!no_edit) {\n+\t\tif (in_merge)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"#\\n\"\n+\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\t\"#\t%s\\n\"\n+\t\t\t\t\"# and try again.\\n\"\n+\t\t\t\t\"#\\n\",\n+\t\t\t\tgit_path(\"MERGE_HEAD\"));\n+\t\tif (cleanup_mode != CLEANUP_NONE)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"# Please enter the commit message for your changes.\\n\");\n+\t\tif (cleanup_mode == CLEANUP_DEFAULT)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\t}\n \tif (only_include_assumed)\n \t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n \n@@ -431,10 +451,13 @@ static int message_is_empty(struct strbuf *sb, int start)\n \tconst char *nl;\n \tint eol, i;\n \n+\tif (cleanup_mode == CLEANUP_NONE && sb->len)\n+\t\treturn 0;\n+\n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n-\t\tstripspace(&tmpl, 1);\n+\t\tstripspace(&tmpl, !no_edit);\n \t\tif (start + tmpl.len <= sb->len &&\n \t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n \t\t\tstart += tmpl.len;\n@@ -587,6 +610,16 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tonly_include_assumed = \"Explicit paths specified without -i nor -o; assuming --only paths...\";\n \t\talso = 0;\n \t}\n+\tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n+\t\tcleanup_mode = CLEANUP_DEFAULT;\n+\telse if (!strcmp(cleanup_arg, \"verbatim\"))\n+\t\tcleanup_mode = CLEANUP_NONE;\n+\telse if (!strcmp(cleanup_arg, \"whitespace\"))\n+\t\tcleanup_mode = CLEANUP_SPACE;\n+\telse if (!strcmp(cleanup_arg, \"strip\"))\n+\t\tcleanup_mode = CLEANUP_ALL;\n+\telse\n+\t\tdie(\"Invalid cleanup mode %s\", cleanup_arg);\n \n \tif (all && argc > 0)\n \t\tdie(\"Paths with -a does not make sense.\");\n@@ -813,7 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tstripspace(&sb, 1);\n+\tif (cleanup_mode != CLEANUP_NONE)\n+\t\tstripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?\n+\t\t\t   !no_edit: cleanup_mode == CLEANUP_ALL);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 21ac785..6219378 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -89,4 +89,44 @@ test_expect_success 'verbose' '\n \n '\n \n+test_expect_success 'verbatim commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >expect &&\n+\tgit commit --cleanup=verbatim -t expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n+\tdiff -u expect actual &&\n+\techo >>negative &&\n+\tgit commit --cleanup=verbatim -F expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual &&\n+\techo >>negative &&\n+\tgit commit --cleanup=verbatim -m \"$(cat expect)\" -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup only whitespace from commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >text &&\n+\techo \"# text\" >expect &&\n+\tgit commit --cleanup=whitespace -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo sample;echo; } >text &&\n+\techo sample >expect &&\n+\tgit commit --cleanup=strip -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n test_done\n-- \n1.5.4.rc1.36.g24341\n"},{"id":"63983","messageId":"7v3atv17o9.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"20071221230851.GA3260@steel.home","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T07:59:34Z","receivedAt":"2007-12-22T07:59:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Junio C Hamano, Fri, Dec 21, 2007 22:43:49 +0100:\n>> Alex Riesen <raa.lkml@gmail.com> writes:\n>> \n>> > The patch is on top of my previos patches. Junio, if you wish, I can\n>> > squash and resend.\n>> \n>> That would be a sane thing to do.\n>\n> Done\n\nThanks.  I am afraid I have to ask you to either refute my\nmisunderstanding or a second round.\n\n> +--cleanup=<mode>::\n> +\tThis option sets how the commit message is cleaned up.\n> +\tThe  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',\n> +\tand 'default'. The 'default' mode will strip leading and\n> +\ttrailing empty lines and #commentary from the commit message\n> +\tonly if the message is to be edited. Otherwise only whitespace\n> +\tremoved. The 'verbatim' mode wont change message at all,\n> +\t'whitespace' removes just leading/trailing whitespace lines\n> +\tand 'strip' removes both whitespace and commentary.\n\n(minor) s/wont/does not/\n\n> @@ -394,20 +410,24 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n>  \t\treturn !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n>  \t}\n>  \n> +\tif (!no_edit) {\n> +\t\tif (in_merge)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"#\\n\"\n> +\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n> +\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n> +\t\t\t\t\"#\t%s\\n\"\n> +\t\t\t\t\"# and try again.\\n\"\n> +\t\t\t\t\"#\\n\",\n> +\t\t\t\tgit_path(\"MERGE_HEAD\"));\n> +\t\tif (cleanup_mode != CLEANUP_NONE)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"\\n\"\n> +\t\t\t\t\"# Please enter the commit message for your changes.\\n\");\n> +\t\tif (cleanup_mode == CLEANUP_DEFAULT)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n\nCan't cleanup_mode be CLEANUP_ALL at this point, and if so\nshouldn't this insn be given as well?\n\nIn addition, if:\n\n - we are talking with editor, and\n - the mode is verbatim, and\n - we added any \"#\" line ourselves (e.g. in_merge adds the insn\n   shown above, or perhaps we added \"git status\" output),\n\nthen perhaps we would want to say that \"#\" lines need to be\nstripped by the user; otherwise it will be left in the commit.\n\n> @@ -431,10 +451,13 @@ static int message_is_empty(struct strbuf *sb, int start)\n>  \tconst char *nl;\n>  \tint eol, i;\n>  \n> +\tif (cleanup_mode == CLEANUP_NONE && sb->len)\n> +\t\treturn 0;\n> +\n>  \t/* See if the template is just a prefix of the message. */\n>  \tstrbuf_init(&tmpl, 0);\n>  \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n> -\t\tstripspace(&tmpl, 1);\n> +\t\tstripspace(&tmpl, !no_edit);\n\nWe at this point know that some stripping would happen.  The\ntemplate needs to be cleaned the same way as the commit message\nwas stripped.  Is checking no_edit the right thing to do?  What\nif cleanup_mode was CLEANUP_ALL and there is no editing going\non?\n\n> @@ -813,7 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \tif (p != NULL)\n>  \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n>  \n> -\tstripspace(&sb, 1);\n> +\tif (cleanup_mode != CLEANUP_NONE)\n> +\t\tstripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?\n> +\t\t\t   !no_edit: cleanup_mode == CLEANUP_ALL);\n\nWhen writing such a long ? : expression, laying it out like:\n\n\tcondition\n        ? if-true\n        : if-false\n\nwould be easier to read.  You tilt your head sideways 90\ndegrees, just like you read a smiley ;-), and you will see the\nparse tree.\n\nBut I suspect, assuming that the two issues around CLEANUP_DEFAULT vs\nCLEANUP_ALL I mentioned above are indeed bugs, it may be cleaner\nto:\n\n  - parse the options; you get one of NONE/DEFAULT/SPACE/ALL\n\n  - convert DEFAULT to SPACE or ALL if editor is in effect.\n\n  - do not worry about no_editor or DEFAULT in the rest of the\n    code when calling stripspace().  The only values you will\n    see are NONE, SPACE or ALL.\n\n> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\n> index 21ac785..6219378 100755\n> --- a/t/t7502-commit.sh\n> +++ b/t/t7502-commit.sh\n> @@ -89,4 +89,44 @@ test_expect_success 'verbose' '\n>  \n>  '\n>  \n> +test_expect_success 'verbatim commit messages' '\n> +\n\nThese tests check too many things at once, and it would be very\nhard to diagnose when breakage does happen.  Please split them\nperhaps into three separate tests like this:\n\n> +\techo >>negative &&\n> +\t{ echo;echo \"# text\";echo; } >expect &&\n> +\tgit commit --cleanup=verbatim -t expect -a &&\n> +\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n> +\tdiff -u expect actual &&\n\nThis is one test --- making sure the commit body match the\nunedited template verbatim, except the status part to be shown\nin the editor.  But we are not using editor in this test and I\nthought the point of this --cleanup=verbatim exercise was that\nthe \"status\" part is not even have to be added to the commit log\nmessage when editor is not in effect...  Isn't the \"head -n 3\"\njust sweeping a bug under the rug?\n\n> +\techo >>negative &&\n> +\tgit commit --cleanup=verbatim -F expect -a &&\n> +\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n> +\tdiff -u expect actual &&\n\nAnd this is another test.\n\n> +\techo >>negative &&\n> +\tgit commit --cleanup=verbatim -m \"$(cat expect)\" -a &&\n> +\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n> +\tdiff -u expect actual\n\nAnd this is yet another.\n\n> +'\n\nAlso, the tests do not check \"-F file -e\", \"-m msg -e\" etc.  We\nwould want to make sure the right insn are shown to the user in\nthe editor, wouldn't we?\n"},{"id":"64014","messageId":"20071222184624.GA4167@steel.home","threadId":"11375","inReplyTo":"7v3atv17o9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-12-22T18:46:24Z","receivedAt":"2007-12-22T18:46:24Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Sometimes the message just have to be the way user wants it.\nFor instance, a template can contain \"#\" characters, or the message\nmust be kept as close to its original source as possible for reimport\nreasons. Or maybe the user just copied a shell script including its\ncomments into the commit message for future reference.\n\nThe cleanup modes are default, verbatim, whitespace and strip. The\ndefault mode depends on if the message is being edited and will either\nstrip whitespace and comments (if editor active) or just strip the\nwhitespace (for where the message is given explicitely).\n\nSuggested by Linus.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nJunio C Hamano, Sat, Dec 22, 2007 08:59:34 +0100:\n> > +\tremoved. The 'verbatim' mode wont change message at all,\n> > +\t'whitespace' removes just leading/trailing whitespace lines\n> > +\tand 'strip' removes both whitespace and commentary.\n> \n> (minor) s/wont/does not/\n> \n\nDone.\n\n> > +\t\tif (cleanup_mode == CLEANUP_DEFAULT)\n> > +\t\t\tfprintf(fp,\n> > +\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n> \n> Can't cleanup_mode be CLEANUP_ALL at this point, and if so\n> shouldn't this insn be given as well?\n\nRewritten as suggested (set cleanup_mode depending on no_edit in\nparse_and_validate_options).\n\n> In addition, if:\n> \n>  - we are talking with editor, and\n>  - the mode is verbatim, and\n>  - we added any \"#\" line ourselves (e.g. in_merge adds the insn\n>    shown above, or perhaps we added \"git status\" output),\n> \n> then perhaps we would want to say that \"#\" lines need to be\n> stripped by the user; otherwise it will be left in the commit.\n\nWell, I don't know. I find these click-through (look-through)\ninstructions uninteresting, and I sometimes feel tempted to suggest\na config option like \"core.autohelp_level\" and make it 0 by default.\nAs soon as someone complains - he'll be told to increase it. The\ncommit message hints would go at 1. We have plenty of space for\nother helpfulness levels in 31 bits.\n\nSeriously though, I agree with Linus: we better just make sure that\nwhatever was generated by Git never ends up in the commit message. And\nmake very sure user knows it. Maybe just don't put anything in commit\nmessage? Show the info elsewhere? Go with something like Björns\nsuggestion?\n\n> >  \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n> > -\t\tstripspace(&tmpl, 1);\n> > +\t\tstripspace(&tmpl, !no_edit);\n> \n> We at this point know that some stripping would happen.  The\n> template needs to be cleaned the same way as the commit message\n> was stripped.  Is checking no_edit the right thing to do?  What\n> if cleanup_mode was CLEANUP_ALL and there is no editing going\n> on?\n\nThe cleanup_mode initialization rewrite took care of this as well\n\n> > @@ -813,7 +846,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> >  \tif (p != NULL)\n> >  \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n> >  \n> > -\tstripspace(&sb, 1);\n> > +\tif (cleanup_mode != CLEANUP_NONE)\n> > +\t\tstripspace(&sb, cleanup_mode == CLEANUP_DEFAULT ?\n> > +\t\t\t   !no_edit: cleanup_mode == CLEANUP_ALL);\n> \n> When writing such a long ? : expression, laying it out like:\n> \n> \tcondition\n>         ? if-true\n>         : if-false\n> \n> would be easier to read.  You tilt your head sideways 90\n> degrees, just like you read a smiley ;-), and you will see the\n> parse tree.\n\nI just tried it. Still don't know if I like it :)\nSadly, the mentioned rewrite removed the need for it.\n\n> But I suspect, assuming that the two issues around CLEANUP_DEFAULT vs\n> CLEANUP_ALL I mentioned above are indeed bugs, it may be cleaner\n> to:\n> \n>   - parse the options; you get one of NONE/DEFAULT/SPACE/ALL\n> \n>   - convert DEFAULT to SPACE or ALL if editor is in effect.\n> \n>   - do not worry about no_editor or DEFAULT in the rest of the\n>     code when calling stripspace().  The only values you will\n>     see are NONE, SPACE or ALL.\n\nWell, why didn't you said this at start?! :)\n\n> > +test_expect_success 'verbatim commit messages' '\n> > +\n> \n> These tests check too many things at once, and it would be very\n> hard to diagnose when breakage does happen.  Please split them\n> perhaps into three separate tests like this:\n\nDone\n\n> > +'\n> \n> Also, the tests do not check \"-F file -e\", \"-m msg -e\" etc.  We\n> would want to make sure the right insn are shown to the user in\n> the editor, wouldn't we?\n\nAs I am not convinced whether they are needed at all, I just added a\nplaceholder which expects what we have today.\n\n Documentation/git-commit.txt |   12 ++++++-\n builtin-commit.c             |   72 ++++++++++++++++++++++++++++++-----------\n t/t7502-commit.sh            |   65 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 128 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 4261384..96383b6 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n 'git-commit' [-a | --interactive] [-s] [-v] [-u]\n \t   [(-c | -C) <commit> | -F <file> | -m <msg> | --amend]\n \t   [--allow-empty] [--no-verify] [-e] [--author <author>]\n-\t   [--] [[-i | -o ]<file>...]\n+\t   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]\n \n DESCRIPTION\n -----------\n@@ -95,6 +95,16 @@ OPTIONS\n \tfrom making such a commit.  This option bypasses the safety, and\n \tis primarily for use by foreign scm interface scripts.\n \n+--cleanup=<mode>::\n+\tThis option sets how the commit message is cleaned up.\n+\tThe  '<mode>' can be one of 'verbatim', 'whitespace', 'strip',\n+\tand 'default'. The 'default' mode will strip leading and\n+\ttrailing empty lines and #commentary from the commit message\n+\tonly if the message is to be edited. Otherwise only whitespace\n+\tremoved. The 'verbatim' mode does not change message at all,\n+\t'whitespace' removes just leading/trailing whitespace lines\n+\tand 'strip' removes both whitespace and commentary.\n+\n -e|--edit::\n \tThe message taken from file with `-F`, command line with\n \t`-m`, and from file with `-C` are usually used as the\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 96410de..34c5fd2 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -47,6 +47,19 @@ static char *logfile, *force_author, *template_file;\n static char *edit_message, *use_message;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n static int quiet, verbose, untracked_files, no_verify, allow_empty;\n+/*\n+ * The default commit message cleanup mode will remove the lines\n+ * beginning with # (shell comments) and leading and trailing\n+ * whitespaces (empty lines or containing only whitespaces)\n+ * if editor is used, and only the whitespaces if the message\n+ * is specified explicitly.\n+ */\n+static enum {\n+\tCLEANUP_SPACE,\n+\tCLEANUP_NONE,\n+\tCLEANUP_ALL,\n+} cleanup_mode;\n+static char *cleanup_arg;\n \n static int no_edit, initial_commit, in_merge;\n const char *only_include_assumed;\n@@ -88,6 +101,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN(0, \"amend\", &amend, \"amend previous commit\"),\n \tOPT_BOOLEAN(0, \"untracked-files\", &untracked_files, \"show all untracked files\"),\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n+\tOPT_STRING(0, \"cleanup\", &cleanup_arg, \"default\", \"how to strip spaces and #comments from message\"),\n \n \tOPT_END()\n };\n@@ -346,7 +360,8 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \tif (fp == NULL)\n \t\tdie(\"could not open %s\", git_path(commit_editmsg));\n \n-\tstripspace(&sb, 0);\n+\tif (cleanup_mode != CLEANUP_NONE)\n+\t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n \t\tstruct strbuf sob;\n@@ -398,23 +413,26 @@ static int prepare_log_message(const char *index_file, const char *prefix)\n \t\treturn !!DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n \t}\n \n-\tif (in_merge && !no_edit)\n-\t\tfprintf(fp,\n-\t\t\t\"#\\n\"\n-\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n-\t\t\t\"# If this is not correct, please remove the file\\n\"\n-\t\t\t\"#\t%s\\n\"\n-\t\t\t\"# and try again.\\n\"\n-\t\t\t\"#\\n\",\n-\t\t\tgit_path(\"MERGE_HEAD\"));\n-\n-\tfprintf(fp,\n-\t\t\"\\n\"\n-\t\t\"# Please enter the commit message for your changes.\\n\"\n-\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n-\tif (only_include_assumed)\n-\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n-\n+\tif (!no_edit) {\n+\t\tif (in_merge)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"#\\n\"\n+\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\t\"#\t%s\\n\"\n+\t\t\t\t\"# and try again.\\n\"\n+\t\t\t\t\"#\\n\",\n+\t\t\t\tgit_path(\"MERGE_HEAD\"));\n+\t\tif (cleanup_mode != CLEANUP_NONE)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"\\n\"\n+\t\t\t\t\"# Please enter the commit message for your changes.\\n\");\n+\t\tif (cleanup_mode == CLEANUP_ALL)\n+\t\t\tfprintf(fp,\n+\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\t\tif (only_include_assumed)\n+\t\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n+\t}\n \tsaved_color_setting = wt_status_use_color;\n \twt_status_use_color = 0;\n \tcommitable = run_status(fp, index_file, prefix, 1);\n@@ -435,10 +453,13 @@ static int message_is_empty(struct strbuf *sb, int start)\n \tconst char *nl;\n \tint eol, i;\n \n+\tif (cleanup_mode == CLEANUP_NONE && sb->len)\n+\t\treturn 0;\n+\n \t/* See if the template is just a prefix of the message. */\n \tstrbuf_init(&tmpl, 0);\n \tif (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) {\n-\t\tstripspace(&tmpl, 1);\n+\t\tstripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n \t\tif (start + tmpl.len <= sb->len &&\n \t\t    memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)\n \t\t\tstart += tmpl.len;\n@@ -591,6 +612,16 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tonly_include_assumed = \"Explicit paths specified without -i nor -o; assuming --only paths...\";\n \t\talso = 0;\n \t}\n+\tif (!cleanup_arg || !strcmp(cleanup_arg, \"default\"))\n+\t\tcleanup_mode = no_edit ? CLEANUP_SPACE: CLEANUP_ALL;\n+\telse if (!strcmp(cleanup_arg, \"verbatim\"))\n+\t\tcleanup_mode = CLEANUP_NONE;\n+\telse if (!strcmp(cleanup_arg, \"whitespace\"))\n+\t\tcleanup_mode = CLEANUP_SPACE;\n+\telse if (!strcmp(cleanup_arg, \"strip\"))\n+\t\tcleanup_mode = CLEANUP_ALL;\n+\telse\n+\t\tdie(\"Invalid cleanup mode %s\", cleanup_arg);\n \n \tif (all && argc > 0)\n \t\tdie(\"Paths with -a does not make sense.\");\n@@ -817,7 +848,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (p != NULL)\n \t\tstrbuf_setlen(&sb, p - sb.buf + 1);\n \n-\tstripspace(&sb, 1);\n+\tif (cleanup_mode != CLEANUP_NONE)\n+\t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\n \tif (sb.len < header_len || message_is_empty(&sb, header_len)) {\n \t\trollback_index_files();\n \t\tdie(\"no commit message?  aborting commit.\");\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex 21ac785..aaf497e 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -89,4 +89,69 @@ test_expect_success 'verbose' '\n \n '\n \n+test_expect_success 'cleanup commit messages (verbatim,-t)' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >expect &&\n+\tgit commit --cleanup=verbatim -t expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\" |head -n 3 >actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages (verbatim,-F)' '\n+\n+\techo >>negative &&\n+\tgit commit --cleanup=verbatim -F expect -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages (verbatim,-m)' '\n+\n+\techo >>negative &&\n+\tgit commit --cleanup=verbatim -m \"$(cat expect)\" -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages (whitespace,-F)' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo; } >text &&\n+\techo \"# text\" >expect &&\n+\tgit commit --cleanup=whitespace -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+test_expect_success 'cleanup commit messages (strip,-F)' '\n+\n+\techo >>negative &&\n+\t{ echo;echo \"# text\";echo sample;echo; } >text &&\n+\techo sample >expect &&\n+\tgit commit --cleanup=strip -F text -a &&\n+\tgit cat-file -p HEAD |sed -e \"1,/^\\$/d\">actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n+echo \"sample\n+\n+# Please enter the commit message for your changes.\n+# (Comment lines starting with '#' will not be included)\" >expect\n+\n+test_expect_success 'cleanup commit messages (strip,-F,-e)' '\n+\n+\techo >>negative &&\n+\t{ echo;echo sample;echo; } >text &&\n+\tgit commit -e -F text -a &&\n+\thead -n 4 .git/COMMIT_EDITMSG >actual &&\n+\tdiff -u expect actual\n+\n+'\n+\n test_done\n-- \n1.5.4.rc1.48.gb232\n"},{"id":"64015","messageId":"alpine.LFD.0.9999.0712221107240.21557@woody.linux-foundation.org","threadId":"11375","inReplyTo":"20071222184624.GA4167@steel.home","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-12-22T19:18:34Z","receivedAt":"2007-12-22T19:18:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 22 Dec 2007, Alex Riesen wrote:\n>\n> +\tif (!no_edit) {\n\nThis is unrelated to the rest of the patch, but I do think double \nnegations are horrible, so I thoink we should probably make the \"no_edit\" \nflag change meaning, and call it \"run_editor\" or something.\n\nThat said, the thing that I'm actually reacting to is:\n\n> +\t\tif (in_merge)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"#\\n\"\n> +\t\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n> +\t\t\t\t\"# If this is not correct, please remove the file\\n\"\n> +\t\t\t\t\"#\t%s\\n\"\n> +\t\t\t\t\"# and try again.\\n\"\n> +\t\t\t\t\"#\\n\",\n> +\t\t\t\tgit_path(\"MERGE_HEAD\"));\n> +\t\tif (cleanup_mode != CLEANUP_NONE)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"\\n\"\n> +\t\t\t\t\"# Please enter the commit message for your changes.\\n\");\n> +\t\tif (cleanup_mode == CLEANUP_ALL)\n> +\t\t\tfprintf(fp,\n> +\t\t\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n> +\t\tif (only_include_assumed)\n> +\t\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n\nThe thing that still worries me about this set is that if you have \n\"cleanup_mode == CLEANUP_NONE\" or \"cleanup_mode == CLEANUP_SPACE\", we're \nnow adding all these messages, and we depend on the user knowing that *he* \nhas to remove them.\n\nSo I wonder if we should perhaps:\n\n (a) Only add these messages at all when we do *not* do CLEANUP_ALL\n\n     In other words, change the\n\n\t\tif (!no_edit) {\n\n     line to a\n\n\t\tif (cleanup_mode == CLEANUP_ALL && !no_edit) {\n\n     instead, or\n\n (b) Add a a new line to replace he \"will not be included\" message, ie\n\n\t\tif (cleanup_mode != CLEANUP_ALL)\n\t\t\tfprintf(p,\n\t\t\t\t\"# Please remove these comment lines manually,  \"\n\t\t\t\t\"they will not be automatically removed\\n\");\n\n     or something similar.\n\nI personally would prefer (a) - since anybody who then explicitly uses\n--cleanup={space|none} would presumably already know what he is doing.\n\nBut this is not a huge deal. Regardless, the patch looks ok, so you can \nadd a \"Acked-by:\" from me.\n\n\t\tLinus\n"},{"id":"64016","messageId":"7vprwywm9c.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712221107240.21557@woody.linux-foundation.org","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-22T19:41:03Z","receivedAt":"2007-12-22T19:41:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 22 Dec 2007, Alex Riesen wrote:\n>>\n>> +\tif (!no_edit) {\n>\n> This is unrelated to the rest of the patch, but I do think double \n> negations are horrible, so I thoink we should probably make the \"no_edit\" \n> flag change meaning, and call it \"run_editor\" or something.\n> ...\n> So I wonder if we should perhaps:\n>\n>  (a) Only add these messages at all when we do *not* do CLEANUP_ALL\n> ...\n>  (b) Add a a new line to replace he \"will not be included\" message, ie\n> ...\n> I personally would prefer (a) - since anybody who then explicitly uses\n> --cleanup={space|none} would presumably already know what he is doing.\n>\n> But this is not a huge deal. Regardless, the patch looks ok, so you can \n> add a \"Acked-by:\" from me.\n\nI was composing essentially the same message, except my\npreference was (as you can guess by the fact that I hinted the\nadditional instruction) (b), but I agree that (a) is better at\nleast for now because the user has to ask for verbatim every\ntime (i.e. there is no config).\n\nBy the way, the \"if (!no_edit)\" conditional itself you quoted\nabove as the first thing in your message is not needed at all.\nIf no_edit is set, the function already has returned and we\nwould not reach that point.\n\nSo a third round?\n"},{"id":"64023","messageId":"7v1w9ew1vm.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"alpine.LFD.0.9999.0712221107240.21557@woody.linux-foundation.org","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-23T03:01:17Z","receivedAt":"2007-12-23T03:01:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> So I wonder if we should perhaps:\n>\n>  (a) Only add these messages at all when we do *not* do CLEANUP_ALL\n>\n> I personally would prefer (a) - since anybody who then explicitly uses\n> --cleanup={space|none} would presumably already know what he is doing.\n\nI thought about this a bit more, but then realized that people\nwould still want to see the status output in their editors.\nThis applies even to \"verbatim\" case.\n\nWhich means that they need to remove the '#' lines when they do\nnot use CLEANUP_ALL.\n\nSo I am inclined to suggest we need to take (b).\n"},{"id":"64024","messageId":"7vodciul7l.fsf@gitster.siamese.dyndns.org","threadId":"11375","inReplyTo":"7v1w9ew1vm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow selection of different cleanup modes for commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-23T03:46:38Z","receivedAt":"2007-12-23T03:46:38Z","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> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> So I wonder if we should perhaps:\n>>\n>>  (a) Only add these messages at all when we do *not* do CLEANUP_ALL\n>>\n>> I personally would prefer (a) - since anybody who then explicitly uses\n>> --cleanup={space|none} would presumably already know what he is doing.\n>\n> I thought about this a bit more, but then realized that people\n> would still want to see the status output in their editors.\n> This applies even to \"verbatim\" case.\n>\n> Which means that they need to remove the '#' lines when they do\n> not use CLEANUP_ALL.\n>\n> So I am inclined to suggest we need to take (b).\n\n... and I ended up doing the clean-up myself as I found a\nhandful buglets in the code before Alex's patch.\n\nWill be posting them shortly.\n"}]}