# [PATCH] builtin-commit: Fix git-commit honoring status.color 4 messages from 2007-11-18 to 2007-11-19. Participants: Ping Yin, Junio C Hamano. Thread: https://gitlist.dev/t/10919 ## Ping Yin, 2007-11-18 17:10 Subject: [PATCH] builtin-commit: Fix git-commit honoring status.color Message-ID: <1195405834-1469-1-git-send-email-pkufranky@gmail.com> URL: https://gitlist.dev/e/1195405834-1469-1-git-send-email-pkufranky%40gmail.com ``` status.color shouldn't be honored when committing since the run-status output is fed to the editor. --- builtin-commit.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index 8db74ed..4396e7d 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -45,6 +45,7 @@ static int quiet, verbose, untracked_files, no_verify; static int no_edit, initial_commit, in_merge; const char *only_include_assumed; struct strbuf message; +extern int wt_status_use_color; static int opt_parse_m(const struct option *opt, const char *arg, int unset) { @@ -325,6 +326,7 @@ static int prepare_log_message(const char *index_file, const char *prefix) if (only_include_assumed) fprintf(fp, "# %s\n", only_include_assumed); + wt_status_use_color = 0; commitable = run_status(fp, index_file, prefix); fclose(fp); -- 1.5.3.5.1876.g7ba19-dirty ``` ## Junio C Hamano, 2007-11-18 19:55 Subject: Re: [PATCH] builtin-commit: Fix git-commit honoring status.color Message-ID: <7vejenuy4i.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vejenuy4i.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1195405834-1469-1-git-send-email-pkufranky@gmail.com> ``` Ping Yin writes: > status.color shouldn't be honored when committing since the run-status > output is fed to the editor. > --- > builtin-commit.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/builtin-commit.c b/builtin-commit.c > index 8db74ed..4396e7d 100644 > --- a/builtin-commit.c > +++ b/builtin-commit.c > @@ -45,6 +45,7 @@ static int quiet, verbose, untracked_files, no_verify; > static int no_edit, initial_commit, in_merge; > const char *only_include_assumed; > struct strbuf message; > +extern int wt_status_use_color; > > static int opt_parse_m(const struct option *opt, const char *arg, int unset) > { > @@ -325,6 +326,7 @@ static int prepare_log_message(const char *index_file, const char *prefix) > if (only_include_assumed) > fprintf(fp, "# %s\n", only_include_assumed); > > + wt_status_use_color = 0; > commitable = run_status(fp, index_file, prefix); > > fclose(fp); Although I admit I do not care much about the "status color", I suspect this patch is not quite right. When prepare_log_message() returns "no committable changes" and we are not in merge, the calling cmd_commit() does another run_status() to show the status of the index and the work tree to the stdout, and at that point, we _do_ want to honor the configuration setting you are discarding with this assignment. --- builtin-commit.c | 5 ++++- wt-status.h | 1 + 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index 4e2f4aa..058cd32 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -300,7 +300,7 @@ static const char sign_off_header[] = "Signed-off-by: "; static int prepare_log_message(const char *index_file, const char *prefix) { struct stat statbuf; - int commitable; + int commitable, saved_color_setting; struct strbuf sb; char *buffer; FILE *fp; @@ -383,7 +383,10 @@ static int prepare_log_message(const char *index_file, const char *prefix) if (only_include_assumed) fprintf(fp, "# %s\n", only_include_assumed); + saved_color_setting = wt_status_use_color; + wt_status_use_color = 0; commitable = run_status(fp, index_file, prefix); + wt_status_use_color = saved_color_setting; fclose(fp); diff --git a/wt-status.h b/wt-status.h index f58ebcb..225fb4d 100644 --- a/wt-status.h +++ b/wt-status.h @@ -27,6 +27,7 @@ struct wt_status { }; int git_status_config(const char *var, const char *value); +int wt_status_use_color; void wt_status_prepare(struct wt_status *s); void wt_status_print(struct wt_status *s); ``` ## Junio C Hamano, 2007-11-18 20:30 Subject: [PATCH] builtin-commit: run commit-msg hook with correct message file Message-ID: <7v1wanuwiy.fsf_-_@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1wanuwiy.fsf_-_%40gitster.siamese.dyndns.org In-Reply-To: <7vejenuy4i.fsf@gitster.siamese.dyndns.org> ``` It should run with $GIT_DIR/COMMIT_EDITMSG, not just COMMIT_EDITMSG. Signed-off-by: Junio C Hamano --- * Other things I noticed that are still broken: - "git commit -v" does not give you the diff in the message template for your final review; - message_is_empty() is bogus. It does run stripspace() but does not strip out the diff "git commit -v" would produce before doing its comparison; builtin-commit.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-commit.c b/builtin-commit.c index 058cd32..439fcc2 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -753,7 +753,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) rollback_index_files(); die("could not read commit message\n"); } - if (run_hook(index_file, "commit-msg", commit_editmsg)) { + if (run_hook(index_file, "commit-msg", git_path(commit_editmsg))) { rollback_index_files(); exit(1); } -- 1.5.3.5.1815.g9445b ``` ## Ping Yin, 2007-11-19 18:26 Subject: Re: [PATCH] builtin-commit: Fix git-commit honoring status.color Message-ID: <46dff0320711191026l6d749886y4fd3879f555c6107@mail.gmail.com> URL: https://gitlist.dev/e/46dff0320711191026l6d749886y4fd3879f555c6107%40mail.gmail.com In-Reply-To: <7vejenuy4i.fsf@gitster.siamese.dyndns.org> ``` > > Although I admit I do not care much about the "status color", I > suspect this patch is not quite right. > > When prepare_log_message() returns "no committable changes" and > we are not in merge, the calling cmd_commit() does another > run_status() to show the status of the index and the work tree > to the stdout, and at that point, we _do_ want to honor the > configuration setting you are discarding with this assignment. > You're right. I forgot about untracked files when there are 'no committable changes' > --- > > builtin-commit.c | 5 ++++- > wt-status.h | 1 + > 2 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/builtin-commit.c b/builtin-commit.c > index 4e2f4aa..058cd32 100644 > --- a/builtin-commit.c > +++ b/builtin-commit.c > @@ -300,7 +300,7 @@ static const char sign_off_header[] = "Signed-off-by: "; > static int prepare_log_message(const char *index_file, const char *prefix) > { > struct stat statbuf; > - int commitable; > + int commitable, saved_color_setting; > struct strbuf sb; > char *buffer; > FILE *fp; > @@ -383,7 +383,10 @@ static int prepare_log_message(const char *index_file, const char *prefix) > if (only_include_assumed) > fprintf(fp, "# %s\n", only_include_assumed); > > + saved_color_setting = wt_status_use_color; > + wt_status_use_color = 0; > commitable = run_status(fp, index_file, prefix); > + wt_status_use_color = saved_color_setting; > > fclose(fp); > > diff --git a/wt-status.h b/wt-status.h > index f58ebcb..225fb4d 100644 > --- a/wt-status.h > +++ b/wt-status.h > @@ -27,6 +27,7 @@ struct wt_status { > }; > > int git_status_config(const char *var, const char *value); > +int wt_status_use_color; > void wt_status_prepare(struct wt_status *s); > void wt_status_print(struct wt_status *s); > > -- Ping Yin ```