Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
Jeff King writes:
> On Mon, Jul 23, 2012 at 02:35:01PM -0700, Junio C Hamano wrote:
> ...
>> I also wondered if something like the following might be useful, but
>> it probably falls into the "water under the bridge" category.
>>
>> builtin/commit.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 149e07d..83bcee4 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file,
>> const char *prefix,
>> "with '#' will be kept; you may remove them"
>> " yourself if you want to.\n"
>> "An empty message aborts the commit.\n"));
>> +status_printf(s, GIT_COLOR_NORMAL,
>> + _("The file '%s' keeps a copy of the log
>> message\n"
>> +"you edited, if you wish to inspect it later.\n"
>> +"It will be wiped by the next invocation of
>> 'git commit'.\n"),
>> + git_path("COMMIT_EDITMSG"));
>
> That seems excessive, as it is inside the file itself. Unless your
> editor is terrible, it already shows you that information.
The pathname was not the part that was interesting; "If you wish to
inspect it later" was.
But that is what makes it water under the bridge. The message will
be gone by the time you really need it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
On Mon, Jul 23, 2012 at 02:35:01PM -0700, Junio C Hamano wrote:
> > +FILES
> > +-
> > +
> > +`$GIT_DIR/COMMIT_EDITMSG`::
> > + This file contains the commit message of a commit in progress.
> > + If `git-commit` exits due to an error before creating a commit,
> > + any commit message that has been provided by the user (e.g., in
> > + an editor session) will be available in this file, but will be
> > + overwritten by the next invocation of `git-commit`.
> >
> > SEE ALSO
> >
>
> Very sensible, modulo s/git-commit/git commit/ and lack of S-o-b.
Yeah, I'm used to using the dashed form in documentation, but it's
probably reasonable to use the normal spaced form here. I assume you can
forge my S-o-b?
> I also wondered if something like the following might be useful, but
> it probably falls into the "water under the bridge" category.
>
> builtin/commit.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 149e07d..83bcee4 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> "with '#' will be kept; you may remove them"
> " yourself if you want to.\n"
> "An empty message aborts the commit.\n"));
> + status_printf(s, GIT_COLOR_NORMAL,
> + _("The file '%s' keeps a copy of the log
> message\n"
> + "you edited, if you wish to inspect it later.\n"
> + "It will be wiped by the next invocation of
> 'git commit'.\n"),
> + git_path("COMMIT_EDITMSG"));
That seems excessive, as it is inside the file itself. Unless your
editor is terrible, it already shows you that information.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
Jeff King writes:
> Here's a documentation patch.
>
> -- >8 --
> Subject: [PATCH] commit: document the temporary commit message file
>
> We do not document COMMIT_EDITMSG at all, but users may want
> to know about it for two reasons:
>
> 1. They may want to tell their editor to configure itself
> for formatting a commit message.
>
> 2. If a commit is aborted by an error, the user may want
> to recover the commit message they typed.
>
> Let's put a note in git-commit(1).
> ---
> Documentation/git-commit.txt | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index f400835..87297dc 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -407,6 +407,15 @@ This command can run `commit-msg`, `prepare-commit-msg`,
> `pre-commit`,
> and `post-commit` hooks. See linkgit:githooks[5] for more
> information.
>
> +FILES
> +-
> +
> +`$GIT_DIR/COMMIT_EDITMSG`::
> + This file contains the commit message of a commit in progress.
> + If `git-commit` exits due to an error before creating a commit,
> + any commit message that has been provided by the user (e.g., in
> + an editor session) will be available in this file, but will be
> + overwritten by the next invocation of `git-commit`.
>
> SEE ALSO
>
Very sensible, modulo s/git-commit/git commit/ and lack of S-o-b.
I also wondered if something like the following might be useful, but
it probably falls into the "water under the bridge" category.
builtin/commit.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/builtin/commit.c b/builtin/commit.c
index 149e07d..83bcee4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -768,6 +768,11 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
"with '#' will be kept; you may remove them"
" yourself if you want to.\n"
"An empty message aborts the commit.\n"));
+ status_printf(s, GIT_COLOR_NORMAL,
+ _("The file '%s' keeps a copy of the log
message\n"
+ "you edited, if you wish to inspect it later.\n"
+ "It will be wiped by the next invocation of
'git commit'.\n"),
+ git_path("COMMIT_EDITMSG"));
if (only_include_assumed)
status_printf_ln(s, GIT_COLOR_NORMAL,
"%s", only_include_assumed);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
On Mon, Jul 23, 2012 at 02:00:19PM -0700, Junio C Hamano wrote: > >> Liberal use of atexit() for something like this makes me cringe > >> somewhat. > > > > I don't like it either, but there's not really a better way. The die() > > that Ramana triggered in the initial report is deep inside the ident > > code. The only other option would be to hook into die_routine, which I > > think is even uglier. > > Then I would rather not worry about it. A documentation update is > probably a good first step, though. I'm OK with dropping this one; the likely cause is ident problems, and the previous patch already helped with that (the next likely is probably commit hooks failing, but that is just a guess). Here's a documentation patch. -- >8 -- Subject: [PATCH] commit: document the temporary commit message file We do not document COMMIT_EDITMSG at all, but users may want to know about it for two reasons: 1. They may want to tell their editor to configure itself for formatting a commit message. 2. If a commit is aborted by an error, the user may want to recover the commit message they typed. Let's put a note in git-commit(1). --- Documentation/git-commit.txt | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index f400835..87297dc 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -407,6 +407,15 @@ This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`, and `post-commit` hooks. See linkgit:githooks[5] for more information. +FILES +- + +`$GIT_DIR/COMMIT_EDITMSG`:: + This file contains the commit message of a commit in progress. + If `git-commit` exits due to an error before creating a commit, + any commit message that has been provided by the user (e.g., in + an editor session) will be available in this file, but will be + overwritten by the next invocation of `git-commit`. SEE ALSO -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
Jeff King writes: > On Mon, Jul 23, 2012 at 01:49:55PM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > If we launch an editor for the user to create a commit >> > message, they may put significant work into doing so. >> > Typically we try to check common mistakes that could cause >> > the commit to fail early, so that we die before the user >> > goes to the trouble. >> > >> > We may still experience some errors afterwards, though; in >> > this case, the user is given no hint that their commit >> > message has been saved. Let's tell them where it is. >> >> Liberal use of atexit() for something like this makes me cringe >> somewhat. > > I don't like it either, but there's not really a better way. The die() > that Ramana triggered in the initial report is deep inside the ident > code. The only other option would be to hook into die_routine, which I > think is even uglier. Then I would rather not worry about it. A documentation update is probably a good first step, though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
On Mon, Jul 23, 2012 at 01:49:55PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > If we launch an editor for the user to create a commit > > message, they may put significant work into doing so. > > Typically we try to check common mistakes that could cause > > the commit to fail early, so that we die before the user > > goes to the trouble. > > > > We may still experience some errors afterwards, though; in > > this case, the user is given no hint that their commit > > message has been saved. Let's tell them where it is. > > Liberal use of atexit() for something like this makes me cringe > somewhat. I don't like it either, but there's not really a better way. The die() that Ramana triggered in the initial report is deep inside the ident code. The only other option would be to hook into die_routine, which I think is even uglier. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] commit: give a hint when a commit message has been abandoned
Jeff King writes:
> If we launch an editor for the user to create a commit
> message, they may put significant work into doing so.
> Typically we try to check common mistakes that could cause
> the commit to fail early, so that we die before the user
> goes to the trouble.
>
> We may still experience some errors afterwards, though; in
> this case, the user is given no hint that their commit
> message has been saved. Let's tell them where it is.
Liberal use of atexit() for something like this makes me cringe
somewhat.
>
> Signed-off-by: Jeff King
> ---
> I did not bother protecting this with advice.* config, as it is unlikely
> to come up regularly. If somebody cares, they are welcome to add it on
> top.
>
> builtin/commit.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 20cef95..149e07d 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -116,6 +116,16 @@ static enum {
> STATUS_FORMAT_PORCELAIN
> } status_format = STATUS_FORMAT_LONG;
>
> +static int mention_abandoned_message;
> +static void maybe_mention_abandoned_message(void)
> +{
> + if (!mention_abandoned_message)
> + return;
> + advise(_("Your commit message has been saved in '%s' and will be\n"
> + "overwritten by the next invocation of \"git commit\"."),
> +git_path(commit_editmsg));
> +}
> +
> static int opt_parse_m(const struct option *opt, const char *arg, int unset)
> {
> struct strbuf *buf = opt->value;
> @@ -848,6 +858,8 @@ static int prepare_to_commit(const char *index_file,
> const char *prefix,
> _("Please supply the message using either -m or -F
> option.\n"));
> exit(1);
> }
> + atexit(maybe_mention_abandoned_message);
> + mention_abandoned_message = 1;
> }
>
> if (!no_verify &&
> @@ -1532,11 +1544,13 @@ int cmd_commit(int argc, const char **argv, const
> char *prefix)
> if (template_untouched(&sb) && !allow_empty_message) {
> rollback_index_files();
> fprintf(stderr, _("Aborting commit; you did not edit the
> message.\n"));
> + mention_abandoned_message = 0;
> exit(1);
> }
> if (message_is_empty(&sb) && !allow_empty_message) {
> rollback_index_files();
> fprintf(stderr, _("Aborting commit due to empty commit
> message.\n"));
> + mention_abandoned_message = 0;
> exit(1);
> }
>
> @@ -1579,6 +1593,7 @@ int cmd_commit(int argc, const char **argv, const char
> *prefix)
> die(_("cannot update HEAD ref"));
> }
>
> + mention_abandoned_message = 0;
> unlink(git_path("CHERRY_PICK_HEAD"));
> unlink(git_path("REVERT_HEAD"));
> unlink(git_path("MERGE_HEAD"));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
