* Using git commit --amend on a commit with an empty message
@ 2012-07-09 14:24 Chris Webb
2012-07-09 17:25 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Chris Webb @ 2012-07-09 14:24 UTC (permalink / raw)
To: git
Github gists can be cloned as normal git repositories, but the commits made
through the web interface appear with an empty commit message. Running
git commit --amend against them exposes a slightly odd behaviour of git,
which I can also demonstrate as follows:
$ git init foo && cd foo
$ touch one && git add one
$ git commit -m '' --allow-empty-message
[master (root-commit) 535cb36]
0 files changed
create mode 100644 one
When I try to correct this commit message in an editor, it refuses to
proceed, objecting to the existing empty commit message:
$ git commit --amend
fatal: commit has empty message
Shouldn't this drop me into the editor and fail only if the resulting
message on exit is empty? (For comparison, git commit --amend -m 'oops' will
work fine; it's apparently only the edit case which doesn't.)
In fact, we even fail to start the editor if --allow-empty-message is
explicitly provided:
$ git commit --amend --allow-empty-message
fatal: commit has empty message
Assuming this isn't intentional for some reason I don't understand, I think
this is the correct tiny fix? make test succeeds fine both before and after.
-- >8 --
Subject: [PATCH] Allow edit of empty message with commit --amend
If git commit --amend is used on a commit with an empty message, it fails
unless -m is given, whether or not --allow-empty-message is specified.
Instead, allow it to proceed to the editor with an empty commit message.
Unless --allow-empty-message is in force, it will still abort later if an
empty message is saved from the editor. (That check was already present
and necessary to prevent a non-empty commit message being edited to an
empty one.)
Signed-off-by: Chris Webb <chris@arachsys.com>
---
builtin/commit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..6515da2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
hook_arg1 = "message";
} else if (use_message) {
buffer = strstr(use_message_buffer, "\n\n");
- if (!buffer || buffer[2] == '\0')
+ if (!use_editor && (!buffer || buffer[2] == '\0'))
die(_("commit has empty message"));
strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
hook_arg1 = "commit";
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Using git commit --amend on a commit with an empty message
2012-07-09 14:24 Using git commit --amend on a commit with an empty message Chris Webb
@ 2012-07-09 17:25 ` Junio C Hamano
2012-07-09 18:17 ` Chris Webb
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2012-07-09 17:25 UTC (permalink / raw)
To: Chris Webb; +Cc: git
Chris Webb <chris@arachsys.com> writes:
> In fact, we even fail to start the editor if --allow-empty-message is
> explicitly provided:
>
> $ git commit --allow-empty --allow-empty-message -m ''
> $ git commit --amend --allow-empty-message
> fatal: commit has empty message
>
> Assuming this isn't intentional for some reason I don't understand, I think
> this is the correct tiny fix? make test succeeds fine both before and after.
Yeah, it is a "bug" that exists only because nobody sane uses empty
message commits, let alone tries to amend such commits, hence went
unnoticed for a long time.
The patch looks sane; if we want to keep this as a feature or a
bugfix, we may want to pretect it with a new test, though.
Thanks.
> -- >8 --
> Subject: [PATCH] Allow edit of empty message with commit --amend
>
> If git commit --amend is used on a commit with an empty message, it fails
> unless -m is given, whether or not --allow-empty-message is specified.
>
> Instead, allow it to proceed to the editor with an empty commit message.
> Unless --allow-empty-message is in force, it will still abort later if an
> empty message is saved from the editor. (That check was already present
> and necessary to prevent a non-empty commit message being edited to an
> empty one.)
>
> Signed-off-by: Chris Webb <chris@arachsys.com>
> ---
> builtin/commit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index f43eaaf..6515da2 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> hook_arg1 = "message";
> } else if (use_message) {
> buffer = strstr(use_message_buffer, "\n\n");
> - if (!buffer || buffer[2] == '\0')
> + if (!use_editor && (!buffer || buffer[2] == '\0'))
> die(_("commit has empty message"));
> strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
> hook_arg1 = "commit";
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Using git commit --amend on a commit with an empty message
2012-07-09 17:25 ` Junio C Hamano
@ 2012-07-09 18:17 ` Chris Webb
2012-07-09 18:53 ` [PATCH v2] Allow edit of empty message with commit --amend Chris Webb
0 siblings, 1 reply; 5+ messages in thread
From: Chris Webb @ 2012-07-09 18:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> Yeah, it is a "bug" that exists only because nobody sane uses empty
> message commits, let alone tries to amend such commits, hence went
> unnoticed for a long time.
Quite. I only noticed it because this is the default behaviour of Github
gists and I wanted to replace the empty commit messages with more meaningful
ones.
> The patch looks sane; if we want to keep this as a feature or a
> bugfix, we may want to pretect it with a new test, though.
Yes, it's hardly something people will test often. Okay, I'll send a version
two with a suitable test.
Best wishes,
Chris.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Allow edit of empty message with commit --amend
2012-07-09 18:17 ` Chris Webb
@ 2012-07-09 18:53 ` Chris Webb
2012-07-09 19:43 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Chris Webb @ 2012-07-09 18:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If git commit --amend is used on a commit with an empty message, it fails
unless -m is given, whether or not --allow-empty-message is specified.
Instead, allow it to proceed to the editor with an empty commit message.
Unless --allow-empty-message is in force, it will still abort later if an
empty message is saved from the editor. (This check was already necessary
to prevent a non-empty commit message being edited to an empty one.)
Add a test for --amend --edit of an empty commit message which fails
without this fix, as it's a rare case that won't get frequently tested
otherwise.
Signed-off-by: Chris Webb <chris@arachsys.com>
---
builtin/commit.c | 2 +-
t/t7501-commit.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..6515da2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
hook_arg1 = "message";
} else if (use_message) {
buffer = strstr(use_message_buffer, "\n\n");
- if (!buffer || buffer[2] == '\0')
+ if (!use_editor && (!buffer || buffer[2] == '\0'))
die(_("commit has empty message"));
strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
hook_arg1 = "commit";
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b20ca0e..5ad636b 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -138,6 +138,21 @@ test_expect_success '--amend --edit' '
test_cmp expect msg
'
+test_expect_success '--amend --edit of empty message' '
+ cat >replace <<-\EOF &&
+ #!/bin/sh
+ echo "amended" >"$1"
+ EOF
+ chmod 755 replace &&
+ echo amended >expect &&
+ git commit --allow-empty --allow-empty-message -m "" &&
+ echo more bongo >file &&
+ git add file &&
+ EDITOR=./replace git commit --edit --amend &&
+ git diff-tree -s --format=%s HEAD >msg &&
+ test_cmp expect msg
+'
+
test_expect_success '-m --edit' '
echo amended >expect &&
git commit --allow-empty -m buffer &&
--
1.7.10
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Allow edit of empty message with commit --amend
2012-07-09 18:53 ` [PATCH v2] Allow edit of empty message with commit --amend Chris Webb
@ 2012-07-09 19:43 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2012-07-09 19:43 UTC (permalink / raw)
To: Chris Webb; +Cc: git
Chris Webb <chris@arachsys.com> writes:
> If git commit --amend is used on a commit with an empty message, it fails
> unless -m is given, whether or not --allow-empty-message is specified.
>
> Instead, allow it to proceed to the editor with an empty commit message.
> Unless --allow-empty-message is in force, it will still abort later if an
> empty message is saved from the editor. (This check was already necessary
> to prevent a non-empty commit message being edited to an empty one.)
>
> Add a test for --amend --edit of an empty commit message which fails
> without this fix, as it's a rare case that won't get frequently tested
> otherwise.
>
> Signed-off-by: Chris Webb <chris@arachsys.com>
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-09 19:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-09 14:24 Using git commit --amend on a commit with an empty message Chris Webb
2012-07-09 17:25 ` Junio C Hamano
2012-07-09 18:17 ` Chris Webb
2012-07-09 18:53 ` [PATCH v2] Allow edit of empty message with commit --amend Chris Webb
2012-07-09 19:43 ` Junio C Hamano
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.