patchgit-commit doc: say -t requires editing commit message

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(-)

I'd suggest being much more verbose about what's going on and why.

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(-)

Adam Monsen <haircut@gmail.com> writes:

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

Junio C. Hamano wrote:

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?

Adam Monsen <haircut@gmail.com> writes:

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
	~~~~~~~~

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Junio C Hamano wrote:

Do you like the patch without those words?

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-----

Adam Monsen <haircut@gmail.com> writes:

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.

Junio C Hamano wrote:

Excellent! I concur.

I wish I wanted to do this enough to make time to work on it. Ivan, how are your C chops? :)

Adam Monsen <haircut@gmail.com> writes:

Don't worry. While looking around the vicinity of the codepath, I noticed a few more bugs there, so I'll post something today.

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(-)

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(+)

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(+)

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(-)

On 03/30/2012 12:45 PM, Junio C Hamano wrote:

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(-)

Adam Monsen <haircut@gmail.com> writes:

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).

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...

On 04/01/2012 03:28 PM, Junio C Hamano wrote:

I can do that. Thanks!

I thought of that, but it made the right margin jagged. :) My new suggestion (below) isolates the changes a bit better.

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(-)

Adam Monsen <haircut@gmail.com> writes:

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.

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?

On 04/03/2012 02:55 PM, Junio C Hamano wrote:

I like it!