* [PATCH] git-commit doc: say -t requires editing commit message @ 2012-03-29 17:57 Adam Monsen 2012-03-29 18:09 ` Ivan Heffner 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-29 17:57 UTC (permalink / raw) To: git; +Cc: Adam Monsen Make it clear that, when using a commit template, the message *must* be changed or the commit will be aborted "due to empty commit message". Signed-off-by: Adam Monsen <haircut@gmail.com> --- I found it confusing that the commit template itself, even if non-empty, must be edited. Hopefully this clears that up a bit. Documentation/git-commit.txt | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 5cc84a1..44947ab 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -133,7 +133,7 @@ OPTIONS -t <file>:: --template=<file>:: Use the contents of the given file as the initial version - of the commit message. The editor is invoked and you can + of the commit message. The editor is invoked and you must make subsequent changes. If a message is specified using the `-m` or `-F` options, this option has no effect. This overrides the `commit.template` configuration variable. -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] git-commit doc: say -t requires editing commit message 2012-03-29 17:57 [PATCH] git-commit doc: say -t requires editing commit message Adam Monsen @ 2012-03-29 18:09 ` Ivan Heffner 2012-03-29 23:04 ` [PATCH v2] git-commit.txt: clarify -t requires editing message Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Ivan Heffner @ 2012-03-29 18:09 UTC (permalink / raw) To: Adam Monsen; +Cc: git I'd suggest being much more verbose about what's going on and why. diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 5cc84a1..e842916 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -133,10 +133,12 @@ OPTIONS -t <file>:: --template=<file>:: Use the contents of the given file as the initial version - of the commit message. The editor is invoked and you can - make subsequent changes. If a message is specified using - the `-m` or `-F` options, this option has no effect. This - overrides the `commit.template` configuration variable. + of the commit message. The editor is invoked so you can + make changes. If a message is specified using the `-m` or `-F` + options, this option has no effect. This overrides the + `commit.template` configuration variable. If the message is + unchanged, the message is considered to be empty and the commit is + aborted -s:: --signoff:: -- 1.7.6.553.g917d7.dirty On Thu, Mar 29, 2012 at 10:57 AM, Adam Monsen <haircut@gmail.com> wrote: > > Make it clear that, when using a commit template, the message *must* be > changed or the commit will be aborted "due to empty commit message". > > Signed-off-by: Adam Monsen <haircut@gmail.com> > --- > > I found it confusing that the commit template itself, even if > non-empty, must be edited. Hopefully this clears that up a bit. > > Documentation/git-commit.txt | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index 5cc84a1..44947ab 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -133,7 +133,7 @@ OPTIONS > -t <file>:: > --template=<file>:: > Use the contents of the given file as the initial version > - of the commit message. The editor is invoked and you can > + of the commit message. The editor is invoked and you must > make subsequent changes. If a message is specified using > the `-m` or `-F` options, this option has no effect. This > overrides the `commit.template` configuration variable. > -- > 1.7.5.4 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-29 18:09 ` Ivan Heffner @ 2012-03-29 23:04 ` Adam Monsen 2012-03-30 2:05 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-29 23:04 UTC (permalink / raw) To: git; +Cc: Ivan Heffner, Adam Monsen Make it clear that, when using a commit template, the message *must* be changed or the commit will be aborted "due to empty commit message". Helped-by: Ivan Heffner <iheffner@gmail.com> Signed-off-by: Adam Monsen <haircut@gmail.com> --- Incorporate feedback from Ivan Heffner. Anyone else agree/disagree with this patch? I would like to see the manpage improved because I found the behavior of "git commit -t" confusing as documented. I wrapped the text at 77 characters because that was the longest line in the file (according to wc -L). I used ":set noet nosta ts=8 sw=8 tw=77" in Vim. Documentation/git-commit.txt | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 5cc84a1..c6df120 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -133,10 +133,12 @@ OPTIONS -t <file>:: --template=<file>:: Use the contents of the given file as the initial version - of the commit message. The editor is invoked and you can - make subsequent changes. If a message is specified using - the `-m` or `-F` options, this option has no effect. This - overrides the `commit.template` configuration variable. + of the commit message. The editor is invoked so you can + make subsequent changes. If you make no changes, the message + is considered empty and the commit is aborted. If a message + is specified using the `-m` or `-F` options, this option has + no effect. This overrides the `commit.template` + configuration variable. -s:: --signoff:: -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-29 23:04 ` [PATCH v2] git-commit.txt: clarify -t requires editing message Adam Monsen @ 2012-03-30 2:05 ` Junio C Hamano 2012-03-30 3:07 ` Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 2:05 UTC (permalink / raw) To: Adam Monsen; +Cc: git, Ivan Heffner Adam Monsen <haircut@gmail.com> writes: > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index 5cc84a1..c6df120 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -133,10 +133,12 @@ OPTIONS > -t <file>:: > --template=<file>:: > Use the contents of the given file as the initial version > + of the commit message. The editor is invoked so you can > + make subsequent changes. If you make no changes, the message > + is considered empty and the commit is aborted. If a message > + is specified using the `-m` or `-F` options, this option has > + no effect. This overrides the `commit.template` > + configuration variable. First, think of template not as the "initial version" but as "a form that needs to be filled", and imagine that you are explaining to a beginner how to create a commit. The word you would choose to use would be very different if you rephrase the above after doing that mental exercise, and I suspect that it would become much easier to read. For example: - subsequent changes --> fill in the form - If ... considerede empty and --> If you did not fill the form ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 2:05 ` Junio C Hamano @ 2012-03-30 3:07 ` Adam Monsen 2012-03-30 3:52 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-30 3:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ivan Heffner [-- Attachment #1: Type: text/plain, Size: 1405 bytes --] Junio C. Hamano wrote: > First, think of template not as the "initial version" but as "a form > that needs to be filled" Ah, that clarifies for me what the authors had in mind when implementing this feature. I was hoping to use this feature to pre-populate the commit message in my editor and have the option to edit it or leave it as-is. This is what the word "template" means for me. For example: if I open a "template" in LibreOffice, I can make changes or not, and LibreOffice still lets me print or save at any point. Now that I know it's not that at all, but rather something more like a mandatory fillable form, I'll find another way to achieve my need. Since git really wants me to enter *something* for a commit message, I understand why git "templates" work differently than LibreOffice "templates". It's getting a little wordy, but here's an attempt to work in the "form" concept: ~~~ Use the contents of the given file as the initial version of the commit message. Think of this initial version as a mandatory fillable form. The editor is invoked so you can fill in the form. If you do not fill in the form (if you make no changes), the message is considered empty and the commit is aborted. If a message is specified using the `-m` or `-F` options, this option has no effect. This overrides the `commit.template` configuration variable. ~~~ Thoughts? [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 3:07 ` Adam Monsen @ 2012-03-30 3:52 ` Junio C Hamano 2012-03-30 4:53 ` Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 3:52 UTC (permalink / raw) To: Adam Monsen; +Cc: git, Ivan Heffner Adam Monsen <haircut@gmail.com> writes: > ~~~ > Use the contents of the given file as the initial version of the > commit message. Think of this initial version as a mandatory > fillable form. The editor is invoked so you can fill in the form. If > you do not fill in the form (if you make no changes), the message is > considered empty and the commit is aborted. If a message is > specified using the `-m` or `-F` options, this option has no effect. > This overrides the `commit.template` configuration variable. > ~~~ > > Thoughts? You still say "the message is considered empty and" but I think it probably reads better without it. Strictly speaking, it is not a "mandatory fillable form", but whatever text you put in the template is advisory to the users. For example, if your project wants its contributor to always refer to a bug id in its issue tracker, it may want to give a customized "template", instead of the plain "template" we give to the users that begins with: ~~~~~~~~ # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. ~~~~~~~~ to guide them what to write in the log and how to explain your change, e.g. something like: ~~~~~~~~ <<one line summary your change here>> # explain the problem your change tries to solve in the first # paragraph # describe the approach your solution takes to solve it in the # second and subsequent paragraphs # Please write issue tracker ID at the end, if available Frotz-Bug-Id: XXXXXX # Always sign-off your commit Signed-off-by: XXXXXX ~~~~~~~~ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 3:52 ` Junio C Hamano @ 2012-03-30 4:53 ` Adam Monsen 2012-03-30 5:08 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-30 4:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ivan Heffner -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Junio C Hamano wrote: > You still say "the message is considered empty and" but I think it > probably reads better without it. Do you like the patch without those words? > Strictly speaking, it is not a "mandatory fillable form", but > whatever text you put in the template is advisory to the users. Ok, right on. I understand the template feature now since you've patiently explained it to me (thank you!). I still want to plainly convey that even if my template is the following: ~~~~ zip module: continue to expand transformer This WIP will eventually provide expanded transformer functionality. ~~~~ I *cannot* just save and quit my editor (unless I supply - --allow-empty-message). That's the behavior I find confusing: git telling me a non-empty commit message is an empty commit message. If I save that text above (zip module... etc) in FILE and do `git commit -t FILE`, save and quit my editor, git says "Aborting commit due to empty commit message." Lies! A more precise message would be "Aborting commit due to unmodified commit message template." Based on the current documentation I misunderstood that -t could be used to review a boilerplate commit message and save it verbatim. ...AHA! I just figured out a way to do exactly that: git commit --edit --file=FILE aka git commit -eF FILE Yay! No idea how I missed that before. Anyway, it still seems like the documentation for "git commit -t" could use improvement. I actually wouldn't mind seeing a short example template like the one you provided, maybe in the EXAMPLES or DISCUSSION section. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQEcBAEBAgAGBQJPdTw/AAoJEJtdmT+DbynAte4IALRx50yW5bEfzXskvSDewPuL SgaU4CqUHRm8sWXHeFbd4I2rG4dEJuqYqzKKbfay3EMwEbIkThiwoC2pJ9xJoFpe 8O95GVp3ikYvsY3mn87ebiwA9FBhnTy1Fz+MREfuzETpJbdtJSHhbRXMxfJ9ZabU FOPE/qeZDvQJA9b9QFY3QS/BcxsGHXhW9xCULZlAprDggMcchhHDEbqJCh/1wObw cQvoONiqZSkXA17K3gxfs7NgafUVFIg3+N9vcq90eZXbT/s1MM+1zxj5ezTh9jbV sOkzepfE5+NBK3PnewMDxDxhF0LD5lzHCwnfkTl1Om3okSE0nxVyRZKabuzc99s= =NUN8 -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 4:53 ` Adam Monsen @ 2012-03-30 5:08 ` Junio C Hamano 2012-03-30 5:43 ` Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 5:08 UTC (permalink / raw) To: Adam Monsen; +Cc: git, Ivan Heffner Adam Monsen <haircut@gmail.com> writes: > I *cannot* just save and quit my editor (unless I supply > --allow-empty-message). That's the behavior I find confusing... The root cause of the confusion is that the entire "--template" code piggy backs on the non-templated case, where we would want to error out if the user leaves the editor without explaining the commit. And an appropriate diagnosis message for the "normal" case is "you gave me an empty message", but "--template" code did not bother updating that to suit what it tries to do better, e.g. "you did not edit the template I gave you". The check to see if the message the user left matches that came from the template was tacked into a wrong function message_is_empty(); it should be made into its own helper function and called from the same caller where it calls message_is_empty() only when template_file is not NULL. Also its honoring "--allow-empty" needs to be reconsidered. In other words, I think that is something that needs fixing the broken code to behave less confusingly, not documenting its wrong behaviour. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 5:08 ` Junio C Hamano @ 2012-03-30 5:43 ` Adam Monsen 2012-03-30 18:17 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-30 5:43 UTC (permalink / raw) To: Junio C Hamano, Ivan Heffner; +Cc: git [-- Attachment #1: Type: text/plain, Size: 277 bytes --] Junio C Hamano wrote: > I think that is something that needs fixing the broken code to behave > less confusingly, not documenting its wrong behaviour. Excellent! I concur. I wish I wanted to do this enough to make time to work on it. Ivan, how are your C chops? :) [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2] git-commit.txt: clarify -t requires editing message 2012-03-30 5:43 ` Adam Monsen @ 2012-03-30 18:17 ` Junio C Hamano 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 18:17 UTC (permalink / raw) To: Adam Monsen; +Cc: Ivan Heffner, git Adam Monsen <haircut@gmail.com> writes: > Junio C Hamano wrote: >> I think that is something that needs fixing the broken code to behave >> less confusingly, not documenting its wrong behaviour. > > Excellent! I concur. > > I wish I wanted to do this enough to make time to work on it. Ivan, how > are your C chops? :) Don't worry. While looking around the vicinity of the codepath, I noticed a few more bugs there, so I'll post something today. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 0/3] "commit --template" fixes 2012-03-30 18:17 ` Junio C Hamano @ 2012-03-30 19:45 ` Junio C Hamano 2012-03-30 19:45 ` [PATCH 1/3] t7501: test the right kind of breakage Junio C Hamano ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 19:45 UTC (permalink / raw) To: git; +Cc: Adam Monsen When the user exited editor without editing the commit log template given by "git commit -t <template>", the commit was aborted (correct) with an error message that said "due to empty commit message" (incorrect). The goal of this series is to fix this, which is the third patch. While looking at this, I found another bug that the contents of the template file is still used for error checking even when it is ignored when the editor is populated for the user to edit. The second patch addresses this. Junio C Hamano (3): t7501: test the right kind of breakage commit: do not trigger bogus "has templated message edited" check commit: rephrase the error when user did not touch templated log message builtin/commit.c | 62 +++++++++++++++++++++++++++++++++++++---------------- t/t7501-commit.sh | 14 ++++++++++++ 2 files changed, 57 insertions(+), 19 deletions(-) -- 1.7.10.rc3.55.g06e99 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] t7501: test the right kind of breakage 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano @ 2012-03-30 19:45 ` Junio C Hamano 2012-03-30 19:45 ` [PATCH 2/3] commit: do not trigger bogus "has templated message edited" check Junio C Hamano ` (2 subsequent siblings) 3 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 19:45 UTC (permalink / raw) To: git; +Cc: Adam Monsen These tests try to run "git commit" with various "forbidden" combinations of options and expect the command to fail, but they do so without having any change added to the index. We wouldn't be able to catch breakages that would allow these combinations by mistake with them because the command will fail with "nothing to commit" anyway. Make sure we have something added to the index before running the command. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t7501-commit.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 8bb3833..45446b1 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -30,10 +30,12 @@ test_expect_success 'setup: initial commit' ' ' test_expect_success '-m and -F do not mix' ' + git checkout HEAD file && echo >>file && git add file && test_must_fail git commit -m foo -m bar -F file ' test_expect_success '-m and -C do not mix' ' + git checkout HEAD file && echo >>file && git add file && test_must_fail git commit -C HEAD -m illegal ' -- 1.7.10.rc3.55.g06e99 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/3] commit: do not trigger bogus "has templated message edited" check 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano 2012-03-30 19:45 ` [PATCH 1/3] t7501: test the right kind of breakage Junio C Hamano @ 2012-03-30 19:45 ` Junio C Hamano 2012-03-30 19:45 ` [PATCH 3/3] commit: rephrase the error when user did not touch templated log message Junio C Hamano 2012-03-31 19:28 ` [PATCH 0/3] "commit --template" fixes Adam Monsen 3 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 19:45 UTC (permalink / raw) To: git; +Cc: Adam Monsen When "-t template" and "-F msg" options are both given (or worse yet, there is "commit.template" configuration but a message is given in some other way), the documentation says that template is ignored. However, the "has the user edited the message?" check still used the contents of the template file as the basis of the emptyness check. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/commit.c | 2 ++ t/t7501-commit.sh | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index eba1377..7141766 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1049,6 +1049,8 @@ static int parse_and_validate_options(int argc, const char *argv[], die(_("Only one of -c/-C/-F/--fixup can be used.")); if (message.len && f > 0) die((_("Option -m cannot be combined with -c/-C/-F/--fixup."))); + if (f || message.len) + template_file = NULL; if (edit_message) use_message = edit_message; if (amend && !use_message && !fixup_message) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 45446b1..e59cc4e 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -81,7 +81,13 @@ test_expect_success 'empty commit message' ' test_must_fail git commit -F msg -a ' +test_expect_success 'template "emptyness" check does not kick in with -F' ' + git checkout HEAD file && echo >>file && git add file && + git commit -t file -F file +' + test_expect_success 'setup: commit message from file' ' + git checkout HEAD file && echo >>file && git add file && echo this is the commit message, coming from a file >msg && git commit -F msg -a ' -- 1.7.10.rc3.55.g06e99 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] commit: rephrase the error when user did not touch templated log message 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano 2012-03-30 19:45 ` [PATCH 1/3] t7501: test the right kind of breakage Junio C Hamano 2012-03-30 19:45 ` [PATCH 2/3] commit: do not trigger bogus "has templated message edited" check Junio C Hamano @ 2012-03-30 19:45 ` Junio C Hamano 2012-03-31 19:28 ` [PATCH 0/3] "commit --template" fixes Adam Monsen 3 siblings, 0 replies; 19+ messages in thread From: Junio C Hamano @ 2012-03-30 19:45 UTC (permalink / raw) To: git; +Cc: Adam Monsen When the user exited editor without editing the commit log template given by "git commit -t <template>", the commit was aborted (correct) with an error message that said "due to empty commit message" (incorrect). This was because the original template support was done by piggybacking on the check to detect an empty log message. Split the codepaths into two independent checks to clarify the error. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/commit.c | 60 ++++++++++++++++++++++++++++++++++++----------------- t/t7501-commit.sh | 6 ++++++ 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 7141766..847d363 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -899,27 +899,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 1; } -/* - * Find out if the message in the strbuf contains only whitespace and - * Signed-off-by lines. - */ -static int message_is_empty(struct strbuf *sb) +static int rest_is_empty(struct strbuf *sb, int start) { - struct strbuf tmpl = STRBUF_INIT; + int i, eol; const char *nl; - int eol, i, start = 0; - - if (cleanup_mode == CLEANUP_NONE && sb->len) - return 0; - - /* See if the template is just a prefix of the message. */ - if (template_file && strbuf_read_file(&tmpl, template_file, 0) > 0) { - stripspace(&tmpl, cleanup_mode == CLEANUP_ALL); - if (start + tmpl.len <= sb->len && - memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0) - start += tmpl.len; - } - strbuf_release(&tmpl); /* Check if the rest is just whitespace and Signed-of-by's. */ for (i = start; i < sb->len; i++) { @@ -942,6 +925,40 @@ static int message_is_empty(struct strbuf *sb) return 1; } +/* + * Find out if the message in the strbuf contains only whitespace and + * Signed-off-by lines. + */ +static int message_is_empty(struct strbuf *sb) +{ + if (cleanup_mode == CLEANUP_NONE && sb->len) + return 0; + return rest_is_empty(sb, 0); +} + +/* + * See if the user edited the message in the editor or left what + * was in the template intact + */ +static int template_untouched(struct strbuf *sb) +{ + struct strbuf tmpl = STRBUF_INIT; + char *start; + + if (cleanup_mode == CLEANUP_NONE && sb->len) + return 0; + + if (!template_file || strbuf_read_file(&tmpl, template_file, 0) <= 0) + return 0; + + stripspace(&tmpl, cleanup_mode == CLEANUP_ALL); + start = (char *)skip_prefix(sb->buf, tmpl.buf); + if (!start) + start = sb->buf; + strbuf_release(&tmpl); + return rest_is_empty(sb, start - sb->buf); +} + static const char *find_author_by_nickname(const char *name) { struct rev_info revs; @@ -1490,6 +1507,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix) if (cleanup_mode != CLEANUP_NONE) stripspace(&sb, cleanup_mode == CLEANUP_ALL); + if (template_untouched(&sb) && !allow_empty_message) { + rollback_index_files(); + fprintf(stderr, _("Aborting commit; you did not edit the message.\n")); + exit(1); + } if (message_is_empty(&sb) && !allow_empty_message) { rollback_index_files(); fprintf(stderr, _("Aborting commit due to empty commit message.\n")); diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index e59cc4e..b20ca0e 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -86,6 +86,12 @@ test_expect_success 'template "emptyness" check does not kick in with -F' ' git commit -t file -F file ' +test_expect_success 'template "emptyness" check' ' + git checkout HEAD file && echo >>file && git add file && + test_must_fail git commit -t file 2>err && + test_i18ngrep "did not edit" err +' + test_expect_success 'setup: commit message from file' ' git checkout HEAD file && echo >>file && git add file && echo this is the commit message, coming from a file >msg && -- 1.7.10.rc3.55.g06e99 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] "commit --template" fixes 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano ` (2 preceding siblings ...) 2012-03-30 19:45 ` [PATCH 3/3] commit: rephrase the error when user did not touch templated log message Junio C Hamano @ 2012-03-31 19:28 ` Adam Monsen 2012-04-01 22:28 ` Junio C Hamano 3 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-03-31 19:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ivan Heffner [-- Attachment #1: Type: text/plain, Size: 2950 bytes --] On 03/30/2012 12:45 PM, Junio C Hamano wrote: > When the user exited editor without editing the commit log template > given by "git commit -t <template>", the commit was aborted (correct) > with an error message that said "due to empty commit message" > (incorrect). The goal of this series is to fix this, which is the > third patch. This is awesome. thanks! I really like the new error message specific to the situation when the user does not edit the template (as we discussed). Your patches apply cleanly to maint b8939b2b3abaa. I tested the patches and they work as expected. When I use `git commit --template FILE` but do not edit the message in my editor, I get Aborting commit; you did not edit the message. Nice. Only thing I'd add is a change to the git-commit(1) manpage. * I prefer pragmatically explaining what will happen when the user uses --template but does not edit the message because it is more direct and terse (than "filling in a form"). * The below applies cleanly to maint as of today. * I don't know the kosher procedure to add this commit to your patch series for further review, so hopefully this works. * I'm not sure if the "Helped-by:" lines are kosher, I'm happy to remove them if not. From 91a62baa1fe89032e7a3598e5d39241f3eb8f84b Mon Sep 17 00:00:00 2001 From: Adam Monsen <haircut@gmail.com> Date: Sat, 31 Mar 2012 12:09:29 -0700 Subject: [PATCH] git-commit.txt: clarify -t requires editing message Make it clear that, when using commit --template, the message *must* be changed or the commit will be aborted. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Ivan Heffner <iheffner@gmail.com> Signed-off-by: Adam Monsen <haircut@gmail.com> --- I wrapped the text at 77 characters because that was the longest line in the file (according to wc -L). I used ":set noet nosta ts=8 sw=8 tw=77" in Vim. Documentation/git-commit.txt | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 5cc84a1..f584a62 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -132,11 +132,11 @@ OPTIONS -t <file>:: --template=<file>:: - Use the contents of the given file as the initial version - of the commit message. The editor is invoked and you can - make subsequent changes. If a message is specified using - the `-m` or `-F` options, this option has no effect. This - overrides the `commit.template` configuration variable. + Use the contents of the given file as the initial version of the + commit message. The editor is invoked so you can make subsequent + changes. If you make no changes, the commit is aborted. If a message + is specified using the `-m` or `-F` options, this option has no + effect. This overrides the `commit.template` configuration variable. -s:: --signoff:: -- 1.7.5.4 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] "commit --template" fixes 2012-03-31 19:28 ` [PATCH 0/3] "commit --template" fixes Adam Monsen @ 2012-04-01 22:28 ` Junio C Hamano 2012-04-03 17:11 ` Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-04-01 22:28 UTC (permalink / raw) To: Adam Monsen; +Cc: git, Ivan Heffner Adam Monsen <haircut@gmail.com> writes: > ... > * I don't know the kosher procedure to add this commit to your patch > series for further review, so hopefully this works. > * I'm not sure if the "Helped-by:" lines are kosher, I'm happy to > remove them if not. One established way to do this is to have a discussion like the above (mostly elided), followed by a "scissors" line "-- >8 --", and then the output from format-patch with most headers except for "Subject: " removed (as From: and Date: will be taken from your e-mailed message anyway, and the first "From <commit object name> <magic constant date>" is a signal to allow automated tools to tell if it is a format-patch output or a random mbox file, and is not appropriate if you are sending it over e-mail). > > From 91a62baa1fe89032e7a3598e5d39241f3eb8f84b Mon Sep 17 00:00:00 2001 > From: Adam Monsen <haircut@gmail.com> > Date: Sat, 31 Mar 2012 12:09:29 -0700 > Subject: [PATCH] git-commit.txt: clarify -t requires editing message > > Make it clear that, when using commit --template, the message *must* be > changed or the commit will be aborted. > > > Helped-by: Junio C Hamano <gitster@pobox.com> > Helped-by: Ivan Heffner <iheffner@gmail.com> > Signed-off-by: Adam Monsen <haircut@gmail.com> > --- > I wrapped the text at 77 characters because that was the longest > line in the file (according to wc -L). > > I used ":set noet nosta ts=8 sw=8 tw=77" in Vim. When rewording or clarifying only a handful of words in the documentation, it is often better to avoid reflowing lines in the same patch. It makes it harder to see what you really changed, and what is merely reflowed. I'll queue it as-is, though. Thanks. This is a tangent, but we might want to rephrase the first sentence without using the word "version"; every time I read this paragraph, the "initial version" makes me go "Huh?" because the word sounds as if it is talking about commits in the context of SCM, which is not the case here. I know that the description wanted to avoid use of the word "template" to explain what the template is, but still... > Documentation/git-commit.txt | 10 +++++----- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt > index 5cc84a1..f584a62 100644 > --- a/Documentation/git-commit.txt > +++ b/Documentation/git-commit.txt > @@ -132,11 +132,11 @@ OPTIONS > > -t <file>:: > --template=<file>:: > - Use the contents of the given file as the initial version > - of the commit message. The editor is invoked and you can > - make subsequent changes. If a message is specified using > - the `-m` or `-F` options, this option has no effect. This > - overrides the `commit.template` configuration variable. > + Use the contents of the given file as the initial version of the > + commit message. The editor is invoked so you can make subsequent > + changes. If you make no changes, the commit is aborted. If a message > + is specified using the `-m` or `-F` options, this option has no > + effect. This overrides the `commit.template` configuration variable. > > -s:: > --signoff:: ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] "commit --template" fixes 2012-04-01 22:28 ` Junio C Hamano @ 2012-04-03 17:11 ` Adam Monsen 2012-04-03 21:55 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Adam Monsen @ 2012-04-03 17:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ivan Heffner On 04/01/2012 03:28 PM, Junio C Hamano wrote: > One established way to do this is to have a discussion like the above > (mostly elided), followed by a "scissors" line... I can do that. Thanks! > When rewording or clarifying only a handful of words in the documentation, > it is often better to avoid reflowing lines in the same patch. I thought of that, but it made the right margin jagged. :) My new suggestion (below) isolates the changes a bit better. > This is a tangent, but we might want to rephrase the first sentence > without using the word "version"; every time I read this paragraph, the > "initial version" makes me go "Huh?" because the word sounds as if it is > talking about commits in the context of SCM, which is not the case here. Yeah, that bugs me too. How about this? I'm a little bummed it doesn't include why commit --template exists at all, but it reads well: terse and to the point like (IMHO) a manpage should. -- >8 -- Subject: [PATCH v4] git-commit.txt: clarify -t requires editing message Make it clear that, when using commit --template, the message *must* be changed or the commit will be aborted. Also, remove the words "initial version" to avoid confusion. Commit messages are not versioned independently of commits. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Ivan Heffner <iheffner@gmail.com> Signed-off-by: Adam Monsen <haircut@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Replaces b0ad5e27803cd of jc/commit-unedited-template. I'm assuming that's ok since the branch isn't merged into maint or master. Documentation/git-commit.txt | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index 5cc84a1..bd82431 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -132,9 +132,9 @@ OPTIONS -t <file>:: --template=<file>:: - Use the contents of the given file as the initial version - of the commit message. The editor is invoked and you can - make subsequent changes. If a message is specified using + Use the contents of the given file as the commit message. The + editor is invoked so you can make subsequent changes. If you make no + changes, the commit is aborted. If a message is specified using the `-m` or `-F` options, this option has no effect. This overrides the `commit.template` configuration variable. -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] "commit --template" fixes 2012-04-03 17:11 ` Adam Monsen @ 2012-04-03 21:55 ` Junio C Hamano 2012-04-05 14:29 ` Adam Monsen 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2012-04-03 21:55 UTC (permalink / raw) To: Adam Monsen; +Cc: Junio C Hamano, git, Ivan Heffner Adam Monsen <haircut@gmail.com> writes: > How about this? I'm a little bummed it doesn't include why commit > --template exists at all, but it reads well: terse and to the point like > (IMHO) a manpage should. Perhaps we should explain why somebody might want to use --template instead of -F; personally, I do not think --template command line option would not make much sense unless it is used as a part of a script that enforces external constraints in a larger workflow, even though such a project could instead require the participants to set commit.template to one supplied by the project. As an enforcement mechanism, use of such stricter "commit wrapper" and commit.template configuration cannot be mechanical and absolute either way, as Git is distributed and whatever happens in the participant's repository is purely up to the participant. > -t <file>:: > --template=<file>:: > + Use the contents of the given file as the commit message. The > + editor is invoked so you can make subsequent changes. If you make no > + changes, the commit is aborted. If a message is specified using > the `-m` or `-F` options, this option has no effect. This > overrides the `commit.template` configuration variable. When editing the commit message, start the editor with the contents in the given file. The `commit.template` configuration variable is often used to give this option implicitly to the command. This mechanism can be used by projects that want to guide participants with some hints on what to write in the message in what order. If the user exits the editor without editing the message, the commit is aborted. This has no effect when a message is given by other means, e.g. with the `-m` or `-F` options. Hmm? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/3] "commit --template" fixes 2012-04-03 21:55 ` Junio C Hamano @ 2012-04-05 14:29 ` Adam Monsen 0 siblings, 0 replies; 19+ messages in thread From: Adam Monsen @ 2012-04-05 14:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ivan Heffner [-- Attachment #1: Type: text/plain, Size: 591 bytes --] On 04/03/2012 02:55 PM, Junio C Hamano wrote: > When editing the commit message, start the editor with the contents in the > given file. The `commit.template` configuration variable is often used to > give this option implicitly to the command. This mechanism can be used by > projects that want to guide participants with some hints on what to write > in the message in what order. If the user exits the editor without editing > the message, the commit is aborted. This has no effect when a message is > given by other means, e.g. with the `-m` or `-F` options. I like it! [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-04-05 14:30 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-03-29 17:57 [PATCH] git-commit doc: say -t requires editing commit message Adam Monsen 2012-03-29 18:09 ` Ivan Heffner 2012-03-29 23:04 ` [PATCH v2] git-commit.txt: clarify -t requires editing message Adam Monsen 2012-03-30 2:05 ` Junio C Hamano 2012-03-30 3:07 ` Adam Monsen 2012-03-30 3:52 ` Junio C Hamano 2012-03-30 4:53 ` Adam Monsen 2012-03-30 5:08 ` Junio C Hamano 2012-03-30 5:43 ` Adam Monsen 2012-03-30 18:17 ` Junio C Hamano 2012-03-30 19:45 ` [PATCH 0/3] "commit --template" fixes Junio C Hamano 2012-03-30 19:45 ` [PATCH 1/3] t7501: test the right kind of breakage Junio C Hamano 2012-03-30 19:45 ` [PATCH 2/3] commit: do not trigger bogus "has templated message edited" check Junio C Hamano 2012-03-30 19:45 ` [PATCH 3/3] commit: rephrase the error when user did not touch templated log message Junio C Hamano 2012-03-31 19:28 ` [PATCH 0/3] "commit --template" fixes Adam Monsen 2012-04-01 22:28 ` Junio C Hamano 2012-04-03 17:11 ` Adam Monsen 2012-04-03 21:55 ` Junio C Hamano 2012-04-05 14:29 ` Adam Monsen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.