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
[PATCH 3/3] commit: give a hint when a commit message has been abandoned
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.
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"));
--
1.7.10.5.40.g059818d
--
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
