* Commit with an empty message broken since v1.8.2.1
@ 2013-05-25 19:12 Mislav Marohnić
2013-05-25 21:43 ` [PATCH] commit: don't start editor if empty message is given with -m René Scharfe
2013-05-27 14:03 ` [PATCH] commit: don't use-editor when allow-empty-message Ramkumar Ramachandra
0 siblings, 2 replies; 7+ messages in thread
From: Mislav Marohnić @ 2013-05-25 19:12 UTC (permalink / raw)
To: git; +Cc: Brandon Casey, Jonathan Nieder
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] commit: don't start editor if empty message is given with -m 2013-05-25 19:12 Commit with an empty message broken since v1.8.2.1 Mislav Marohnić @ 2013-05-25 21:43 ` René Scharfe 2013-05-27 14:03 ` [PATCH] commit: don't use-editor when allow-empty-message Ramkumar Ramachandra 1 sibling, 0 replies; 7+ messages in thread From: René Scharfe @ 2013-05-25 21:43 UTC (permalink / raw) To: Mislav Marohnić; +Cc: git, Brandon Casey, Jonathan Nieder 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] commit: don't use-editor when allow-empty-message 2013-05-25 19:12 Commit with an empty message broken since v1.8.2.1 Mislav Marohnić 2013-05-25 21:43 ` [PATCH] commit: don't start editor if empty message is given with -m René Scharfe @ 2013-05-27 14:03 ` Ramkumar Ramachandra 2013-05-27 14:07 ` Ramkumar Ramachandra 2013-05-27 14:20 ` [PATCH v2] " Ramkumar Ramachandra 1 sibling, 2 replies; 7+ messages in thread From: Ramkumar Ramachandra @ 2013-05-27 14:03 UTC (permalink / raw) To: Mislav Marohnić; +Cc: Brandon Casey, Jonathan Nieder, Git List 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] commit: don't use-editor when allow-empty-message 2013-05-27 14:03 ` [PATCH] commit: don't use-editor when allow-empty-message Ramkumar Ramachandra @ 2013-05-27 14:07 ` Ramkumar Ramachandra 2013-05-27 14:20 ` [PATCH v2] " Ramkumar Ramachandra 1 sibling, 0 replies; 7+ messages in thread From: Ramkumar Ramachandra @ 2013-05-27 14:07 UTC (permalink / raw) To: Mislav Marohnić; +Cc: Brandon Casey, Jonathan Nieder, Git List 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] commit: don't use-editor when allow-empty-message 2013-05-27 14:03 ` [PATCH] commit: don't use-editor when allow-empty-message Ramkumar Ramachandra 2013-05-27 14:07 ` Ramkumar Ramachandra @ 2013-05-27 14:20 ` Ramkumar Ramachandra 2013-05-27 14:53 ` Antoine Pelisse 1 sibling, 1 reply; 7+ messages in thread From: Ramkumar Ramachandra @ 2013-05-27 14:20 UTC (permalink / raw) To: Mislav Marohnić; +Cc: Brandon Casey, Jonathan Nieder, Git List 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] commit: don't use-editor when allow-empty-message 2013-05-27 14:20 ` [PATCH v2] " Ramkumar Ramachandra @ 2013-05-27 14:53 ` Antoine Pelisse 2013-05-27 17:57 ` Ramkumar Ramachandra 0 siblings, 1 reply; 7+ messages in thread From: Antoine Pelisse @ 2013-05-27 14:53 UTC (permalink / raw) To: Ramkumar Ramachandra Cc: Mislav Marohnić, Brandon Casey, Jonathan Nieder, Git List 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.html ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] commit: don't use-editor when allow-empty-message 2013-05-27 14:53 ` Antoine Pelisse @ 2013-05-27 17:57 ` Ramkumar Ramachandra 0 siblings, 0 replies; 7+ messages in thread From: Ramkumar Ramachandra @ 2013-05-27 17:57 UTC (permalink / raw) To: Antoine Pelisse Cc: Mislav Marohnić, Brandon Casey, Jonathan Nieder, Git List 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-05-27 17:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-25 19:12 Commit with an empty message broken since v1.8.2.1 Mislav Marohnić 2013-05-25 21:43 ` [PATCH] commit: don't start editor if empty message is given with -m René Scharfe 2013-05-27 14:03 ` [PATCH] commit: don't use-editor when allow-empty-message Ramkumar Ramachandra 2013-05-27 14:07 ` Ramkumar Ramachandra 2013-05-27 14:20 ` [PATCH v2] " Ramkumar Ramachandra 2013-05-27 14:53 ` Antoine Pelisse 2013-05-27 17:57 ` Ramkumar Ramachandra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).