{"thread":{"id":"31059","subject":"Re: empty ident name trashes commit message","startedAt":"2012-07-21T14:26:26Z","lastAt":"2012-07-23T22:09:47Z","messageCount":16,"participants":["Ramana Kumar","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"195401","messageId":"CAMej=270FDFxyVtKTC9v4KUSkSw+TBNOg+LbGfWOgmTe_xKiRQ@mail.gmail.com","threadId":"31059","inReplyTo":"CAMej=25=xj61pc+k42dv3byuBqUJzW21Sz+BXwoufqnKwV5Bbg@mail.gmail.com","subject":"Re: empty ident name trashes commit message","fromName":"Ramana Kumar","fromEmail":"ramana@member.fsf.org","sentAt":"2012-07-21T14:26:26Z","receivedAt":"2012-07-21T14:26:26Z","isPatch":false,"sender":{"key":"ramana@member.fsf.org","avatar":null},"body":"If I forget to set user.email and user.name config options and do a commit\n(possibly the --amend option also required to make this show up), then git\n1.7.11.2 will drops me into an editor for a commit message, then after that\ncomplain with the fatal message:\n\n *** Please tell me who you are.\n\n Run\n\n git config --global user.email \"you@example.com\"\n git config --global user.name \"Your Name\"\n\n to set your account's default identity.\n Omit --global to set the identity only in this repository.\n\n fatal: empty ident name (for ) not allowed\n\nThe commit message I wrote is now lost. This is bad behaviour - the error\nshould happen before one writes the commit message, or the message should be\nsaved somewhere.\n"},{"id":"195534","messageId":"20120723172726.GA2463@sigill.intra.peff.net","threadId":"31059","inReplyTo":"CAMej=270FDFxyVtKTC9v4KUSkSw+TBNOg+LbGfWOgmTe_xKiRQ@mail.gmail.com","subject":"Re: empty ident name trashes commit message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T17:27:26Z","receivedAt":"2012-07-23T17:27:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 21, 2012 at 03:26:26PM +0100, Ramana Kumar wrote:\n\n> If I forget to set user.email and user.name config options and do a commit\n> (possibly the --amend option also required to make this show up), then git\n> 1.7.11.2 will drops me into an editor for a commit message, then after that\n> complain with the fatal message:\n> \n> *** Please tell me who you are.\n> [...]\n\nHmm. I think this is an artifact of running --amend. In the normal case,\nwe check the author ident beforehand. But in the --amend case, we take\nthe existing author, but then fail trying to generate the committer\nident. So we could probably do better by checking both explicitly\nbeforehand.\n\n> fatal: empty ident name (for ) not allowed\n\nUsually we would fall back to your name from /etc/passwd. I guess it is\nblank on your system.\n\n> The commit message I wrote is now lost. This is bad behaviour - the error\n> should happen before one writes the commit message, or the message should be\n> saved somewhere.\n\nIt's not lost. It's in .git/COMMIT_EDITMSG.\n\nWe could probably do a better job of informing the user of this when\ncommit dies prematurely.\n\n-Peff\n"},{"id":"195536","messageId":"CAMej=25rwCtY6NAJYXNw3jMZSmsZ81Ta1d11kic7CfBkO4zmaw@mail.gmail.com","threadId":"31059","inReplyTo":"20120723172726.GA2463@sigill.intra.peff.net","subject":"Re: empty ident name trashes commit message","fromName":"Ramana Kumar","fromEmail":"ramana@member.fsf.org","sentAt":"2012-07-23T18:05:10Z","receivedAt":"2012-07-23T18:05:10Z","isPatch":false,"sender":{"key":"ramana@member.fsf.org","avatar":null},"body":"On Mon, Jul 23, 2012 at 6:27 PM, Jeff King wrote:\n> On Sat, Jul 21, 2012 at 03:26:26PM +0100, Ramana Kumar wrote:\n>\n>> If I forget to set user.email and user.name config options and do a commit\n>> (possibly the --amend option also required to make this show up), then git\n>> 1.7.11.2 will drops me into an editor for a commit message, then after that\n>> complain with the fatal message:\n>>\n>> *** Please tell me who you are.\n>> [...]\n>\n> Hmm. I think this is an artifact of running --amend. In the normal case,\n> we check the author ident beforehand. But in the --amend case, we take\n> the existing author, but then fail trying to generate the committer\n> ident. So we could probably do better by checking both explicitly\n> beforehand.\n\nIndeed.\n\n> Usually we would fall back to your name from /etc/passwd. I guess it is\n> blank on your system.\n>\n>> The commit message I wrote is now lost. [...]\n>\n> It's not lost. It's in .git/COMMIT_EDITMSG.\n>\n> We could probably do a better job of informing the user of this when\n> commit dies prematurely.\n>\n> -Peff\n\nI agree, and thank you very much for those two useful pieces of\ninformation! (names stored in /etc/passwd and saving of\n.git/COMMIT_EDITMSG).\n"},{"id":"195548","messageId":"20120723184634.GA31905@sigill.intra.peff.net","threadId":"31059","inReplyTo":"20120723172726.GA2463@sigill.intra.peff.net","subject":"Re: empty ident name trashes commit message","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T18:46:34Z","receivedAt":"2012-07-23T18:46:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 23, 2012 at 01:27:26PM -0400, Jeff King wrote:\n\n> On Sat, Jul 21, 2012 at 03:26:26PM +0100, Ramana Kumar wrote:\n> \n> > If I forget to set user.email and user.name config options and do a commit\n> > (possibly the --amend option also required to make this show up), then git\n> > 1.7.11.2 will drops me into an editor for a commit message, then after that\n> > complain with the fatal message:\n> > \n> > *** Please tell me who you are.\n> > [...]\n> \n> Hmm. I think this is an artifact of running --amend. In the normal case,\n> we check the author ident beforehand. But in the --amend case, we take\n> the existing author, but then fail trying to generate the committer\n> ident. So we could probably do better by checking both explicitly\n> beforehand.\n\nIt seems we already had such a check, but it was not done correctly.\nThis is fixed by patch 2 below.\n\n> > The commit message I wrote is now lost. This is bad behaviour - the error\n> > should happen before one writes the commit message, or the message should be\n> > saved somewhere.\n> \n> It's not lost. It's in .git/COMMIT_EDITMSG.\n> \n> We could probably do a better job of informing the user of this when\n> commit dies prematurely.\n\nAnd patch 3 improves this situation.\n\n [1/3]: advice: pass varargs to strbuf_vaddf, not strbuf_addf\n [2/3]: commit: check committer identity more strictly\n [3/3]: commit: give a hint when a commit message has been abandoned\n\n-Peff\n"},{"id":"195549","messageId":"20120723184857.GA27588@sigill.intra.peff.net","threadId":"31059","inReplyTo":"20120723184634.GA31905@sigill.intra.peff.net","subject":"[PATCH 1/3] advice: pass varargs to strbuf_vaddf, not strbuf_addf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T18:48:57Z","receivedAt":"2012-07-23T18:48:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The advise() function takes a variable number of arguments\nand converts them into a va_list object to pass to strbuf\nfor handling. However, we accidentally called strbuf_addf\n(that takes a variable number of arguments) instead of\nstrbuf_vaddf (that takes a va_list).\n\nThis bug dates back to v1.7.8.1-1-g23cb5bf, but we never\nnoticed because none of the current callers passes a string\nwith a format specifier in it. And the compiler did not\nnotice because the format string is not available at\ncompile time.\n\nSigned-off-by: Jeff King \n---\n advice.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/advice.c b/advice.c\nindex a492eea..edfbd4a 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -32,7 +32,7 @@ void advise(const char *advice, ...)\n \tconst char *cp, *np;\n \n \tva_start(params, advice);\n-\tstrbuf_addf(&buf, advice, params);\n+\tstrbuf_vaddf(&buf, advice, params);\n \tva_end(params);\n \n \tfor (cp = buf.buf; *cp; cp = np) {\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195550","messageId":"20120723185035.GB27588@sigill.intra.peff.net","threadId":"31059","inReplyTo":"20120723184634.GA31905@sigill.intra.peff.net","subject":"[PATCH 2/3] commit: check committer identity more strictly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T18:50:35Z","receivedAt":"2012-07-23T18:50:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The identity of the committer will ultimately be pulled from\nthe ident code by commit_tree(). However, we make an attempt\nto check the author and committer identity early, before the\nuser has done any manual work like inputting a commit\nmessage. That lets us abort without them having to worry\nabout salvaging the work from .git/COMMIT_EDITMSG.\n\nThe early check for committer ident does not use the\nIDENT_STRICT flag, meaning that it would not find an empty\nname field. The motivation was presumably because we did not\nwant to be too restrictive, as later calls might be more lax\n(for example, when we create the reflog entry, we do not\ncare too much about a real name). However, because\ncommit_tree will always get a strict identity to put in the\ncommit object itself, there is no point in being lax only to\ndie later (and in fact it is harmful, because the user will\nhave wasted time typing their commit message).\n\nIncidentally, this bug was masked prior to 060d4bb, as the\ninitial loose call would taint the later strict call. So the\ncommit would succeed (albeit with a bogus committer line in\nthe commit object), and nobody noticed that our early check\ndid not match the later one.\n\nSigned-off-by: Jeff King \n---\n builtin/commit.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 95eeab1..20cef95 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -725,7 +725,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tstrbuf_release(&sb);\n \n \t/* This checks if committer ident is explicitly given */\n-\tstrbuf_addstr(&committer_ident, git_committer_info(0));\n+\tstrbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));\n \tif (use_editor && include_status) {\n \t\tchar *ai_tmp, *ci_tmp;\n \t\tif (whence != FROM_COMMIT)\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195551","messageId":"20120723185218.GC27588@sigill.intra.peff.net","threadId":"31059","inReplyTo":"20120723184634.GA31905@sigill.intra.peff.net","subject":"[PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T18:52:18Z","receivedAt":"2012-07-23T18:52:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we launch an editor for the user to create a commit\nmessage, they may put significant work into doing so.\nTypically we try to check common mistakes that could cause\nthe commit to fail early, so that we die before the user\ngoes to the trouble.\n\nWe may still experience some errors afterwards, though; in\nthis case, the user is given no hint that their commit\nmessage has been saved. Let's tell them where it is.\n\nSigned-off-by: Jeff King \n---\nI did not bother protecting this with advice.* config, as it is unlikely\nto come up regularly. If somebody cares, they are welcome to add it on\ntop.\n\n builtin/commit.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 20cef95..149e07d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -116,6 +116,16 @@ static enum {\n \tSTATUS_FORMAT_PORCELAIN\n } status_format = STATUS_FORMAT_LONG;\n \n+static int mention_abandoned_message;\n+static void maybe_mention_abandoned_message(void)\n+{\n+\tif (!mention_abandoned_message)\n+\t\treturn;\n+\tadvise(_(\"Your commit message has been saved in '%s' and will be\\n\"\n+\t\t \"overwritten by the next invocation of \\\"git commit\\\".\"),\n+\t git_path(commit_editmsg));\n+}\n+\n static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n {\n \tstruct strbuf *buf = opt->value;\n@@ -848,6 +858,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t_(\"Please supply the message using either -m or -F option.\\n\"));\n \t\t\texit(1);\n \t\t}\n+\t\tatexit(maybe_mention_abandoned_message);\n+\t\tmention_abandoned_message = 1;\n \t}\n \n \tif (!no_verify &&\n@@ -1532,11 +1544,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (template_untouched(&sb) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\n+\t\tmention_abandoned_message = 0;\n \t\texit(1);\n \t}\n \tif (message_is_empty(&sb) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit due to empty commit message.\\n\"));\n+\t\tmention_abandoned_message = 0;\n \t\texit(1);\n \t}\n \n@@ -1579,6 +1593,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"cannot update HEAD ref\"));\n \t}\n \n+\tmention_abandoned_message = 0;\n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n \tunlink(git_path(\"REVERT_HEAD\"));\n \tunlink(git_path(\"MERGE_HEAD\"));\n-- \n1.7.10.5.40.g059818d\n"},{"id":"195569","messageId":"7vzk6qjj7w.fsf@alter.siamese.dyndns.org","threadId":"31059","inReplyTo":"20120723185218.GC27588@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-23T20:49:55Z","receivedAt":"2012-07-23T20:49:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If we launch an editor for the user to create a commit\n> message, they may put significant work into doing so.\n> Typically we try to check common mistakes that could cause\n> the commit to fail early, so that we die before the user\n> goes to the trouble.\n>\n> We may still experience some errors afterwards, though; in\n> this case, the user is given no hint that their commit\n> message has been saved. Let's tell them where it is.\n\nLiberal use of atexit() for something like this makes me cringe\nsomewhat.\n\n>\n> Signed-off-by: Jeff King \n> ---\n> I did not bother protecting this with advice.* config, as it is unlikely\n> to come up regularly. If somebody cares, they are welcome to add it on\n> top.\n>\n> builtin/commit.c | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 20cef95..149e07d 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -116,6 +116,16 @@ static enum {\n> \tSTATUS_FORMAT_PORCELAIN\n> } status_format = STATUS_FORMAT_LONG;\n> \n> +static int mention_abandoned_message;\n> +static void maybe_mention_abandoned_message(void)\n> +{\n> +\tif (!mention_abandoned_message)\n> +\t\treturn;\n> +\tadvise(_(\"Your commit message has been saved in '%s' and will be\\n\"\n> +\t\t \"overwritten by the next invocation of \\\"git commit\\\".\"),\n> +\t git_path(commit_editmsg));\n> +}\n> +\n> static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n> {\n> \tstruct strbuf *buf = opt->value;\n> @@ -848,6 +858,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\t_(\"Please supply the message using either -m or -F option.\\n\"));\n> \t\t\texit(1);\n> \t\t}\n> +\t\tatexit(maybe_mention_abandoned_message);\n> +\t\tmention_abandoned_message = 1;\n> \t}\n> \n> \tif (!no_verify &&\n> @@ -1532,11 +1544,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \tif (template_untouched(&sb) && !allow_empty_message) {\n> \t\trollback_index_files();\n> \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\n> +\t\tmention_abandoned_message = 0;\n> \t\texit(1);\n> \t}\n> \tif (message_is_empty(&sb) && !allow_empty_message) {\n> \t\trollback_index_files();\n> \t\tfprintf(stderr, _(\"Aborting commit due to empty commit message.\\n\"));\n> +\t\tmention_abandoned_message = 0;\n> \t\texit(1);\n> \t}\n> \n> @@ -1579,6 +1593,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t\tdie(_(\"cannot update HEAD ref\"));\n> \t}\n> \n> +\tmention_abandoned_message = 0;\n> \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n> \tunlink(git_path(\"REVERT_HEAD\"));\n> \tunlink(git_path(\"MERGE_HEAD\"));\n"},{"id":"195570","messageId":"7vvchejj5e.fsf@alter.siamese.dyndns.org","threadId":"31059","inReplyTo":"20120723185035.GB27588@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] commit: check committer identity more strictly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-23T20:51:25Z","receivedAt":"2012-07-23T20:51:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Incidentally, this bug was masked prior to 060d4bb, as the\n> initial loose call would taint the later strict call. So the\n> commit would succeed (albeit with a bogus committer line in\n> the commit object), and nobody noticed that our early check\n> did not match the later one.\n>\n> Signed-off-by: Jeff King \n> ---\n> builtin/commit.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 95eeab1..20cef95 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -725,7 +725,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \tstrbuf_release(&sb);\n> \n> \t/* This checks if committer ident is explicitly given */\n> -\tstrbuf_addstr(&committer_ident, git_committer_info(0));\n> +\tstrbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));\n> \tif (use_editor && include_status) {\n> \t\tchar *ai_tmp, *ci_tmp;\n> \t\tif (whence != FROM_COMMIT)\n\nLooks sensible. Is this something we can detect in automated tests,\nor is it too cumbersome to set up?\n"},{"id":"195571","messageId":"20120723205209.GA6745@sigill.intra.peff.net","threadId":"31059","inReplyTo":"7vzk6qjj7w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T20:52:09Z","receivedAt":"2012-07-23T20:52:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 23, 2012 at 01:49:55PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > If we launch an editor for the user to create a commit\n> > message, they may put significant work into doing so.\n> > Typically we try to check common mistakes that could cause\n> > the commit to fail early, so that we die before the user\n> > goes to the trouble.\n> >\n> > We may still experience some errors afterwards, though; in\n> > this case, the user is given no hint that their commit\n> > message has been saved. Let's tell them where it is.\n> \n> Liberal use of atexit() for something like this makes me cringe\n> somewhat.\n\nI don't like it either, but there's not really a better way. The die()\nthat Ramana triggered in the initial report is deep inside the ident\ncode. The only other option would be to hook into die_routine, which I\nthink is even uglier.\n\n-Peff\n"},{"id":"195572","messageId":"20120723205321.GB6745@sigill.intra.peff.net","threadId":"31059","inReplyTo":"7vvchejj5e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] commit: check committer identity more strictly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T20:53:21Z","receivedAt":"2012-07-23T20:53:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 23, 2012 at 01:51:25PM -0700, Junio C Hamano wrote:\n\n> > diff --git a/builtin/commit.c b/builtin/commit.c\n> > index 95eeab1..20cef95 100644\n> > --- a/builtin/commit.c\n> > +++ b/builtin/commit.c\n> > @@ -725,7 +725,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> > \tstrbuf_release(&sb);\n> > \n> > \t/* This checks if committer ident is explicitly given */\n> > -\tstrbuf_addstr(&committer_ident, git_committer_info(0));\n> > +\tstrbuf_addstr(&committer_ident, git_committer_info(IDENT_STRICT));\n> > \tif (use_editor && include_status) {\n> > \t\tchar *ai_tmp, *ci_tmp;\n> > \t\tif (whence != FROM_COMMIT)\n> \n> Looks sensible. Is this something we can detect in automated tests,\n> or is it too cumbersome to set up?\n\nSorry, I meant to mention that in the cover letter. No, we can't test\nthis easily, because the code path in question is triggered by finding a\nblank name in /etc/passwd. We'd have to override our getpwent lookup.\n\n-Peff\n"},{"id":"195573","messageId":"7vr4s2jiqk.fsf@alter.siamese.dyndns.org","threadId":"31059","inReplyTo":"20120723205209.GA6745@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-23T21:00:19Z","receivedAt":"2012-07-23T21:00:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jul 23, 2012 at 01:49:55PM -0700, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > If we launch an editor for the user to create a commit\n>> > message, they may put significant work into doing so.\n>> > Typically we try to check common mistakes that could cause\n>> > the commit to fail early, so that we die before the user\n>> > goes to the trouble.\n>> >\n>> > We may still experience some errors afterwards, though; in\n>> > this case, the user is given no hint that their commit\n>> > message has been saved. Let's tell them where it is.\n>> \n>> Liberal use of atexit() for something like this makes me cringe\n>> somewhat.\n>\n> I don't like it either, but there's not really a better way. The die()\n> that Ramana triggered in the initial report is deep inside the ident\n> code. The only other option would be to hook into die_routine, which I\n> think is even uglier.\n\nThen I would rather not worry about it. A documentation update is\nprobably a good first step, though.\n"},{"id":"195575","messageId":"20120723211312.GA12533@sigill.intra.peff.net","threadId":"31059","inReplyTo":"7vr4s2jiqk.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T21:13:12Z","receivedAt":"2012-07-23T21:13:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 23, 2012 at 02:00:19PM -0700, Junio C Hamano wrote:\n\n> >> Liberal use of atexit() for something like this makes me cringe\n> >> somewhat.\n> >\n> > I don't like it either, but there's not really a better way. The die()\n> > that Ramana triggered in the initial report is deep inside the ident\n> > code. The only other option would be to hook into die_routine, which I\n> > think is even uglier.\n> \n> Then I would rather not worry about it. A documentation update is\n> probably a good first step, though.\n\nI'm OK with dropping this one; the likely cause is ident problems, and\nthe previous patch already helped with that (the next likely is probably\ncommit hooks failing, but that is just a guess).\n\nHere's a documentation patch.\n\n-- >8 --\nSubject: [PATCH] commit: document the temporary commit message file\n\nWe do not document COMMIT_EDITMSG at all, but users may want\nto know about it for two reasons:\n\n 1. They may want to tell their editor to configure itself\n for formatting a commit message.\n\n 2. If a commit is aborted by an error, the user may want\n to recover the commit message they typed.\n\nLet's put a note in git-commit(1).\n---\n Documentation/git-commit.txt | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex f400835..87297dc 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -407,6 +407,15 @@ This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,\n and `post-commit` hooks. See linkgit:githooks[5] for more\n information.\n \n+FILES\n+-----\n+\n+`$GIT_DIR/COMMIT_EDITMSG`::\n+\tThis file contains the commit message of a commit in progress.\n+\tIf `git-commit` exits due to an error before creating a commit,\n+\tany commit message that has been provided by the user (e.g., in\n+\tan editor session) will be available in this file, but will be\n+\toverwritten by the next invocation of `git-commit`.\n \n SEE ALSO\n --------\n"},{"id":"195582","messageId":"7veho2jh4q.fsf@alter.siamese.dyndns.org","threadId":"31059","inReplyTo":"20120723211312.GA12533@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-23T21:35:01Z","receivedAt":"2012-07-23T21:35:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Here's a documentation patch.\n>\n> -- >8 --\n> Subject: [PATCH] commit: document the temporary commit message file\n>\n> We do not document COMMIT_EDITMSG at all, but users may want\n> to know about it for two reasons:\n>\n> 1. They may want to tell their editor to configure itself\n> for formatting a commit message.\n>\n> 2. If a commit is aborted by an error, the user may want\n> to recover the commit message they typed.\n>\n> Let's put a note in git-commit(1).\n> ---\n> Documentation/git-commit.txt | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n>\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index f400835..87297dc 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -407,6 +407,15 @@ This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,\n> and `post-commit` hooks. See linkgit:githooks[5] for more\n> information.\n> \n> +FILES\n> +-----\n> +\n> +`$GIT_DIR/COMMIT_EDITMSG`::\n> +\tThis file contains the commit message of a commit in progress.\n> +\tIf `git-commit` exits due to an error before creating a commit,\n> +\tany commit message that has been provided by the user (e.g., in\n> +\tan editor session) will be available in this file, but will be\n> +\toverwritten by the next invocation of `git-commit`.\n> \n> SEE ALSO\n> --------\n\nVery sensible, modulo s/git-commit/git commit/ and lack of S-o-b.\n\nI also wondered if something like the following might be useful, but\nit probably falls into the \"water under the bridge\" category.\n\n builtin/commit.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 149e07d..83bcee4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\t\t\t\"with '#' will be kept; you may remove them\"\n \t\t\t\t\" yourself if you want to.\\n\"\n \t\t\t\t\"An empty message aborts the commit.\\n\"));\n+\t\tstatus_printf(s, GIT_COLOR_NORMAL,\n+\t\t\t _(\"The file '%s' keeps a copy of the log message\\n\"\n+\t\t\t\t\"you edited, if you wish to inspect it later.\\n\"\n+\t\t\t\t\"It will be wiped by the next invocation of 'git commit'.\\n\"),\n+\t\t\t git_path(\"COMMIT_EDITMSG\"));\n \t\tif (only_include_assumed)\n \t\t\tstatus_printf_ln(s, GIT_COLOR_NORMAL,\n \t\t\t\t\t\"%s\", only_include_assumed);\n"},{"id":"195583","messageId":"20120723214339.GA13931@sigill.intra.peff.net","threadId":"31059","inReplyTo":"7veho2jh4q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-07-23T21:43:39Z","receivedAt":"2012-07-23T21:43:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 23, 2012 at 02:35:01PM -0700, Junio C Hamano wrote:\n\n> > +FILES\n> > +-----\n> > +\n> > +`$GIT_DIR/COMMIT_EDITMSG`::\n> > +\tThis file contains the commit message of a commit in progress.\n> > +\tIf `git-commit` exits due to an error before creating a commit,\n> > +\tany commit message that has been provided by the user (e.g., in\n> > +\tan editor session) will be available in this file, but will be\n> > +\toverwritten by the next invocation of `git-commit`.\n> > \n> > SEE ALSO\n> > --------\n> \n> Very sensible, modulo s/git-commit/git commit/ and lack of S-o-b.\n\nYeah, I'm used to using the dashed form in documentation, but it's\nprobably reasonable to use the normal spaced form here. I assume you can\nforge my S-o-b?\n\n> I also wondered if something like the following might be useful, but\n> it probably falls into the \"water under the bridge\" category.\n> \n> builtin/commit.c | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 149e07d..83bcee4 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\t\t\t\"with '#' will be kept; you may remove them\"\n> \t\t\t\t\" yourself if you want to.\\n\"\n> \t\t\t\t\"An empty message aborts the commit.\\n\"));\n> +\t\tstatus_printf(s, GIT_COLOR_NORMAL,\n> +\t\t\t _(\"The file '%s' keeps a copy of the log message\\n\"\n> +\t\t\t\t\"you edited, if you wish to inspect it later.\\n\"\n> +\t\t\t\t\"It will be wiped by the next invocation of 'git commit'.\\n\"),\n> +\t\t\t git_path(\"COMMIT_EDITMSG\"));\n\nThat seems excessive, as it is inside the file itself. Unless your\neditor is terrible, it already shows you that information.\n\n-Peff\n"},{"id":"195585","messageId":"7va9yqjfis.fsf@alter.siamese.dyndns.org","threadId":"31059","inReplyTo":"20120723214339.GA13931@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-23T22:09:47Z","receivedAt":"2012-07-23T22:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jul 23, 2012 at 02:35:01PM -0700, Junio C Hamano wrote:\n> ...\n>> I also wondered if something like the following might be useful, but\n>> it probably falls into the \"water under the bridge\" category.\n>> \n>> builtin/commit.c | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>> \n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> index 149e07d..83bcee4 100644\n>> --- a/builtin/commit.c\n>> +++ b/builtin/commit.c\n>> @@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>> \t\t\t\t\"with '#' will be kept; you may remove them\"\n>> \t\t\t\t\" yourself if you want to.\\n\"\n>> \t\t\t\t\"An empty message aborts the commit.\\n\"));\n>> +\t\tstatus_printf(s, GIT_COLOR_NORMAL,\n>> +\t\t\t _(\"The file '%s' keeps a copy of the log message\\n\"\n>> +\t\t\t\t\"you edited, if you wish to inspect it later.\\n\"\n>> +\t\t\t\t\"It will be wiped by the next invocation of 'git commit'.\\n\"),\n>> +\t\t\t git_path(\"COMMIT_EDITMSG\"));\n>\n> That seems excessive, as it is inside the file itself. Unless your\n> editor is terrible, it already shows you that information.\n\nThe pathname was not the part that was interesting; \"If you wish to\ninspect it later\" was.\n\nBut that is what makes it water under the bridge. The message will\nbe gone by the time you really need it.\n"}]}