git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/3] commit: fix exit code for --short/--porcelain

From
Samuel Lijin <sxlijin@gmail.com>
Date
Jul 15, 2018, 11:08 UTC
Message-ID
<20180715110807.25544-4-sxlijin@gmail.com>
In-Reply-To
<20180426092524.25264-1-sxlijin@gmail.com>

In wt-status.c, the s->commitable bit is set only in the call tree of wt_longstatus_print(), which means that when there are changes to be committed or all merge conflicts have been resolved, --dry-run and --long return the correct exit code, but --short and --porcelain do not, even though they both imply --dry-run.

Teach wt_status_collect() to set s->committable correctly so that --short and --porcelain return the correct exit code in the above described situations and mark the documenting tests as fixed.

Also stop setting s->committable in wt_longstatus_print_updated() and show_merge_in_progress(), and const-ify wt_status_state in the method signatures in those callpaths.

Signed-off-by: Samuel Lijin <sxlijin@gmail.com>
---
 t/t7501-commit.sh |  8 ++---
 wt-status.c       | 82 +++++++++++++++++++++++++++++------------------
 2 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index be087e73f..b6492322f 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -87,12 +87,12 @@ test_expect_success '--dry-run with stuff to commit returns ok' '
 	git commit -m next -a --dry-run
 '
 
-test_expect_failure '--short with stuff to commit returns ok' '
+test_expect_success '--short with stuff to commit returns ok' '
 	echo bongo bongo bongo >>file &&
 	git commit -m next -a --short
 '
 
-test_expect_failure '--porcelain with stuff to commit returns ok' '
+test_expect_success '--porcelain with stuff to commit returns ok' '
 	echo bongo bongo bongo >>file &&
 	git commit -m next -a --porcelain
 '
@@ -691,11 +691,11 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '
 	git commit --dry-run
 '
 
-test_expect_failure '--short with conflicts fixed from a merge' '
+test_expect_success '--short with conflicts fixed from a merge' '
 	git commit --short
 '
 
-test_expect_failure '--porcelain with conflicts fixed from a merge' '
+test_expect_success '--porcelain with conflicts fixed from a merge' '
 	git commit --porcelain
 '
 
diff --git a/wt-status.c b/wt-status.c
index 75d389944..4ba657978 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -718,6 +718,39 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		s->untracked_in_ms = (getnanotime() - t_begin) / 1000000;
 }
 
+static int has_unmerged(const struct wt_status *s)
+{
+	int i;
+
+	for (i = 0; i < s->change.nr; i++) {
+		struct wt_status_change_data *d;
+		d = s->change.items[i].util;
+		if (d->stagemask)
+			return 1;
+	}
+	return 0;
+}
+
+static void wt_status_mark_committable(
+		struct wt_status *s, const struct wt_status_state *state)
+{
+	int i;
+
+	if (state->merge_in_progress && !has_unmerged(s)) {
+		s->committable = 1;
+		return;
+	}
+
+	for (i = 0; i < s->change.nr; i++) {
+		struct wt_status_change_data *d = (s->change.items[i]).util;
+
+		if (d->index_status && d->index_status != DIFF_STATUS_UNMERGED) {
+			s->committable = 1;
+			return;
+		}
+	}
+}
+
 void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)
 {
 	wt_status_collect_changes_worktree(s);
@@ -728,6 +761,8 @@ void wt_status_collect(struct wt_status *s, const struct wt_status_state *state)
 		wt_status_collect_changes_index(s);
 
 	wt_status_collect_untracked(s);
+
+	wt_status_mark_committable(s, state);
 }
 
 static void wt_longstatus_print_unmerged(const struct wt_status *s)
@@ -753,28 +788,28 @@ static void wt_longstatus_print_unmerged(const struct wt_status *s)
 
 }
 
-static void wt_longstatus_print_updated(struct wt_status *s)
+static void wt_longstatus_print_updated(const struct wt_status *s)
 {
-	int shown_header = 0;
 	int i;
 
+	if (!s->committable) {
+		return;
+	}
+
+	wt_longstatus_print_cached_header(s);
+
 	for (i = 0; i < s->change.nr; i++) {
 		struct wt_status_change_data *d;
 		struct string_list_item *it;
 		it = &(s->change.items[i]);
 		d = it->util;
-		if (!d->index_status ||
-		    d->index_status == DIFF_STATUS_UNMERGED)
-			continue;
-		if (!shown_header) {
-			wt_longstatus_print_cached_header(s);
-			s->committable = 1;
-			shown_header = 1;
+		if (d->index_status &&
+		    d->index_status != DIFF_STATUS_UNMERGED) {
+			wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);
 		}
-		wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);
 	}
-	if (shown_header)
-		wt_longstatus_print_trailer(s);
+
+	wt_longstatus_print_trailer(s);
 }
 
 /*
@@ -1056,21 +1091,7 @@ static void wt_longstatus_print_tracking(const struct wt_status *s)
 	strbuf_release(&sb);
 }
 
-static int has_unmerged(const struct wt_status *s)
-{
-	int i;
-
-	for (i = 0; i < s->change.nr; i++) {
-		struct wt_status_change_data *d;
-		d = s->change.items[i].util;
-		if (d->stagemask)
-			return 1;
-	}
-	return 0;
-}
-
-static void show_merge_in_progress(struct wt_status *s,
-				const struct wt_status_state *state,
+static void show_merge_in_progress(const struct wt_status *s,
 				const char *color)
 {
 	if (has_unmerged(s)) {
@@ -1082,7 +1103,6 @@ static void show_merge_in_progress(struct wt_status *s,
 					 _("  (use \"git merge --abort\" to abort the merge)"));
 		}
 	} else {
-		s-> committable = 1;
 		status_printf_ln(s, color,
 			_("All conflicts fixed but you are still merging."));
 		if (s->hints)
@@ -1576,12 +1596,12 @@ void wt_status_clear_state(struct wt_status_state *state)
 	free(state->detached_from);
 }
 
-static void wt_longstatus_print_state(struct wt_status *s,
+static void wt_longstatus_print_state(const struct wt_status *s,
 				      const struct wt_status_state *state)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
 	if (state->merge_in_progress)
-		show_merge_in_progress(s, state, state_color);
+		show_merge_in_progress(s, state_color);
 	else if (state->am_in_progress)
 		show_am_in_progress(s, state, state_color);
 	else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
@@ -1594,7 +1614,7 @@ static void wt_longstatus_print_state(struct wt_status *s,
 		show_bisect_in_progress(s, state, state_color);
 }
 
-static void wt_longstatus_print(struct wt_status *s, const struct wt_status_state *state)
+static void wt_longstatus_print(const struct wt_status *s, const struct wt_status_state *state)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);

-- 
2.18.0
Previous: Junio C HamanoNext: Junio C Hamano
Message 21 of 26 in “Fix --short and --porcelain options for commit”
  1. 0/2 Fix --short and --porcelain options for commitSamuel Lijin, Apr 18, 2018
  2. 1/2 commit: fix --short and --porcelainSamuel Lijin, Apr 18, 2018
  3. Martin ÅgrenApr 18, 2018
  4. Eric SunshineApr 20, 2018
  5. 2/2 wt-status: const-ify all printf helper methodsSamuel Lijin, Apr 18, 2018
  6. 0/2 Fix --short and --porcelain options for commitSamuel Lijin, Apr 26, 2018
  7. 1/2 commit: fix --short and --porcelain optionsSamuel Lijin, Apr 26, 2018
  8. Junio C HamanoMay 2, 2018
  9. Samuel LijinMay 2, 2018
  10. 2/2 wt-status: const-ify all printf helper methodsSamuel Lijin, Apr 26, 2018
  11. 0/3 Fix --short/--porcelain options for git commitSamuel Lijin, Jul 15, 2018
  12. 0/4 Rerolling patch series to fix t7501Samuel Lijin, Jul 23, 2018
  13. Junio C HamanoJul 30, 2018
  14. 1/4 t7501: add coverage for flags which imply dry runsSamuel Lijin, Jul 23, 2018
  15. 4/4 commit: fix exit code when doing a dry runSamuel Lijin, Jul 23, 2018
  16. 2/4 wt-status: rename commitable to committableSamuel Lijin, Jul 23, 2018
  17. 3/4 wt-status: teach wt_status_collect about merges in progressSamuel Lijin, Jul 23, 2018
  18. 1/3 t7501: add merge conflict tests for dry runSamuel Lijin, Jul 15, 2018
  19. Junio C HamanoJul 17, 2018
  20. Junio C HamanoJul 17, 2018
  21. 3/3 commit: fix exit code for --short/--porcelainSamuel Lijin, Jul 15, 2018
  22. Junio C HamanoJul 17, 2018
  23. Samuel LijinJul 19, 2018
  24. 2/3 wt-status: teach wt_status_collect about merges in progressSamuel Lijin, Jul 15, 2018
  25. Junio C HamanoJul 17, 2018
  26. Samuel LijinApr 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.