threads / discuss / 33932
Subject: Commit with an empty message broken since v1.8.2.1
Commit a24a41ea9a928ccde2db074ab0835c4817223c9d introduces a bug which is still present in latest master.
This command
git commit -m "" --allow-empty --allow-empty-message
should create an empty commit with an empty message and never open a text editor for the commit message. Since the change, the editor is always opened.
My current workaround to skip the editor is setting the environment variable:
GIT_EDITOR=true
René Scharfe[PATCH] commit: don't start editor if empty message is given with -m
If an empty message is specified with the option -m of git commit then the editor is started. That's unexpected and unnecessary. Instead of using the length of the message string for checking if the user specified one, directly remember if the option -m was given.
Reported-by: Mislav Marohnić <mislav.marohnic@gmail.com> Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx> --- builtin/commit.c | 10 ++++++---- t/t7502-commit.sh | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c index d2f30d9..1621dfc 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -107,7 +107,7 @@ static const char *cleanup_arg; static enum commit_whence whence; static int use_editor = 1, include_status = 1; -static int show_ignored_in_status; +static int show_ignored_in_status, have_option_m; static const char *only_include_assumed; static struct strbuf message = STRBUF_INIT; @@ -121,9 +121,11 @@ static enum { static int opt_parse_m(const struct option *opt, const char *arg, int unset) { struct strbuf *buf = opt->value; - if (unset) + if (unset) { + have_option_m = 0; strbuf_setlen(buf, 0); - else { + } else { + have_option_m = 1; if (buf->len) strbuf_addch(buf, '\n'); strbuf_addstr(buf, arg); @@ -975,7 +977,7 @@ static int parse_and_validate_options(int argc, const char *argv[], if (force_author && renew_authorship) die(_("Using both --reset-author and --author does not make sense")); - if (logfile || message.len || use_message || fixup_message) + if (logfile || have_option_m || use_message || fixup_message) use_editor = 0; if (0 <= edit_flag) use_editor = edit_flag; diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index a4938b1..6313da2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -361,6 +361,23 @@ test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' ' test_cmp expect .git/result ' +test_expect_success 'do not fire editor if -m <msg> was given' ' + echo tick >file && + git add file && + echo "editor not started" >.git/result && + (GIT_EDITOR="\"$(pwd)/.git/FAKE_EDITOR\"" git commit -m tick) && + test "$(cat .git/result)" = "editor not started" +' + +test_expect_success 'do not fire editor if -m "" was given' ' + echo tock >file && + git add file && + echo "editor not started" >.git/result && + (GIT_EDITOR="\"$(pwd)/.git/FAKE_EDITOR\"" \ + git commit -m "" --allow-empty-message) && + test "$(cat .git/result)" = "editor not started" +' + test_expect_success 'do not fire editor in the presence of conflicts' ' git clean -f &&
-- 1.8.3
Ramkumar Ramachandra[PATCH] commit: don't use-editor when allow-empty-message
Commit a24a41e (git-commit: only append a newline to -m mesg if necessary, 2013-02-18) introduced a regression: when --allow-empty-message is used and an empty message is explicitly specified with -m "", git commit still launches $EDITOR unnecessarily. The commit (correctly) fixes opt_parse_m() to not fill in two newlines into the message buffer unconditionally. The real problem is that launching $EDITOR only depends on use_editor and whether message is empty. Fix the problem by setting use_editor to 0 when --allow-empty-message is specified in the codepath where an explicit string is passed via -m.
Reported-by: Mislav Marohnić <mislav.marohnic@gmail.com> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- builtin/commit.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/builtin/commit.c b/builtin/commit.c index d2f30d9..1f5da9d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -128,6 +128,8 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset) strbuf_addch(buf, '\n'); strbuf_addstr(buf, arg); strbuf_complete_line(buf); + if (allow_empty_message) + use_editor = 0; } return 0; }
-- 1.8.3.1.g33669de
Ramkumar RamachandraRe: [PATCH] commit: don't use-editor when allow-empty-message
Ramkumar Ramachandra wrote:
> builtin/commit.c | 2 ++ > 1 file changed, 2 insertions(+)
I just made this dependent on the order in which options are parsed. If --allow-empty-message is specified before -m "", it works. Otherwise, not.
Sorry about the stupidity.
Ramkumar Ramachandra[PATCH v2] commit: don't use-editor when allow-empty-message
Commit a24a41e (git-commit: only append a newline to -m mesg if necessary, 2013-02-18) introduced a regression: when --allow-empty-message is used and an empty message is explicitly specified with -m "", git commit still launches $EDITOR unnecessarily. The commit (correctly) fixes opt_parse_m() to not fill in two newlines into the message buffer unconditionally. The real problem is that launching $EDITOR only depends on use_editor and whether message is empty. Fix the problem by setting explicit_message in the codepath where an explicit string is passed via -m, and then checking it before launching $EDITOR.
Reported-by: Mislav Marohnić <mislav.marohnic@gmail.com> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- Works?
builtin/commit.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c index d2f30d9..7d72ba7 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -108,6 +108,7 @@ static const char *cleanup_arg; static enum commit_whence whence; static int use_editor = 1, include_status = 1; static int show_ignored_in_status; +static int explicit_message = 0; static const char *only_include_assumed; static struct strbuf message = STRBUF_INIT; @@ -128,6 +129,7 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset) strbuf_addch(buf, '\n'); strbuf_addstr(buf, arg); strbuf_complete_line(buf); + explicit_message = 1; } return 0; } @@ -824,7 +826,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, git_path(commit_editmsg), hook_arg1, hook_arg2, NULL)) return 0; - if (use_editor) { + if (use_editor && !explicit_message) { char index[PATH_MAX]; const char *env[2] = { NULL }; env[0] = index;
-- 1.8.3.1.g33669de.dirty
Antoine PelisseRe: [PATCH v2] commit: don't use-editor when allow-empty-message
So now we have two fixes for the same issue, don't we ? You probably missed $gmane/225534.
On Mon, May 27, 2013 at 4:20 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Commit a24a41e (git-commit: only append a newline to -m mesg if
> necessary, 2013-02-18) introduced a regression: when
> --allow-empty-message is used and an empty message is explicitly
> specified with -m "", git commit still launches $EDITOR unnecessarily.
> The commit (correctly) fixes opt_parse_m() to not fill in two newlines
> into the message buffer unconditionally. The real problem is that
> launching $EDITOR only depends on use_editor and whether message is
> empty. Fix the problem by setting explicit_message in the codepath
> where an explicit string is passed via -m, and then checking it before
> launching $EDITOR.
>
> Reported-by: Mislav Marohnić <mislav.marohnic@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Works?
>
> builtin/commit.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d2f30d9..7d72ba7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -108,6 +108,7 @@ static const char *cleanup_arg;
> static enum commit_whence whence;
> static int use_editor = 1, include_status = 1;
> static int show_ignored_in_status;
> +static int explicit_message = 0;
> static const char *only_include_assumed;
> static struct strbuf message = STRBUF_INIT;
>
> @@ -128,6 +129,7 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
> strbuf_addch(buf, '\n');
> strbuf_addstr(buf, arg);
> strbuf_complete_line(buf);
> + explicit_message = 1;
> }
> return 0;
> }
> @@ -824,7 +826,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
> git_path(commit_editmsg), hook_arg1, hook_arg2, NULL))
> return 0;
>
> - if (use_editor) {
> + if (use_editor && !explicit_message) {
> char index[PATH_MAX];
> const char *env[2] = { NULL };
> env[0] = index;
> --
> 1.8.3.1.g33669de.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.htmlRamkumar RamachandraRe: [PATCH v2] commit: don't use-editor when allow-empty-message
Antoine Pelisse wrote:
> So now we have two fixes for the same issue, don't we ? > You probably missed $gmane/225534.
Gah, missed that. Sorry for the noise.