[PATCH v2 1/3] commit: factor status configuration is a helper function

[PATCH v2 1/3] commit: factor status configuration is a helper function

[Matthieu Moy]

cmd_commit and cmd_status use very similar code to initialize the
wt_status structure. Factor this code into a function to ensure future
changes will keep both versions consistent.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
New patch, as discussed with Peff.

 builtin/commit.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 60812b5..dc957a9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -163,6 +163,14 @@ static void determine_whence(struct wt_status *s)
 		s->whence = whence;
 }
 
+static void status_init_config(struct wt_status *s, config_fn_t fn)
+{
+	wt_status_prepare(s);
+	gitmodules_config();
+	git_config(fn, s);
+	determine_whence(s);
+}
+
 static void rollback_index_files(void)
 {
 	switch (commit_style) {
@@ -1246,10 +1254,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_status_usage, builtin_status_options);
 
-	wt_status_prepare(&s);
-	gitmodules_config();
-	git_config(git_status_config, &s);
-	determine_whence(&s);
+	status_init_config(&s, git_status_config);
 	argc = parse_options(argc, argv, prefix,
 			     builtin_status_options,
 			     builtin_status_usage, 0);
@@ -1490,11 +1495,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(builtin_commit_usage, builtin_commit_options);
 
-	wt_status_prepare(&s);
-	gitmodules_config();
-	git_config(git_commit_config, &s);
+	status_init_config(&s, git_commit_config);
 	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
-	determine_whence(&s);
 	s.colopts = 0;
 
 	if (get_sha1("HEAD", sha1))
-- 
1.8.4.8.g834017f

[PATCH v2 2/3] wt-status: turn advice_status_hints into a field of wt_status

[Matthieu Moy]

No behavior change in this patch, but this makes the display of status
hints more flexible as they can be enabled or disabled for individual
calls to commit.c:run_status().

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
No real change since v1, just a slight adaptation after the PATCH 1.

 builtin/commit.c |  1 +
 wt-status.c      | 38 +++++++++++++++++++-------------------
 wt-status.h      |  1 +
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index dc957a9..7bfce94 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -169,6 +169,7 @@ static void status_init_config(struct wt_status *s, config_fn_t fn)
 	gitmodules_config();
 	git_config(fn, s);
 	determine_whence(s);
+	s->hints = advice_status_hints; /* must come after git_config() */
 }
 
 static void rollback_index_files(void)
diff --git a/wt-status.c b/wt-status.c
index cb24f1f..885ee66 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -160,7 +160,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 		}
 	}
 
-	if (!advice_status_hints)
+	if (!s->hints)
 		return;
 	if (s->whence != FROM_COMMIT)
 		;
@@ -187,7 +187,7 @@ static void wt_status_print_cached_header(struct wt_status *s)
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	status_printf_ln(s, c, _("Changes to be committed:"));
-	if (!advice_status_hints)
+	if (!s->hints)
 		return;
 	if (s->whence != FROM_COMMIT)
 		; /* NEEDSWORK: use "git reset --unresolve"??? */
@@ -205,7 +205,7 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 	const char *c = color(WT_STATUS_HEADER, s);
 
 	status_printf_ln(s, c, _("Changes not staged for commit:"));
-	if (!advice_status_hints)
+	if (!s->hints)
 		return;
 	if (!has_deleted)
 		status_printf_ln(s, c, _("  (use \"git add <file>...\" to update what will be committed)"));
@@ -223,7 +223,7 @@ static void wt_status_print_other_header(struct wt_status *s,
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	status_printf_ln(s, c, "%s:", what);
-	if (!advice_status_hints)
+	if (!s->hints)
 		return;
 	status_printf_ln(s, c, _("  (use \"git %s <file>...\" to include in what will be committed)"), how);
 	status_printf_ln(s, c, "");
@@ -801,13 +801,13 @@ static void show_merge_in_progress(struct wt_status *s,
 {
 	if (has_unmerged(s)) {
 		status_printf_ln(s, color, _("You have unmerged paths."));
-		if (advice_status_hints)
+		if (s->hints)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git commit\")"));
 	} else {
 		status_printf_ln(s, color,
 			_("All conflicts fixed but you are still merging."));
-		if (advice_status_hints)
+		if (s->hints)
 			status_printf_ln(s, color,
 				_("  (use \"git commit\" to conclude merge)"));
 	}
@@ -823,7 +823,7 @@ static void show_am_in_progress(struct wt_status *s,
 	if (state->am_empty_patch)
 		status_printf_ln(s, color,
 			_("The current patch is empty."));
-	if (advice_status_hints) {
+	if (s->hints) {
 		if (!state->am_empty_patch)
 			status_printf_ln(s, color,
 				_("  (fix conflicts and then run \"git am --continue\")"));
@@ -896,7 +896,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 		else
 			status_printf_ln(s, color,
 					 _("You are currently rebasing."));
-		if (advice_status_hints) {
+		if (s->hints) {
 			status_printf_ln(s, color,
 				_("  (fix conflicts and then run \"git rebase --continue\")"));
 			status_printf_ln(s, color,
@@ -913,7 +913,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 		else
 			status_printf_ln(s, color,
 					 _("You are currently rebasing."));
-		if (advice_status_hints)
+		if (s->hints)
 			status_printf_ln(s, color,
 				_("  (all conflicts fixed: run \"git rebase --continue\")"));
 	} else if (split_commit_in_progress(s)) {
@@ -925,7 +925,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 		else
 			status_printf_ln(s, color,
 					 _("You are currently splitting a commit during a rebase."));
-		if (advice_status_hints)
+		if (s->hints)
 			status_printf_ln(s, color,
 				_("  (Once your working directory is clean, run \"git rebase --continue\")"));
 	} else {
@@ -937,7 +937,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 		else
 			status_printf_ln(s, color,
 					 _("You are currently editing a commit during a rebase."));
-		if (advice_status_hints && !s->amend) {
+		if (s->hints && !s->amend) {
 			status_printf_ln(s, color,
 				_("  (use \"git commit --amend\" to amend the current commit)"));
 			status_printf_ln(s, color,
@@ -952,7 +952,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 					const char *color)
 {
 	status_printf_ln(s, color, _("You are currently cherry-picking."));
-	if (advice_status_hints) {
+	if (s->hints) {
 		if (has_unmerged(s))
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git cherry-pick --continue\")"));
@@ -971,7 +971,7 @@ static void show_revert_in_progress(struct wt_status *s,
 {
 	status_printf_ln(s, color, _("You are currently reverting commit %s."),
 			 find_unique_abbrev(state->revert_head_sha1, DEFAULT_ABBREV));
-	if (advice_status_hints) {
+	if (s->hints) {
 		if (has_unmerged(s))
 			status_printf_ln(s, color,
 				_("  (fix conflicts and run \"git revert --continue\")"));
@@ -995,7 +995,7 @@ static void show_bisect_in_progress(struct wt_status *s,
 	else
 		status_printf_ln(s, color,
 				 _("You are currently bisecting."));
-	if (advice_status_hints)
+	if (s->hints)
 		status_printf_ln(s, color,
 			_("  (use \"git bisect reset\" to get back to the original branch)"));
 	wt_status_print_trailer(s);
@@ -1233,7 +1233,7 @@ void wt_status_print(struct wt_status *s)
 		}
 	} else if (s->commitable)
 		status_printf_ln(s, GIT_COLOR_NORMAL, _("Untracked files not listed%s"),
-			advice_status_hints
+			s->hints
 			? _(" (use -u option to show untracked files)") : "");
 
 	if (s->verbose)
@@ -1244,25 +1244,25 @@ void wt_status_print(struct wt_status *s)
 		else if (s->nowarn)
 			; /* nothing */
 		else if (s->workdir_dirty) {
-			if (advice_status_hints)
+			if (s->hints)
 				printf(_("no changes added to commit "
 					 "(use \"git add\" and/or \"git commit -a\")\n"));
 			else
 				printf(_("no changes added to commit\n"));
 		} else if (s->untracked.nr) {
-			if (advice_status_hints)
+			if (s->hints)
 				printf(_("nothing added to commit but untracked files "
 					 "present (use \"git add\" to track)\n"));
 			else
 				printf(_("nothing added to commit but untracked files present\n"));
 		} else if (s->is_initial) {
-			if (advice_status_hints)
+			if (s->hints)
 				printf(_("nothing to commit (create/copy files "
 					 "and use \"git add\" to track)\n"));
 			else
 				printf(_("nothing to commit\n"));
 		} else if (!s->show_untracked_files) {
-			if (advice_status_hints)
+			if (s->hints)
 				printf(_("nothing to commit (use -u to show untracked files)\n"));
 			else
 				printf(_("nothing to commit\n"));
diff --git a/wt-status.h b/wt-status.h
index fb7152e..b4c9cb4 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -59,6 +59,7 @@ struct wt_status {
 	unsigned colopts;
 	int null_termination;
 	int show_branch;
+	int hints;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.8.4.8.g834017f

[PATCH v2 3/3] commit: disable status hints when writing to COMMIT_EDITMSG

[Matthieu Moy]

This turns the template COMMIT_EDITMSG from e.g

  # [...]
  # Changes to be committed:
  #   (use "git reset HEAD <file>..." to unstage)
  #
  #	modified:   builtin/commit.c
  #
  # Untracked files:
  #   (use "git add <file>..." to include in what will be committed)
  #
  #	t/foo
  #

to

  # [...]
  # Changes to be committed:
  #	modified:   builtin/commit.c
  #
  # Untracked files:
  #	t/foo
  #

Most status hints were written to be accurate when running "git status"
before running a commit. Many of them are not applicable when the commit
has already been started, and should not be shown in COMMIT_EDITMSG. The
most obvious are hints advising to run "git commit",
"git rebase/am/cherry-pick --continue", which do not make sense when the
command has already been run.

Other messages become slightly inaccurate (e.g. hint to use "git add" to
add untracked files), as the suggested commands are not immediately
applicable during the editing of COMMIT_EDITMSG, but would be applicable
if the commit is aborted. These messages are both potentially helpful and
slightly misleading. This patch chose to remove them too, to avoid
introducing too much complexity in the status code.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Unchanged since v1.

 builtin/commit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 7bfce94..e32246a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -705,6 +705,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (s->fp == NULL)
 		die_errno(_("could not open '%s'"), git_path(commit_editmsg));
 
+	/*
+	 * Most hints are counter-productive when the commit has
+	 * already started.
+	 */
+	s->hints = 0;
+
 	if (clean_message_contents)
 		stripspace(&sb, 0);
 
-- 
1.8.4.8.g834017f