git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v5 0/9] status: V2 porcelain status
@ 2016-08-05 22:00 Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 1/9] status: rename long-format print routines Jeff Hostetler
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

This patch series adds porcelain V2 format to status.
This provides detailed information about file changes
and about the current branch.

The new output is accessed via:
    git status --porcelain=v2 [--branch]

This v5 patch series addresses all comments received
today about the v4 patch series. It includes updates
to the manpage and the unit tests and minor cleanup.

New in this series is a change to test-lib-functions.sh
to add lf_to_nul().


Jeff Hostetler (9):
  status: rename long-format print routines
  status: cleanup API to wt_status_print
  status: support --porcelain[=<version>]
  status: collect per-file data for --porcelain=v2
  status: print per-file porcelain v2 status data
  status: print branch info with --porcelain=v2 --branch
  git-status.txt: describe --porcelain=v2 format
  test-lib-functions.sh: Add lf_to_nul
  status: unit tests for --porcelain=v2

 Documentation/git-status.txt | 133 +++++++++-
 builtin/commit.c             |  78 +++---
 t/t7060-wtstatus.sh          |  21 ++
 t/t7064-wtstatus-pv2.sh      | 597 +++++++++++++++++++++++++++++++++++++++++++
 t/test-lib-functions.sh      |   4 +
 wt-status.c                  | 569 ++++++++++++++++++++++++++++++++++++-----
 wt-status.h                  |  19 +-
 7 files changed, 1310 insertions(+), 111 deletions(-)
 create mode 100755 t/t7064-wtstatus-pv2.sh

-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v5 1/9] status: rename long-format print routines
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 2/9] status: cleanup API to wt_status_print Jeff Hostetler
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Rename the various wt_status_print*() routines to be
wt_longstatus_print*() to make it clear that these
routines are only concerned with the normal/long
status output and reduce developer confusion as other
status formats are added in the future.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 builtin/commit.c |   4 +-
 wt-status.c      | 110 +++++++++++++++++++++++++++----------------------------
 wt-status.h      |   2 +-
 3 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1f6dbcd..b80273b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -515,7 +515,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 		break;
 	case STATUS_FORMAT_NONE:
 	case STATUS_FORMAT_LONG:
-		wt_status_print(s);
+		wt_longstatus_print(s);
 		break;
 	}
 
@@ -1403,7 +1403,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
 		s.ignore_submodule_arg = ignore_submodule_arg;
-		wt_status_print(&s);
+		wt_longstatus_print(&s);
 		break;
 	}
 	return 0;
diff --git a/wt-status.c b/wt-status.c
index de62ab2..b9a58fd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -139,7 +139,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->display_comment_prefix = 0;
 }
 
-static void wt_status_print_unmerged_header(struct wt_status *s)
+static void wt_longstatus_print_unmerged_header(struct wt_status *s)
 {
 	int i;
 	int del_mod_conflict = 0;
@@ -191,7 +191,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 	status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_cached_header(struct wt_status *s)
+static void wt_longstatus_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 
@@ -207,9 +207,9 @@ static void wt_status_print_cached_header(struct wt_status *s)
 	status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_dirty_header(struct wt_status *s,
-					 int has_deleted,
-					 int has_dirty_submodules)
+static void wt_longstatus_print_dirty_header(struct wt_status *s,
+					     int has_deleted,
+					     int has_dirty_submodules)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 
@@ -226,9 +226,9 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 	status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_other_header(struct wt_status *s,
-					 const char *what,
-					 const char *how)
+static void wt_longstatus_print_other_header(struct wt_status *s,
+					     const char *what,
+					     const char *how)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	status_printf_ln(s, c, "%s:", what);
@@ -238,7 +238,7 @@ static void wt_status_print_other_header(struct wt_status *s,
 	status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_trailer(struct wt_status *s)
+static void wt_longstatus_print_trailer(struct wt_status *s)
 {
 	status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 }
@@ -304,8 +304,8 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval)
 	return result;
 }
 
-static void wt_status_print_unmerged_data(struct wt_status *s,
-					  struct string_list_item *it)
+static void wt_longstatus_print_unmerged_data(struct wt_status *s,
+					      struct string_list_item *it)
 {
 	const char *c = color(WT_STATUS_UNMERGED, s);
 	struct wt_status_change_data *d = it->util;
@@ -331,9 +331,9 @@ static void wt_status_print_unmerged_data(struct wt_status *s,
 	strbuf_release(&onebuf);
 }
 
-static void wt_status_print_change_data(struct wt_status *s,
-					int change_type,
-					struct string_list_item *it)
+static void wt_longstatus_print_change_data(struct wt_status *s,
+					    int change_type,
+					    struct string_list_item *it)
 {
 	struct wt_status_change_data *d = it->util;
 	const char *c = color(change_type, s);
@@ -378,7 +378,7 @@ static void wt_status_print_change_data(struct wt_status *s,
 		status = d->worktree_status;
 		break;
 	default:
-		die("BUG: unhandled change_type %d in wt_status_print_change_data",
+		die("BUG: unhandled change_type %d in wt_longstatus_print_change_data",
 		    change_type);
 	}
 
@@ -627,7 +627,7 @@ void wt_status_collect(struct wt_status *s)
 	wt_status_collect_untracked(s);
 }
 
-static void wt_status_print_unmerged(struct wt_status *s)
+static void wt_longstatus_print_unmerged(struct wt_status *s)
 {
 	int shown_header = 0;
 	int i;
@@ -640,17 +640,17 @@ static void wt_status_print_unmerged(struct wt_status *s)
 		if (!d->stagemask)
 			continue;
 		if (!shown_header) {
-			wt_status_print_unmerged_header(s);
+			wt_longstatus_print_unmerged_header(s);
 			shown_header = 1;
 		}
-		wt_status_print_unmerged_data(s, it);
+		wt_longstatus_print_unmerged_data(s, it);
 	}
 	if (shown_header)
-		wt_status_print_trailer(s);
+		wt_longstatus_print_trailer(s);
 
 }
 
-static void wt_status_print_updated(struct wt_status *s)
+static void wt_longstatus_print_updated(struct wt_status *s)
 {
 	int shown_header = 0;
 	int i;
@@ -664,14 +664,14 @@ static void wt_status_print_updated(struct wt_status *s)
 		    d->index_status == DIFF_STATUS_UNMERGED)
 			continue;
 		if (!shown_header) {
-			wt_status_print_cached_header(s);
+			wt_longstatus_print_cached_header(s);
 			s->commitable = 1;
 			shown_header = 1;
 		}
-		wt_status_print_change_data(s, WT_STATUS_UPDATED, it);
+		wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);
 	}
 	if (shown_header)
-		wt_status_print_trailer(s);
+		wt_longstatus_print_trailer(s);
 }
 
 /*
@@ -703,7 +703,7 @@ static int wt_status_check_worktree_changes(struct wt_status *s,
 	return changes;
 }
 
-static void wt_status_print_changed(struct wt_status *s)
+static void wt_longstatus_print_changed(struct wt_status *s)
 {
 	int i, dirty_submodules;
 	int worktree_changes = wt_status_check_worktree_changes(s, &dirty_submodules);
@@ -711,7 +711,7 @@ static void wt_status_print_changed(struct wt_status *s)
 	if (!worktree_changes)
 		return;
 
-	wt_status_print_dirty_header(s, worktree_changes < 0, dirty_submodules);
+	wt_longstatus_print_dirty_header(s, worktree_changes < 0, dirty_submodules);
 
 	for (i = 0; i < s->change.nr; i++) {
 		struct wt_status_change_data *d;
@@ -721,12 +721,12 @@ static void wt_status_print_changed(struct wt_status *s)
 		if (!d->worktree_status ||
 		    d->worktree_status == DIFF_STATUS_UNMERGED)
 			continue;
-		wt_status_print_change_data(s, WT_STATUS_CHANGED, it);
+		wt_longstatus_print_change_data(s, WT_STATUS_CHANGED, it);
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
-static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitted)
+static void wt_longstatus_print_submodule_summary(struct wt_status *s, int uncommitted)
 {
 	struct child_process sm_summary = CHILD_PROCESS_INIT;
 	struct strbuf cmd_stdout = STRBUF_INIT;
@@ -772,10 +772,10 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	strbuf_release(&summary);
 }
 
-static void wt_status_print_other(struct wt_status *s,
-				  struct string_list *l,
-				  const char *what,
-				  const char *how)
+static void wt_longstatus_print_other(struct wt_status *s,
+				      struct string_list *l,
+				      const char *what,
+				      const char *how)
 {
 	int i;
 	struct strbuf buf = STRBUF_INIT;
@@ -785,7 +785,7 @@ static void wt_status_print_other(struct wt_status *s,
 	if (!l->nr)
 		return;
 
-	wt_status_print_other_header(s, what, how);
+	wt_longstatus_print_other_header(s, what, how);
 
 	for (i = 0; i < l->nr; i++) {
 		struct string_list_item *it;
@@ -845,7 +845,7 @@ void wt_status_add_cut_line(FILE *fp)
 	strbuf_release(&buf);
 }
 
-static void wt_status_print_verbose(struct wt_status *s)
+static void wt_longstatus_print_verbose(struct wt_status *s)
 {
 	struct rev_info rev;
 	struct setup_revision_opt opt;
@@ -878,7 +878,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	if (s->verbose > 1 && s->commitable) {
 		/* print_updated() printed a header, so do we */
 		if (s->fp != stdout)
-			wt_status_print_trailer(s);
+			wt_longstatus_print_trailer(s);
 		status_printf_ln(s, c, _("Changes to be committed:"));
 		rev.diffopt.a_prefix = "c/";
 		rev.diffopt.b_prefix = "i/";
@@ -896,7 +896,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	}
 }
 
-static void wt_status_print_tracking(struct wt_status *s)
+static void wt_longstatus_print_tracking(struct wt_status *s)
 {
 	struct strbuf sb = STRBUF_INIT;
 	const char *cp, *ep, *branch_name;
@@ -959,7 +959,7 @@ static void show_merge_in_progress(struct wt_status *s,
 			status_printf_ln(s, color,
 				_("  (use \"git commit\" to conclude merge)"));
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 static void show_am_in_progress(struct wt_status *s,
@@ -980,7 +980,7 @@ static void show_am_in_progress(struct wt_status *s,
 		status_printf_ln(s, color,
 			_("  (use \"git am --abort\" to restore the original branch)"));
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 static char *read_line_from_git_path(const char *filename)
@@ -1204,7 +1204,7 @@ static void show_rebase_in_progress(struct wt_status *s,
 				_("  (use \"git rebase --continue\" once you are satisfied with your changes)"));
 		}
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 static void show_cherry_pick_in_progress(struct wt_status *s,
@@ -1223,7 +1223,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s,
 		status_printf_ln(s, color,
 			_("  (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)"));
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 static void show_revert_in_progress(struct wt_status *s,
@@ -1242,7 +1242,7 @@ static void show_revert_in_progress(struct wt_status *s,
 		status_printf_ln(s, color,
 			_("  (use \"git revert --abort\" to cancel the revert operation)"));
 	}
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 static void show_bisect_in_progress(struct wt_status *s,
@@ -1259,7 +1259,7 @@ static void show_bisect_in_progress(struct wt_status *s,
 	if (s->hints)
 		status_printf_ln(s, color,
 			_("  (use \"git bisect reset\" to get back to the original branch)"));
-	wt_status_print_trailer(s);
+	wt_longstatus_print_trailer(s);
 }
 
 /*
@@ -1429,8 +1429,8 @@ void wt_status_get_state(struct wt_status_state *state,
 		wt_status_get_detached_from(state);
 }
 
-static void wt_status_print_state(struct wt_status *s,
-				  struct wt_status_state *state)
+static void wt_longstatus_print_state(struct wt_status *s,
+				      struct wt_status_state *state)
 {
 	const char *state_color = color(WT_STATUS_HEADER, s);
 	if (state->merge_in_progress)
@@ -1447,7 +1447,7 @@ static void wt_status_print_state(struct wt_status *s,
 		show_bisect_in_progress(s, state, state_color);
 }
 
-void wt_status_print(struct wt_status *s)
+void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
@@ -1484,10 +1484,10 @@ void wt_status_print(struct wt_status *s)
 		status_printf_more(s, branch_status_color, "%s", on_what);
 		status_printf_more(s, branch_color, "%s\n", branch_name);
 		if (!s->is_initial)
-			wt_status_print_tracking(s);
+			wt_longstatus_print_tracking(s);
 	}
 
-	wt_status_print_state(s, &state);
+	wt_longstatus_print_state(s, &state);
 	free(state.branch);
 	free(state.onto);
 	free(state.detached_from);
@@ -1498,19 +1498,19 @@ void wt_status_print(struct wt_status *s)
 		status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 	}
 
-	wt_status_print_updated(s);
-	wt_status_print_unmerged(s);
-	wt_status_print_changed(s);
+	wt_longstatus_print_updated(s);
+	wt_longstatus_print_unmerged(s);
+	wt_longstatus_print_changed(s);
 	if (s->submodule_summary &&
 	    (!s->ignore_submodule_arg ||
 	     strcmp(s->ignore_submodule_arg, "all"))) {
-		wt_status_print_submodule_summary(s, 0);  /* staged */
-		wt_status_print_submodule_summary(s, 1);  /* unstaged */
+		wt_longstatus_print_submodule_summary(s, 0);  /* staged */
+		wt_longstatus_print_submodule_summary(s, 1);  /* unstaged */
 	}
 	if (s->show_untracked_files) {
-		wt_status_print_other(s, &s->untracked, _("Untracked files"), "add");
+		wt_longstatus_print_other(s, &s->untracked, _("Untracked files"), "add");
 		if (s->show_ignored_files)
-			wt_status_print_other(s, &s->ignored, _("Ignored files"), "add -f");
+			wt_longstatus_print_other(s, &s->ignored, _("Ignored files"), "add -f");
 		if (advice_status_u_option && 2000 < s->untracked_in_ms) {
 			status_printf_ln(s, GIT_COLOR_NORMAL, "%s", "");
 			status_printf_ln(s, GIT_COLOR_NORMAL,
@@ -1525,7 +1525,7 @@ void wt_status_print(struct wt_status *s)
 			? _(" (use -u option to show untracked files)") : "");
 
 	if (s->verbose)
-		wt_status_print_verbose(s);
+		wt_longstatus_print_verbose(s);
 	if (!s->commitable) {
 		if (s->amend)
 			status_printf_ln(s, GIT_COLOR_NORMAL, _("No changes"));
diff --git a/wt-status.h b/wt-status.h
index 2ca93f6..2023a3c 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -99,7 +99,6 @@ struct wt_status_state {
 void wt_status_truncate_message_at_cut_line(struct strbuf *);
 void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct wt_status *s);
-void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 int wt_status_check_rebase(const struct worktree *wt,
@@ -107,6 +106,7 @@ int wt_status_check_rebase(const struct worktree *wt,
 int wt_status_check_bisect(const struct worktree *wt,
 			   struct wt_status_state *state);
 
+void wt_longstatus_print(struct wt_status *s);
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
 
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 2/9] status: cleanup API to wt_status_print
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 1/9] status: rename long-format print routines Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 3/9] status: support --porcelain[=<version>] Jeff Hostetler
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Refactor the API between builtin/commit.c and wt-status.[ch].

Hide the details of the various wt_*status_print() routines inside
wt-status.c behind a single (new) wt_status_print() routine.
Eliminate the switch statements from builtin/commit.c.
Allow details of new status formats to be isolated within wt-status.c

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 builtin/commit.c | 51 +++++++++------------------------------------------
 wt-status.c      | 25 ++++++++++++++++++++++---
 wt-status.h      | 16 ++++++++++++----
 3 files changed, 43 insertions(+), 49 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index b80273b..a792deb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -142,14 +142,7 @@ static int show_ignored_in_status, have_option_m;
 static const char *only_include_assumed;
 static struct strbuf message = STRBUF_INIT;
 
-static enum status_format {
-	STATUS_FORMAT_NONE = 0,
-	STATUS_FORMAT_LONG,
-	STATUS_FORMAT_SHORT,
-	STATUS_FORMAT_PORCELAIN,
-
-	STATUS_FORMAT_UNSPECIFIED
-} status_format = STATUS_FORMAT_UNSPECIFIED;
+static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
@@ -500,24 +493,11 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	s->fp = fp;
 	s->nowarn = nowarn;
 	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+	s->status_format = status_format;
+	s->ignore_submodule_arg = ignore_submodule_arg;
 
 	wt_status_collect(s);
-
-	switch (status_format) {
-	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(s);
-		break;
-	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(s);
-		break;
-	case STATUS_FORMAT_UNSPECIFIED:
-		die("BUG: finalize_deferred_config() should have been called");
-		break;
-	case STATUS_FORMAT_NONE:
-	case STATUS_FORMAT_LONG:
-		wt_longstatus_print(s);
-		break;
-	}
+	wt_status_print(s);
 
 	return s->commitable;
 }
@@ -1099,7 +1079,7 @@ static const char *read_commit_message(const char *name)
  * is not in effect here.
  */
 static struct status_deferred_config {
-	enum status_format status_format;
+	enum wt_status_format status_format;
 	int show_branch;
 } status_deferred_config = {
 	STATUS_FORMAT_UNSPECIFIED,
@@ -1381,6 +1361,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
 	s.ignore_submodule_arg = ignore_submodule_arg;
+	s.status_format = status_format;
+	s.verbose = verbose;
+
 	wt_status_collect(&s);
 
 	if (0 <= fd)
@@ -1389,23 +1372,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (s.relative_paths)
 		s.prefix = prefix;
 
-	switch (status_format) {
-	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(&s);
-		break;
-	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(&s);
-		break;
-	case STATUS_FORMAT_UNSPECIFIED:
-		die("BUG: finalize_deferred_config() should have been called");
-		break;
-	case STATUS_FORMAT_NONE:
-	case STATUS_FORMAT_LONG:
-		s.verbose = verbose;
-		s.ignore_submodule_arg = ignore_submodule_arg;
-		wt_longstatus_print(&s);
-		break;
-	}
+	wt_status_print(&s);
 	return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index b9a58fd..a9031e4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1447,7 +1447,7 @@ static void wt_longstatus_print_state(struct wt_status *s,
 		show_bisect_in_progress(s, state, state_color);
 }
 
-void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print(struct wt_status *s)
 {
 	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
 	const char *branch_status_color = color(WT_STATUS_HEADER, s);
@@ -1714,7 +1714,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
-void wt_shortstatus_print(struct wt_status *s)
+static void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
 
@@ -1746,7 +1746,7 @@ void wt_shortstatus_print(struct wt_status *s)
 	}
 }
 
-void wt_porcelain_print(struct wt_status *s)
+static void wt_porcelain_print(struct wt_status *s)
 {
 	s->use_color = 0;
 	s->relative_paths = 0;
@@ -1754,3 +1754,22 @@ void wt_porcelain_print(struct wt_status *s)
 	s->no_gettext = 1;
 	wt_shortstatus_print(s);
 }
+
+void wt_status_print(struct wt_status *s)
+{
+	switch (s->status_format) {
+	case STATUS_FORMAT_SHORT:
+		wt_shortstatus_print(s);
+		break;
+	case STATUS_FORMAT_PORCELAIN:
+		wt_porcelain_print(s);
+		break;
+	case STATUS_FORMAT_UNSPECIFIED:
+		die("BUG: finalize_deferred_config() should have been called");
+		break;
+	case STATUS_FORMAT_NONE:
+	case STATUS_FORMAT_LONG:
+		wt_longstatus_print(s);
+		break;
+	}
+}
diff --git a/wt-status.h b/wt-status.h
index 2023a3c..9389076 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -43,6 +43,15 @@ struct wt_status_change_data {
 	unsigned new_submodule_commits : 1;
 };
 
+enum wt_status_format {
+	STATUS_FORMAT_NONE = 0,
+	STATUS_FORMAT_LONG,
+	STATUS_FORMAT_SHORT,
+	STATUS_FORMAT_PORCELAIN,
+
+	STATUS_FORMAT_UNSPECIFIED
+};
+
 struct wt_status {
 	int is_initial;
 	char *branch;
@@ -66,6 +75,8 @@ struct wt_status {
 	int show_branch;
 	int hints;
 
+	enum wt_status_format status_format;
+
 	/* These are computed during processing of the individual sections */
 	int commitable;
 	int workdir_dirty;
@@ -99,6 +110,7 @@ struct wt_status_state {
 void wt_status_truncate_message_at_cut_line(struct strbuf *);
 void wt_status_add_cut_line(FILE *fp);
 void wt_status_prepare(struct wt_status *s);
+void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 int wt_status_check_rebase(const struct worktree *wt,
@@ -106,10 +118,6 @@ int wt_status_check_rebase(const struct worktree *wt,
 int wt_status_check_bisect(const struct worktree *wt,
 			   struct wt_status_state *state);
 
-void wt_longstatus_print(struct wt_status *s);
-void wt_shortstatus_print(struct wt_status *s);
-void wt_porcelain_print(struct wt_status *s);
-
 __attribute__((format (printf, 3, 4)))
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);
 __attribute__((format (printf, 3, 4)))
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 3/9] status: support --porcelain[=<version>]
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 1/9] status: rename long-format print routines Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 2/9] status: cleanup API to wt_status_print Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 4/9] status: collect per-file data for --porcelain=v2 Jeff Hostetler
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Update --porcelain argument to take optional version parameter
to allow multiple porcelain formats to be supported in the future.

The token "v1" is the default value and indicates the traditional
porcelain format.  (The token "1" is an alias for that.)

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 Documentation/git-status.txt |  7 +++++--
 builtin/commit.c             | 21 ++++++++++++++++++---
 t/t7060-wtstatus.sh          | 21 +++++++++++++++++++++
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index e1e8f57..6b1454b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -32,11 +32,14 @@ OPTIONS
 --branch::
 	Show the branch and tracking info even in short-format.
 
---porcelain::
+--porcelain[=<version>]::
 	Give the output in an easy-to-parse format for scripts.
 	This is similar to the short output, but will remain stable
 	across Git versions and regardless of user configuration. See
 	below for details.
++
+The version parameter is used to specify the format version.
+This is optional and defaults to the original version 'v1' format.
 
 --long::
 	Give the output in the long-format. This is the default.
@@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1].
 
 -z::
 	Terminate entries with NUL, instead of LF.  This implies
-	the `--porcelain` output format if no other format is given.
+	the `--porcelain=v1` output format if no other format is given.
 
 --column[=<options>]::
 --no-column::
diff --git a/builtin/commit.c b/builtin/commit.c
index a792deb..185ac35 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -144,6 +144,21 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
+{
+	enum wt_status_format *value = (enum wt_status_format *)opt->value;
+	if (unset)
+		*value = STATUS_FORMAT_NONE;
+	else if (!arg)
+		*value = STATUS_FORMAT_PORCELAIN;
+	else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
+		*value = STATUS_FORMAT_PORCELAIN;
+	else
+		die("unsupported porcelain version '%s'", arg);
+
+	return 0;
+}
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
 	struct strbuf *buf = opt->value;
@@ -1316,9 +1331,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 			    N_("show status concisely"), STATUS_FORMAT_SHORT),
 		OPT_BOOL('b', "branch", &s.show_branch,
 			 N_("show branch information")),
-		OPT_SET_INT(0, "porcelain", &status_format,
-			    N_("machine-readable output"),
-			    STATUS_FORMAT_PORCELAIN),
+		{ OPTION_CALLBACK, 0, "porcelain", &status_format,
+		  N_("version"), N_("machine-readable output"),
+		  PARSE_OPT_OPTARG, opt_parse_porcelain },
 		OPT_SET_INT(0, "long", &status_format,
 			    N_("show status in long format (default)"),
 			    STATUS_FORMAT_LONG),
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 44bf1d8..00e0ceb 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -228,4 +228,25 @@ test_expect_success 'status --branch with detached HEAD' '
 	test_i18ncmp expected actual
 '
 
+## Duplicate the above test and verify --porcelain=v1 arg parsing.
+test_expect_success 'status --porcelain=v1 --branch with detached HEAD' '
+	git reset --hard &&
+	git checkout master^0 &&
+	git status --branch --porcelain=v1 >actual &&
+	cat >expected <<-EOF &&
+	## HEAD (no branch)
+	?? .gitconfig
+	?? actual
+	?? expect
+	?? expected
+	?? mdconflict/
+	EOF
+	test_i18ncmp expected actual
+'
+
+## Verify parser error on invalid --porcelain argument.
+test_expect_success 'status --porcelain=bogus' '
+	test_must_fail git status --porcelain=bogus
+'
+
 test_done
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 4/9] status: collect per-file data for --porcelain=v2
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (2 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 3/9] status: support --porcelain[=<version>] Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:48   ` Junio C Hamano
  2016-08-05 22:00 ` [PATCH v5 5/9] status: print per-file porcelain v2 status data Jeff Hostetler
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Collect extra per-file data for porcelain V2 format.

The output of `git status --porcelain` leaves out many
details about the current status that clients might like
to have.  This can force them to be less efficient as they
may need to launch secondary commands (and try to match
the logic within git) to accumulate this extra information.
For example, a GUI IDE might want the file mode to display
the correct icon for a changed item (without having to stat
it afterwards).

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 builtin/commit.c |  3 +++
 wt-status.c      | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 wt-status.h      |  4 ++++
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 185ac35..3d222d3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un
 		*value = STATUS_FORMAT_PORCELAIN;
 	else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
 		*value = STATUS_FORMAT_PORCELAIN;
+	else if (!strcmp(arg, "v2") || !strcmp(arg, "2"))
+		*value = STATUS_FORMAT_PORCELAIN_V2;
 	else
 		die("unsupported porcelain version '%s'", arg);
 
@@ -1104,6 +1106,7 @@ static struct status_deferred_config {
 static void finalize_deferred_config(struct wt_status *s)
 {
 	int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+				   status_format != STATUS_FORMAT_PORCELAIN_V2 &&
 				   !s->null_termination);
 
 	if (s->null_termination) {
diff --git a/wt-status.c b/wt-status.c
index a9031e4..ca8023e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -434,6 +434,31 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
 		if (S_ISGITLINK(p->two->mode))
 			d->new_submodule_commits = !!oidcmp(&p->one->oid,
 							    &p->two->oid);
+
+		switch (p->status) {
+		case DIFF_STATUS_ADDED:
+			die("BUG: worktree status add???");
+			break;
+
+		case DIFF_STATUS_DELETED:
+			d->mode_index = p->one->mode;
+			oidcpy(&d->oid_index, &p->one->oid);
+			/* mode_worktree is zero for a delete. */
+			break;
+
+		case DIFF_STATUS_MODIFIED:
+		case DIFF_STATUS_TYPE_CHANGED:
+		case DIFF_STATUS_UNMERGED:
+			d->mode_index = p->one->mode;
+			d->mode_worktree = p->two->mode;
+			oidcpy(&d->oid_index, &p->one->oid);
+			break;
+
+		case DIFF_STATUS_UNKNOWN:
+			die("BUG: worktree status unknown???");
+			break;
+		}
+
 	}
 }
 
@@ -479,12 +504,36 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
 		if (!d->index_status)
 			d->index_status = p->status;
 		switch (p->status) {
+		case DIFF_STATUS_ADDED:
+			/* Leave {mode,oid}_head zero for an add. */
+			d->mode_index = p->two->mode;
+			oidcpy(&d->oid_index, &p->two->oid);
+			break;
+		case DIFF_STATUS_DELETED:
+			d->mode_head = p->one->mode;
+			oidcpy(&d->oid_head, &p->one->oid);
+			/* Leave {mode,oid}_index zero for a delete. */
+			break;
+
 		case DIFF_STATUS_COPIED:
 		case DIFF_STATUS_RENAMED:
 			d->head_path = xstrdup(p->one->path);
+			d->score = p->score * 100 / MAX_SCORE;
+			/* fallthru */
+		case DIFF_STATUS_MODIFIED:
+		case DIFF_STATUS_TYPE_CHANGED:
+			d->mode_head = p->one->mode;
+			d->mode_index = p->two->mode;
+			oidcpy(&d->oid_head, &p->one->oid);
+			oidcpy(&d->oid_index, &p->two->oid);
 			break;
 		case DIFF_STATUS_UNMERGED:
 			d->stagemask = unmerged_mask(p->two->path);
+			/*
+			 * Don't bother setting {mode,oid}_{head,index} since the print
+			 * code will output the stage values directly and not use the
+			 * values in these fields.
+			 */
 			break;
 		}
 	}
@@ -565,9 +614,18 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
 		if (ce_stage(ce)) {
 			d->index_status = DIFF_STATUS_UNMERGED;
 			d->stagemask |= (1 << (ce_stage(ce) - 1));
+			/*
+			 * Don't bother setting {mode,oid}_{head,index} since the print
+			 * code will output the stage values directly and not use the
+			 * values in these fields.
+			 */
 		}
-		else
+		else {
 			d->index_status = DIFF_STATUS_ADDED;
+			/* Leave {mode,oid}_head zero for adds. */
+			d->mode_index = ce->ce_mode;
+			hashcpy(d->oid_index.hash, ce->sha1);
+		}
 	}
 }
 
@@ -1764,6 +1822,9 @@ void wt_status_print(struct wt_status *s)
 	case STATUS_FORMAT_PORCELAIN:
 		wt_porcelain_print(s);
 		break;
+	case STATUS_FORMAT_PORCELAIN_V2:
+		/* TODO */
+		break;
 	case STATUS_FORMAT_UNSPECIFIED:
 		die("BUG: finalize_deferred_config() should have been called");
 		break;
diff --git a/wt-status.h b/wt-status.h
index 9389076..43fd3fc 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -38,6 +38,9 @@ struct wt_status_change_data {
 	int worktree_status;
 	int index_status;
 	int stagemask;
+	int score;
+	int mode_head, mode_index, mode_worktree;
+	struct object_id oid_head, oid_index;
 	char *head_path;
 	unsigned dirty_submodule       : 2;
 	unsigned new_submodule_commits : 1;
@@ -48,6 +51,7 @@ enum wt_status_format {
 	STATUS_FORMAT_LONG,
 	STATUS_FORMAT_SHORT,
 	STATUS_FORMAT_PORCELAIN,
+	STATUS_FORMAT_PORCELAIN_V2,
 
 	STATUS_FORMAT_UNSPECIFIED
 };
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 5/9] status: print per-file porcelain v2 status data
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (3 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 4/9] status: collect per-file data for --porcelain=v2 Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:51   ` Junio C Hamano
  2016-08-05 22:00 ` [PATCH v5 6/9] status: print branch info with --porcelain=v2 --branch Jeff Hostetler
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Print per-file information in porcelain v2 format.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 wt-status.c | 285 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 284 insertions(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index ca8023e..4b6a090 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1813,6 +1813,289 @@ static void wt_porcelain_print(struct wt_status *s)
 	wt_shortstatus_print(s);
 }
 
+/*
+ * Convert various submodule status values into a
+ * fixed-length string of characters in the buffer provided.
+ */
+static void wt_porcelain_v2_submodule_state(
+	struct wt_status_change_data *d,
+	char sub[5])
+{
+	if (S_ISGITLINK(d->mode_head) ||
+		S_ISGITLINK(d->mode_index) ||
+		S_ISGITLINK(d->mode_worktree)) {
+		sub[0] = 'S';
+		sub[1] = d->new_submodule_commits ? 'C' : '.';
+		sub[2] = (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED) ? 'M' : '.';
+		sub[3] = (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ? 'U' : '.';
+	} else {
+		sub[0] = 'N';
+		sub[1] = '.';
+		sub[2] = '.';
+		sub[3] = '.';
+	}
+	sub[4] = 0;
+}
+
+/*
+ * Fix-up changed entries before we print them.
+ */
+static void wt_porcelain_v2_fix_up_changed(
+	struct string_list_item *it,
+	struct wt_status *s)
+{
+	struct wt_status_change_data *d = it->util;
+
+	if (!d->index_status) {
+		/*
+		 * This entry is unchanged in the index (relative to the head).
+		 * Therefore, the collect_updated_cb was never called for this
+		 * entry (during the head-vs-index scan) and so the head column
+		 * fields were never set.
+		 *
+		 * We must have data for the index column (from the
+		 * index-vs-worktree scan (otherwise, this entry should not be
+		 * in the list of changes)).
+		 *
+		 * Copy index column fields to the head column, so that our
+		 * output looks complete.
+		 */
+		assert(d->mode_head == 0);
+		d->mode_head = d->mode_index;
+		oidcpy(&d->oid_head, &d->oid_index);
+	}
+
+	if (!d->worktree_status) {
+		/*
+		 * This entry is unchanged in the worktree (relative to the index).
+		 * Therefore, the collect_changed_cb was never called for this entry
+		 * (during the index-vs-worktree scan) and so the worktree column
+		 * fields were never set.
+		 *
+		 * We must have data for the index column (from the head-vs-index
+		 * scan).
+		 *
+		 * Copy the index column fields to the worktree column so that
+		 * our output looks complete.
+		 *
+		 * Note that we only have a mode field in the worktree column
+		 * because the scan code tries really hard to not have to compute it.
+		 */
+		assert(d->mode_worktree == 0);
+		d->mode_worktree = d->mode_index;
+	}
+}
+
+/*
+ * Print porcelain v2 info for tracked entries with changes.
+ */
+static void wt_porcelain_v2_print_changed_entry(
+	struct string_list_item *it,
+	struct wt_status *s)
+{
+	struct wt_status_change_data *d = it->util;
+	struct strbuf buf_index = STRBUF_INIT;
+	struct strbuf buf_head = STRBUF_INIT;
+	const char *path_index = NULL;
+	const char *path_head = NULL;
+	char key[3];
+	char submodule_token[5];
+	char sep_char, eol_char;
+
+	wt_porcelain_v2_fix_up_changed(it, s);
+	wt_porcelain_v2_submodule_state(d, submodule_token);
+
+	key[0] = d->index_status ? d->index_status : '.';
+	key[1] = d->worktree_status ? d->worktree_status : '.';
+	key[2] = 0;
+
+	if (s->null_termination) {
+		/*
+		 * In -z mode, we DO NOT C-quote pathnames.  Current path is ALWAYS first.
+		 * A single NUL character separates them.
+		 */
+		sep_char = '\0';
+		eol_char = '\0';
+		path_index = it->string;
+		path_head = d->head_path;
+	} else {
+		/*
+		 * Path(s) are C-quoted if necessary. Current path is ALWAYS first.
+		 * The source path is only present when necessary.
+		 * A single TAB separates them (because paths can contain spaces
+		 * which are not escaped and C-quoting does escape TAB characters).
+		 */
+		sep_char = '\t';
+		eol_char = '\n';
+		path_index = quote_path(it->string, s->prefix, &buf_index);
+		if (d->head_path)
+			path_head = quote_path(d->head_path, s->prefix, &buf_head);
+	}
+
+	if (path_head)
+		fprintf(s->fp, "2 %s %s %06o %06o %06o %s %s %c%d %s%c%s%c",
+				key, submodule_token,
+				d->mode_head, d->mode_index, d->mode_worktree,
+				oid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),
+				key[0], d->score,
+				path_index, sep_char, path_head, eol_char);
+	else
+		fprintf(s->fp, "1 %s %s %06o %06o %06o %s %s %s%c",
+				key, submodule_token,
+				d->mode_head, d->mode_index, d->mode_worktree,
+				oid_to_hex(&d->oid_head), oid_to_hex(&d->oid_index),
+				path_index, eol_char);
+
+	strbuf_release(&buf_index);
+	strbuf_release(&buf_head);
+}
+
+/*
+ * Print porcelain v2 status info for unmerged entries.
+ */
+static void wt_porcelain_v2_print_unmerged_entry(
+	struct string_list_item *it,
+	struct wt_status *s)
+{
+	struct wt_status_change_data *d = it->util;
+	const struct cache_entry *ce;
+	struct strbuf buf_index = STRBUF_INIT;
+	const char *path_index = NULL;
+	int pos, stage, sum;
+	struct {
+		int mode;
+		struct object_id oid;
+	} stages[3];
+	char *key;
+	char submodule_token[5];
+	char unmerged_prefix = 'u';
+	char eol_char = s->null_termination ? '\0' : '\n';
+
+	wt_porcelain_v2_submodule_state(d, submodule_token);
+
+	switch (d->stagemask) {
+	case 1: key = "DD"; break; /* both deleted */
+	case 2: key = "AU"; break; /* added by us */
+	case 3: key = "UD"; break; /* deleted by them */
+	case 4: key = "UA"; break; /* added by them */
+	case 5: key = "DU"; break; /* deleted by us */
+	case 6: key = "AA"; break; /* both added */
+	case 7: key = "UU"; break; /* both modified */
+	default:
+		die("BUG: unhandled unmerged status %x", d->stagemask);
+	}
+
+	/*
+	 * Disregard d.aux.porcelain_v2 data that we accumulated
+	 * for the head and index columns during the scans and
+	 * replace with the actual stage data.
+	 *
+	 * Note that this is a last-one-wins for each the individual
+	 * stage [123] columns in the event of multiple cache entries
+	 * for same stage.
+	 */
+	memset(stages, 0, sizeof(stages));
+	sum = 0;
+	pos = cache_name_pos(it->string, strlen(it->string));
+	assert(pos < 0);
+	pos = -pos-1;
+	while (pos < active_nr) {
+		ce = active_cache[pos++];
+		stage = ce_stage(ce);
+		if (strcmp(ce->name, it->string) || !stage)
+			break;
+		stages[stage - 1].mode = ce->ce_mode;
+		hashcpy(stages[stage - 1].oid.hash, ce->sha1);
+		sum |= (1 << (stage - 1));
+	}
+	if (sum != d->stagemask)
+		die("BUG: observed stagemask 0x%x != expected stagemask 0x%x", sum, d->stagemask);
+
+	if (s->null_termination)
+		path_index = it->string;
+	else
+		path_index = quote_path(it->string, s->prefix, &buf_index);
+
+	fprintf(s->fp, "%c %s %s %06o %06o %06o %06o %s %s %s %s%c",
+			unmerged_prefix, key, submodule_token,
+			stages[0].mode, /* stage 1 */
+			stages[1].mode, /* stage 2 */
+			stages[2].mode, /* stage 3 */
+			d->mode_worktree,
+			oid_to_hex(&stages[0].oid), /* stage 1 */
+			oid_to_hex(&stages[1].oid), /* stage 2 */
+			oid_to_hex(&stages[2].oid), /* stage 3 */
+			path_index,
+			eol_char);
+
+	strbuf_release(&buf_index);
+}
+
+/*
+ * Print porcelain V2 status info for untracked and ignored entries.
+ */
+static void wt_porcelain_v2_print_other(
+	struct string_list_item *it,
+	struct wt_status *s,
+	char prefix)
+{
+	struct strbuf buf = STRBUF_INIT;
+	const char *path;
+	char eol_char;
+
+	if (s->null_termination) {
+		path = it->string;
+		eol_char = '\0';
+	} else {
+		path = quote_path(it->string, s->prefix, &buf);
+		eol_char = '\n';
+	}
+
+	fprintf(s->fp, "%c %s%c", prefix, path, eol_char);
+
+	strbuf_release(&buf);
+}
+
+/*
+ * Print porcelain V2 status.
+ *
+ * [<v2_changed_items>]*
+ * [<v2_unmerged_items>]*
+ * [<v2_untracked_items>]*
+ * [<v2_ignored_items>]*
+ *
+ */
+static void wt_porcelain_v2_print(struct wt_status *s)
+{
+	struct wt_status_change_data *d;
+	struct string_list_item *it;
+	int i;
+
+	for (i = 0; i < s->change.nr; i++) {
+		it = &(s->change.items[i]);
+		d = it->util;
+		if (!d->stagemask)
+			wt_porcelain_v2_print_changed_entry(it, s);
+	}
+
+	for (i = 0; i < s->change.nr; i++) {
+		it = &(s->change.items[i]);
+		d = it->util;
+		if (d->stagemask)
+			wt_porcelain_v2_print_unmerged_entry(it, s);
+	}
+
+	for (i = 0; i < s->untracked.nr; i++) {
+		it = &(s->untracked.items[i]);
+		wt_porcelain_v2_print_other(it, s, '?');
+	}
+
+	for (i = 0; i < s->ignored.nr; i++) {
+		it = &(s->ignored.items[i]);
+		wt_porcelain_v2_print_other(it, s, '!');
+	}
+}
+
 void wt_status_print(struct wt_status *s)
 {
 	switch (s->status_format) {
@@ -1823,7 +2106,7 @@ void wt_status_print(struct wt_status *s)
 		wt_porcelain_print(s);
 		break;
 	case STATUS_FORMAT_PORCELAIN_V2:
-		/* TODO */
+		wt_porcelain_v2_print(s);
 		break;
 	case STATUS_FORMAT_UNSPECIFIED:
 		die("BUG: finalize_deferred_config() should have been called");
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 6/9] status: print branch info with --porcelain=v2 --branch
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (4 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 5/9] status: print per-file porcelain v2 status data Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 7/9] git-status.txt: describe --porcelain=v2 format Jeff Hostetler
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Expand porcelain v2 output to include branch and tracking
branch information. This includes the commit id, the branch,
the upstream branch, and the ahead and behind counts.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 builtin/commit.c |  5 ++++
 wt-status.c      | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 wt-status.h      |  1 +
 3 files changed, 96 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3d222d3..5504afe 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -510,6 +510,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	s->fp = fp;
 	s->nowarn = nowarn;
 	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+	if (!s->is_initial)
+		hashcpy(s->sha1_commit, sha1);
 	s->status_format = status_format;
 	s->ignore_submodule_arg = ignore_submodule_arg;
 
@@ -1378,6 +1380,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	fd = hold_locked_index(&index_lock, 0);
 
 	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+	if (!s.is_initial)
+		hashcpy(s.sha1_commit, sha1);
+
 	s.ignore_submodule_arg = ignore_submodule_arg;
 	s.status_format = status_format;
 	s.verbose = verbose;
diff --git a/wt-status.c b/wt-status.c
index 4b6a090..51405d2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1814,6 +1814,92 @@ static void wt_porcelain_print(struct wt_status *s)
 }
 
 /*
+ * Print branch information for porcelain v2 output.  These lines
+ * are printed when the '--branch' parameter is given.
+ *
+ *    # branch.oid <commit><eol>
+ *    # branch.head <head><eol>
+ *   [# branch.upstream <upstream><eol>
+ *   [# branch.ab +<ahead> -<behind><eol>]]
+ *
+ *      <commit> ::= the current commit hash or the the literal
+ *                   "(initial)" to indicate an initialized repo
+ *                   with no commits.
+ *
+ *        <head> ::= <branch_name> the current branch name or
+ *                   "(detached)" literal when detached head or
+ *                   "(unknown)" when something is wrong.
+ *
+ *    <upstream> ::= the upstream branch name, when set.
+ *
+ *       <ahead> ::= integer ahead value, when upstream set
+ *                   and the commit is present (not gone).
+ *
+ *      <behind> ::= integer behind value, when upstream set
+ *                   and commit is present.
+ *
+ *
+ * The end-of-line is defined by the -z flag.
+ *
+ *                 <eol> ::= NUL when -z,
+ *                           LF when NOT -z.
+ *
+ */
+static void wt_porcelain_v2_print_tracking(struct wt_status *s)
+{
+	struct branch *branch;
+	const char *base;
+	const char *branch_name;
+	struct wt_status_state state;
+	int ab_info, nr_ahead, nr_behind;
+	char eol = s->null_termination ? '\0' : '\n';
+
+	memset(&state, 0, sizeof(state));
+	wt_status_get_state(&state, s->branch && !strcmp(s->branch, "HEAD"));
+
+	fprintf(s->fp, "# branch.oid %s%c",
+			(s->is_initial ? "(initial)" : sha1_to_hex(s->sha1_commit)),
+			eol);
+
+	if (!s->branch)
+		fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol);
+	else {
+		if (!strcmp(s->branch, "HEAD")) {
+			fprintf(s->fp, "# branch.head %s%c", "(detached)", eol);
+
+			if (state.rebase_in_progress || state.rebase_interactive_in_progress)
+				branch_name = state.onto;
+			else if (state.detached_from)
+				branch_name = state.detached_from;
+			else
+				branch_name = "";
+		} else {
+			branch_name = NULL;
+			skip_prefix(s->branch, "refs/heads/", &branch_name);
+
+			fprintf(s->fp, "# branch.head %s%c", branch_name, eol);
+		}
+
+		/* Lookup stats on the upstream tracking branch, if set. */
+		branch = branch_get(branch_name);
+		base = NULL;
+		ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, &base) == 0);
+		if (base) {
+			base = shorten_unambiguous_ref(base, 0);
+			fprintf(s->fp, "# branch.upstream %s%c", base, eol);
+			free((char *)base);
+
+			if (ab_info)
+				fprintf(s->fp, "# branch.ab +%d -%d%c", nr_ahead, nr_behind, eol);
+		}
+	}
+
+	free(state.branch);
+	free(state.onto);
+	free(state.detached_from);
+}
+
+/*
  * Convert various submodule status values into a
  * fixed-length string of characters in the buffer provided.
  */
@@ -2059,6 +2145,7 @@ static void wt_porcelain_v2_print_other(
 /*
  * Print porcelain V2 status.
  *
+ * [<v2_branch>]
  * [<v2_changed_items>]*
  * [<v2_unmerged_items>]*
  * [<v2_untracked_items>]*
@@ -2071,6 +2158,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)
 	struct string_list_item *it;
 	int i;
 
+	if (s->show_branch)
+		wt_porcelain_v2_print_tracking(s);
+
 	for (i = 0; i < s->change.nr; i++) {
 		it = &(s->change.items[i]);
 		d = it->util;
diff --git a/wt-status.h b/wt-status.h
index 43fd3fc..e401837 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -80,6 +80,7 @@ struct wt_status {
 	int hints;
 
 	enum wt_status_format status_format;
+	unsigned char sha1_commit[GIT_SHA1_RAWSZ]; /* when not Initial */
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 7/9] git-status.txt: describe --porcelain=v2 format
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (5 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 6/9] status: print branch info with --porcelain=v2 --branch Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 8/9] test-lib-functions.sh: Add lf_to_nul Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 9/9] status: unit tests for --porcelain=v2 Jeff Hostetler
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Update status manpage to include information about
porcelain v2 format.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 Documentation/git-status.txt | 126 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 122 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6b1454b..a58973b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -183,12 +183,12 @@ in which case `XY` are `!!`.
 
 If -b is used the short-format status is preceded by a line
 
-## branchname tracking info
+    ## branchname tracking info
 
-Porcelain Format
-~~~~~~~~~~~~~~~~
+Porcelain Format Version 1
+~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-The porcelain format is similar to the short format, but is guaranteed
+Version 1 porcelain format is similar to the short format, but is guaranteed
 not to change in a backwards-incompatible way between Git versions or
 based on user configuration. This makes it ideal for parsing by scripts.
 The description of the short format above also describes the porcelain
@@ -210,6 +210,124 @@ field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Porcelain Format Version 2
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Version 2 format adds more detailed information about the state of
+the worktree and changed items.  Version 2 also defines an extensible
+set of easy to parse optional headers.
+
+Header lines start with "#" and are added in response to specific
+command line arguments.  Parsers should ignore headers they
+don't recognize.
+
+### Branch Headers
+
+If `--branch` is given, a series of header lines are printed with
+information about the current branch.
+
+    Line                                     Notes
+    ------------------------------------------------------------
+    # branch.oid <commit> | (initial)        Current commit.
+    # branch.head <branch> | (detached)      Current branch.
+    # branch.upstream <upstream_branch>      If upstream is set.
+    # branch.ab +<ahead> -<behind>           If upstream is set and
+                                             the commit is present.
+    ------------------------------------------------------------
+
+### Changed Tracked Entries
+
+Following the headers, a series of lines are printed for tracked
+entries.  One of three different line formats may be used to describe
+an entry depending on the type of change.  Tracked entries are printed
+in an undefined order; parsers should allow for a mixture of the 3
+line types in any order.
+
+Ordinary changed entries have the following format:
+
+    1 <XY> <sub> <mH> <mI> <mW> <hH> <hI> <path>
+
+Renamed or copied entries have the following format:
+
+    2 <XY> <sub> <mH> <mI> <mW> <hH> <hI> <X><score> <path><sep><origPath>
+
+    Field       Meaning
+    --------------------------------------------------------
+    <XY>        A 2 character field containing the staged and
+                unstaged XY values described in the short format,
+                with unchanged indicated by a "." rather than
+                a space.
+    <sub>       A 4 character field describing the submodule state.
+                "N..." when the entry is not a submodule.
+                "S<c><m><u>" when the entry is a submodule.
+                <c> is "C" if the commit changed; otherwise ".".
+                <m> is "M" if it has tracked changes; otherwise ".".
+                <u> is "U" if there are untracked changes; otherwise ".".
+    <mH>        The octal file mode in HEAD.
+    <mI>        The octal file mode in the index.
+    <mW>        The octal file mode in the worktree.
+    <hH>        The object name in HEAD.
+    <hI>        The object name in the index.
+    <X><score>  The rename or copy score (denoting the percentage
+                of similarity between the source and target of the
+                move or copy). For example "R100" or "C75".
+    <path>      The pathname.  In a renamed/copied entry, this
+                is the path in the index and in the working tree.
+    <sep>       When the `-z` option is used, the 2 pathnames are separated
+                with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)
+                byte separates them.
+    <origPath>  The pathname in the commit at HEAD.  This is only
+                present in a renamed/copied entry, and tells
+                where the renamed/copied contents came from.
+    --------------------------------------------------------
+
+Unmerged entries have the following format; the first character is
+a "u" to distinguish from ordinary changed entries.
+
+    u <xy> <sub> <m1> <m2> <m3> <mW> <h1> <h2> <h3> <path>
+
+    Field       Meaning
+    --------------------------------------------------------
+    <XY>        A 2 character field describing the conflict type
+                as described in the short format.
+    <sub>       A 4 character field describing the submodule state
+                as described above.
+    <m1>        The octal file mode in stage 1.
+    <m2>        The octal file mode in stage 2.
+    <m3>        The octal file mode in stage 3.
+    <mW>        The octal file mode in the worktree.
+    <h1>        The object name in stage 1.
+    <h2>        The object name in stage 2.
+    <h3>        The object name in stage 3.
+    <path>      The pathname.
+    --------------------------------------------------------
+
+### Other Items
+
+Following the tracked entries (and if requested), a series of
+lines will be printed for untracked and then ignored items
+found in the worktree.
+
+Untracked items have the following format:
+
+    ? <path>
+
+Ignored items have the following format:
+
+    ! <path>
+
+### Pathname Format Notes and -z
+
+When the `-z` option is given, pathnames are printed as is and
+without any quoting and lines are terminated with a NUL (ASCII 0x00)
+byte.
+
+Otherwise, all pathnames will be "C-quoted" if they contain any tab,
+linefeed, double quote, or backslash characters. In C-quoting, these
+characters will be replaced with the corresponding C-style escape
+sequences and the resulting pathname will be double quoted.
+
+
 CONFIGURATION
 -------------
 
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 8/9] test-lib-functions.sh: Add lf_to_nul
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (6 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 7/9] git-status.txt: describe --porcelain=v2 format Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-05 22:00 ` [PATCH v5 9/9] status: unit tests for --porcelain=v2 Jeff Hostetler
  8 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Add lf_to_nul() function to test-lib-functions.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 t/test-lib-functions.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4f7eadb..fdaeb3a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -81,6 +81,10 @@ test_decode_color () {
 	'
 }
 
+lf_to_nul () {
+	perl -pe 'y/\012/\000/'
+}
+
 nul_to_q () {
 	perl -pe 'y/\000/Q/'
 }
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v5 9/9] status: unit tests for --porcelain=v2
  2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
                   ` (7 preceding siblings ...)
  2016-08-05 22:00 ` [PATCH v5 8/9] test-lib-functions.sh: Add lf_to_nul Jeff Hostetler
@ 2016-08-05 22:00 ` Jeff Hostetler
  2016-08-08 17:07   ` Junio C Hamano
  8 siblings, 1 reply; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-05 22:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Test porcelain v2 status format.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 t/t7064-wtstatus-pv2.sh | 597 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 597 insertions(+)
 create mode 100755 t/t7064-wtstatus-pv2.sh

diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
new file mode 100755
index 0000000..edc65ce4
--- /dev/null
+++ b/t/t7064-wtstatus-pv2.sh
@@ -0,0 +1,597 @@
+#!/bin/sh
+
+test_description='git status --porcelain=v2
+
+This test exercises porcelain V2 output for git status.'
+
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	test_tick &&
+	git config --local core.autocrlf false &&
+	echo x >file_x &&
+	echo y >file_y &&
+	echo z >file_z &&
+	mkdir dir1 &&
+	echo a >dir1/file_a &&
+	echo b >dir1/file_b
+'
+
+
+##################################################################
+## Confirm output prior to initial commit.
+##################################################################
+
+test_expect_success pre_initial_commit_0 '
+	cat >expect <<-EOF &&
+	# branch.oid (initial)
+	# branch.head master
+	? actual
+	? dir1/
+	? expect
+	? file_x
+	? file_y
+	? file_z
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=normal >actual &&
+	test_cmp expect actual
+'
+
+
+test_expect_success pre_initial_commit_1 '
+	git add file_x file_y file_z dir1 &&
+	OID_A=$(git hash-object -t blob -- dir1/file_a) &&
+	OID_B=$(git hash-object -t blob -- dir1/file_b) &&
+	OID_X=$(git hash-object -t blob -- file_x) &&
+	OID_Y=$(git hash-object -t blob -- file_y) &&
+	OID_Z=$(git hash-object -t blob -- file_z) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid (initial)
+	# branch.head master
+	1 A. N... 000000 100644 100644 $_z40 $OID_A dir1/file_a
+	1 A. N... 000000 100644 100644 $_z40 $OID_B dir1/file_b
+	1 A. N... 000000 100644 100644 $_z40 $OID_X file_x
+	1 A. N... 000000 100644 100644 $_z40 $OID_Y file_y
+	1 A. N... 000000 100644 100644 $_z40 $OID_Z file_z
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+## Try -z on the above
+test_expect_success pre_initial_commit_2 '
+	lf_to_nul >expect <<-EOF &&
+	# branch.oid (initial)
+	# branch.head master
+	1 A. N... 000000 100644 100644 $_z40 $OID_A dir1/file_a
+	1 A. N... 000000 100644 100644 $_z40 $OID_B dir1/file_b
+	1 A. N... 000000 100644 100644 $_z40 $OID_X file_x
+	1 A. N... 000000 100644 100644 $_z40 $OID_Y file_y
+	1 A. N... 000000 100644 100644 $_z40 $OID_Z file_z
+	? actual
+	? expect
+	EOF
+
+	git status -z --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+##################################################################
+## Create first commit. Confirm commit oid in new track header.
+## Then make some changes on top of it.
+##################################################################
+
+test_expect_success initial_commit_0 '
+	git commit -m initial &&
+	H0=$(git rev-parse HEAD) &&
+	cat >expect <<-EOF &&
+	# branch.oid $H0
+	# branch.head master
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+
+test_expect_success initial_commit_1 '
+	echo x >>file_x &&
+	OID_X1=$(git hash-object -t blob -- file_x) &&
+	rm file_z &&
+	H0=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $H0
+	# branch.head master
+	1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
+	1 .D N... 100644 100644 000000 $OID_Z $OID_Z file_z
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+
+test_expect_success initial_commit_2 '
+	git add file_x &&
+	git rm file_z &&
+	H0=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $H0
+	# branch.head master
+	1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+	1 D. N... 100644 000000 000000 $OID_Z $_z40 file_z
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+
+test_expect_success initial_commit_3 '
+	git mv file_y renamed_y &&
+	H0=$(git rev-parse HEAD) &&
+
+	q_to_tab >expect <<-EOF &&
+	# branch.oid $H0
+	# branch.head master
+	1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+	1 D. N... 100644 000000 000000 $OID_Z $_z40 file_z
+	2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+
+##################################################################
+## Create second commit.
+##################################################################
+
+test_expect_success second_commit_0 '
+	git commit -m second &&
+	H1=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $H1
+	# branch.head master
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual
+'
+
+
+##################################################################
+## Ignore a file
+##################################################################
+
+test_expect_success ignore_file_0 '
+	echo x.ign >.gitignore &&
+	echo "ignore me" >x.ign &&
+	H1=$(git rev-parse HEAD) &&
+
+	## ignored file SHOULD NOT appear in output when --ignored is not used.
+	cat >expect <<-EOF &&
+	# branch.oid $H1
+	# branch.head master
+	? .gitignore
+	? actual
+	? expect
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	test_cmp expect actual &&
+
+	## ignored file SHOULD appear in output when --ignored is used.
+	cat >expect <<-EOF &&
+	# branch.oid $H1
+	# branch.head master
+	? .gitignore
+	? actual
+	? expect
+	! x.ign
+	EOF
+
+	git status --porcelain=v2 --branch --ignored --untracked-files=all >actual &&
+	rm x.ign &&
+	rm .gitignore &&
+	test_cmp expect actual
+'
+
+##################################################################
+## Create a permanent .gitignore file so we can stop worrying
+## about test trash in subsequent tests.
+##################################################################
+
+test_expect_success ignore_trash '
+	cat >.gitignore <<-EOF &&
+	actual*
+	expect*
+	EOF
+
+	git add .gitignore &&
+	git commit -m ignore_trash &&
+	H1=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $H1
+	# branch.head master
+	EOF
+
+	git status --porcelain=v2 --branch >actual &&
+	test_cmp expect actual
+'
+
+##################################################################
+## Create some conflicts.
+##################################################################
+
+test_expect_success conflict_AA '
+	git branch AA_A master &&
+	git checkout AA_A &&
+	echo "Branch AA_A" >conflict.txt &&
+	OID_AA_A=$(git hash-object -t blob -- conflict.txt) &&
+	git add conflict.txt &&
+	git commit -m "branch aa_a" &&
+
+	git branch AA_B master &&
+	git checkout AA_B &&
+	echo "Branch AA_B" >conflict.txt &&
+	OID_AA_B=$(git hash-object -t blob -- conflict.txt) &&
+	git add conflict.txt &&
+	git commit -m "branch aa_b" &&
+
+	git branch AA_M AA_B &&
+	git checkout AA_M &&
+	test_must_fail git merge AA_A &&
+
+	HM=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $HM
+	# branch.head AA_M
+	u AA N... 000000 100644 100644 100644 $_z40 $OID_AA_B $OID_AA_A conflict.txt
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	git reset --hard &&
+	test_cmp expect actual
+'
+
+
+test_expect_success conflict_UU '
+	git branch UU_ANC master &&
+	git checkout UU_ANC &&
+	echo "Ancestor" >conflict.txt &&
+	OID_UU_ANC=$(git hash-object -t blob -- conflict.txt) &&
+	git add conflict.txt &&
+	git commit -m "UU_ANC" &&
+
+	git branch UU_A UU_ANC &&
+	git checkout UU_A &&
+	echo "Branch UU_A" >conflict.txt &&
+	OID_UU_A=$(git hash-object -t blob -- conflict.txt) &&
+	git add conflict.txt &&
+	git commit -m "branch uu_a" &&
+
+	git branch UU_B UU_ANC &&
+	git checkout UU_B &&
+	echo "Branch UU_B" >conflict.txt &&
+	OID_UU_B=$(git hash-object -t blob -- conflict.txt) &&
+	git add conflict.txt &&
+	git commit -m "branch uu_b" &&
+
+	git branch UU_M UU_B &&
+	git checkout UU_M &&
+	test_must_fail git merge UU_A &&
+
+	HM=$(git rev-parse HEAD) &&
+
+	cat >expect <<-EOF &&
+	# branch.oid $HM
+	# branch.head UU_M
+	u UU N... 100644 100644 100644 100644 $OID_UU_ANC $OID_UU_B $OID_UU_A conflict.txt
+	EOF
+
+	git status --porcelain=v2 --branch --untracked-files=all >actual &&
+	git reset --hard &&
+	test_cmp expect actual
+'
+
+
+##################################################################
+## Test upstream fields in branch header
+##################################################################
+
+test_expect_success 'upstream_fields_0' '
+	git checkout master &&
+	test_when_finished rm -rf sub_repo &&
+	git clone . sub_repo &&
+	(
+		## Confirm local master tracks remote master.
+		cd sub_repo &&
+		HUF=$(git rev-parse HEAD) &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HUF
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		## Test ahead/behind.
+		echo xyz >file_xyz &&
+		git add file_xyz &&
+		git commit -m xyz &&
+
+		HUF=$(git rev-parse HEAD) &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HUF
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +1 -0
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		## Repeat the above but without --branch.
+		cat >expect <<-EOF &&
+		EOF
+
+		git status --porcelain=v2 --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		## Test upstream-gone case. Fake this by pointing origin/master at
+		## a non-existing commit.
+		OLD=$(git rev-parse origin/master) &&
+		NEW=$_z40 &&
+		mv .git/packed-refs .git/old-packed-refs &&
+		sed "s/$OLD/$NEW/g" <.git/old-packed-refs >.git/packed-refs &&
+
+		HUF=$(git rev-parse HEAD) &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HUF
+		# branch.head master
+		# branch.upstream origin/master
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+
+##################################################################
+## Test submodule status flags.
+##################################################################
+
+test_expect_success 'submodule_flags_0' '
+	git checkout master &&
+	git clone . sub_repo &&
+	git clone . super_repo &&
+	(	cd super_repo &&
+		git submodule add ../sub_repo sub1 &&
+
+		## Confirm stage/add of clean submodule.
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 A. S... 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_1' '
+	(	cd super_repo &&
+		## Make some untracked dirt in the submodule.
+		(	cd sub1 &&
+			echo "dirt" >file_in_sub
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 AM S..U 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_2' '
+	(	cd super_repo &&
+		## Make some staged dirt in the submodule.
+		(	cd sub1 &&
+			git add file_in_sub
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 AM S.M. 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_3' '
+	(	cd super_repo &&
+		## Make some staged and unstaged dirt (on the same file) in the submodule.
+		## This does not cause us to get S.MU (because the submodule does not report
+		## a "?" line for the unstaged changes).
+		(	cd sub1 &&
+			echo "more dirt" >>file_in_sub
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 AM S.M. 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_4' '
+	(	cd super_repo &&
+		## Make some staged and untracked dirt (on different files) in the submodule.
+		(	cd sub1 &&
+			git add file_in_sub &&
+			echo "dirt" >>another_file_in_sub
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 AM S.MU 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_5' '
+	(	cd super_repo &&
+		## Make a new commit in the submodule.
+		(	cd sub1 &&
+			git add file_in_sub &&
+			rm -f another_file_in_sub &&
+			git commit -m "new commit"
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$HSUP &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +0 -0
+		1 A. N... 000000 100644 100644 $_z40 $HMOD .gitmodules
+		1 AM SC.. 000000 160000 160000 $_z40 $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_6' '
+	(	cd super_repo &&
+		## Commit the new submodule commit in the super.
+		git add sub1 &&
+		git commit -m "super commit" &&
+
+		HSUP=$(git rev-parse HEAD) &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +1 -0
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'submodule_flags_7' '
+	(	cd super_repo &&
+		## Make some untracked dirt in the submodule.
+		(	cd sub1 &&
+			echo "yet more dirt" >>file_in_sub
+		) &&
+
+		HMOD=$(git hash-object -t blob -- .gitmodules) &&
+		HSUP=$(git rev-parse HEAD) &&
+		HSUB=$(cd sub1 && git rev-parse HEAD) &&
+
+		cat >expect <<-EOF &&
+		# branch.oid $HSUP
+		# branch.head master
+		# branch.upstream origin/master
+		# branch.ab +1 -0
+		1 .M S.M. 160000 160000 160000 $HSUB $HSUB sub1
+		EOF
+
+		git status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual
+	)
+'
+
+##################################################################
+## The end.
+##################################################################
+
+test_done
-- 
2.8.0.rc4.17.gac42084.dirty


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2
  2016-08-05 22:00 ` [PATCH v5 4/9] status: collect per-file data for --porcelain=v2 Jeff Hostetler
@ 2016-08-05 22:48   ` Junio C Hamano
  2016-08-07  8:34     ` Johannes Schindelin
  0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2016-08-05 22:48 UTC (permalink / raw)
  To: Jeff Hostetler; +Cc: git, Jeff Hostetler

Jeff Hostetler <git@jeffhostetler.com> writes:

>  		if (ce_stage(ce)) {
>  			d->index_status = DIFF_STATUS_UNMERGED;
>  			d->stagemask |= (1 << (ce_stage(ce) - 1));
> +			/*
> +			 * Don't bother setting {mode,oid}_{head,index} since the print
> +			 * code will output the stage values directly and not use the
> +			 * values in these fields.
> +			 */
>  		}
> -		else
> +		else {
>  			d->index_status = DIFF_STATUS_ADDED;
> +			/* Leave {mode,oid}_head zero for adds. */
> +			d->mode_index = ce->ce_mode;
> +			hashcpy(d->oid_index.hash, ce->sha1);
> +		}

Not a big deal (no need to resend for this one alone), but let's
make the above properly formatted, i.e.

		if (ce_stage(ce)) {
                	...
		} else {
                	...
		}                        

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 5/9] status: print per-file porcelain v2 status data
  2016-08-05 22:00 ` [PATCH v5 5/9] status: print per-file porcelain v2 status data Jeff Hostetler
@ 2016-08-05 22:51   ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-08-05 22:51 UTC (permalink / raw)
  To: Jeff Hostetler; +Cc: git, Jeff Hostetler

Jeff Hostetler <git@jeffhostetler.com> writes:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Print per-file information in porcelain v2 format.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---

Relative to the previous version, the renaming of variables
(current, src)->(index, head) made it much easier to understand, at
least to me.  All other changes look sensible.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2
  2016-08-05 22:48   ` Junio C Hamano
@ 2016-08-07  8:34     ` Johannes Schindelin
  2016-08-07 22:29       ` Eric Wong
  0 siblings, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2016-08-07  8:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff Hostetler, git, Jeff Hostetler

Hi Junio,

On Fri, 5 Aug 2016, Junio C Hamano wrote:

> Jeff Hostetler <git@jeffhostetler.com> writes:
> 
> >  		if (ce_stage(ce)) {
> >  			d->index_status = DIFF_STATUS_UNMERGED;
> >  			d->stagemask |= (1 << (ce_stage(ce) - 1));
> > +			/*
> > +			 * Don't bother setting {mode,oid}_{head,index} since the print
> > +			 * code will output the stage values directly and not use the
> > +			 * values in these fields.
> > +			 */
> >  		}
> > -		else
> > +		else {
> >  			d->index_status = DIFF_STATUS_ADDED;
> > +			/* Leave {mode,oid}_head zero for adds. */
> > +			d->mode_index = ce->ce_mode;
> > +			hashcpy(d->oid_index.hash, ce->sha1);
> > +		}
> 
> Not a big deal (no need to resend for this one alone), but let's
> make the above properly formatted, i.e.
> 
> 		if (ce_stage(ce)) {
>                 	...
> 		} else {
>                 	...
> 		}                        

Do I understand correctly that your objections is against having the curly
brace before the "else" on its own line?

If so, when did our coding style change? I vividly remember that we
strongly favored putting the "else" on a new line after a closing brace,
to make diffs nicer in case the braces were removed or added.

BTW your suggestion has 24 extra spaces after the final closing brace ;-)

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2
  2016-08-07  8:34     ` Johannes Schindelin
@ 2016-08-07 22:29       ` Eric Wong
  2016-08-08  1:57         ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Wong @ 2016-08-07 22:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Jeff Hostetler, git, Jeff Hostetler

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 5 Aug 2016, Junio C Hamano wrote:
> > Jeff Hostetler <git@jeffhostetler.com> writes:
> > >  		}
> > > -		else
> > > +		else {
> > >  			d->index_status = DIFF_STATUS_ADDED;
> > > +			/* Leave {mode,oid}_head zero for adds. */
> > > +			d->mode_index = ce->ce_mode;
> > > +			hashcpy(d->oid_index.hash, ce->sha1);
> > > +		}
> > 
> > Not a big deal (no need to resend for this one alone), but let's
> > make the above properly formatted, i.e.
> > 
> > 		if (ce_stage(ce)) {
> >                 	...
> > 		} else {
> >                 	...
> > 		}                        
> 
> Do I understand correctly that your objections is against having the curly
> brace before the "else" on its own line?
> 
> If so, when did our coding style change? I vividly remember that we
> strongly favored putting the "else" on a new line after a closing brace,
> to make diffs nicer in case the braces were removed or added.

AFAIK, Linux kernel CodingStyle has always been what Junio
suggested (just w/o the trailing spaces :),
and we inherit from that.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/CodingStyle

> BTW your suggestion has 24 extra spaces after the final closing brace ;-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2
  2016-08-07 22:29       ` Eric Wong
@ 2016-08-08  1:57         ` Junio C Hamano
  0 siblings, 0 replies; 19+ messages in thread
From: Junio C Hamano @ 2016-08-08  1:57 UTC (permalink / raw)
  To: Eric Wong; +Cc: Johannes Schindelin, Jeff Hostetler, git, Jeff Hostetler

Eric Wong <e@80x24.org> writes:

>> > Not a big deal (no need to resend for this one alone), but let's
>> > make the above properly formatted, i.e.
>> > 
>> > 		if (ce_stage(ce)) {
>> >                 	...
>> > 		} else {
>> >                 	...
>> > 		}                        
>> 
>> Do I understand correctly that your objections is against having the curly
>> brace before the "else" on its own line?
>> 
>> If so, when did our coding style change? I vividly remember that we
>> strongly favored putting the "else" on a new line after a closing brace,
>> to make diffs nicer in case the braces were removed or added.
>
> AFAIK, Linux kernel CodingStyle has always been what Junio
> suggested (just w/o the trailing spaces :),
> and we inherit from that.

What Eric said.

While I admit that I sometimes break line between "}" and "else {"
by inertia when I am being careless and get caught by checkpatch.pl
myself, I do not recall trying to justify it; you probably may
remember somebody else saying that, but I don't recall anybody
making that argument on the list, and more importantly I don't
recall us making that our style based on that argument.

The only two and half kinds of warnings we knowingly ignore from
scripts/checkpatch.pl in the Linux kernel source tree are:

 * "Avoid typedefs."  We do avoid making graduitous use of typedef to
   hide a structure behind a type and pretty much limit ourselves to
   use it for (callback) function types, though.

 * "We've never heard of Helped-by/Mentored-by footers"; well,
   kernel folks may not, but we have ;-)

 * "No spaces for bitfield width".  This may not be justifyable, but
   the majority of our bitfield widths are defined in the way not
   blessed by checkpatch.pl checker.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 9/9] status: unit tests for --porcelain=v2
  2016-08-05 22:00 ` [PATCH v5 9/9] status: unit tests for --porcelain=v2 Jeff Hostetler
@ 2016-08-08 17:07   ` Junio C Hamano
  2016-08-10 21:28     ` Jeff Hostetler
  0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2016-08-08 17:07 UTC (permalink / raw)
  To: Jeff Hostetler; +Cc: git, Jeff Hostetler

Jeff Hostetler <git@jeffhostetler.com> writes:

> +test_expect_success pre_initial_commit_0 '
> +	...
> +	git status --porcelain=v2 --branch --untracked-files=normal >actual &&
> +	test_cmp expect actual
> +'
> +
> +
> +test_expect_success pre_initial_commit_1 '
> +	git add file_x file_y file_z dir1 &&
> +	...
> +	cat >expect <<-EOF &&
> +	# branch.oid (initial)
> +	# branch.head master
> +	1 A. N... 000000 100644 100644 $_z40 $OID_A dir1/file_a
> +	...

This makes one wonder what would/should be shown on the "A." column
if one of these files are added with "-N" (intent-to-add).  I do not
see any test for such entries in this patch, but I think it should.

> +	git status --porcelain=v2 --branch --untracked-files=all >actual &&
> +	test_cmp expect actual
> +'

> +## Try -z on the above
> +test_expect_success pre_initial_commit_2 '
> +	lf_to_nul >expect <<-EOF &&
> +	...
> +	git status -z --porcelain=v2 --branch --untracked-files=all >actual &&
> +	test_cmp expect actual
> +'

"Try -z on the above" can and should be in the test title to help
those who are watching the test run.  Instead of trying to clarify
code with comments, clarify them by letting the code speak for
themselves.  I would have named the above three perhaps like this:

	test_expect_success 'before first commit, nothing added'
        test_expect_success 'before first commit, some files added'
        test_expect_success 'before first commit, some files added (-z)'

"pre-initial-commit-$number" is not very interesting; it does not
convey a more interesting aspect of these tests, like (a) _0 is
distinct (nothing added) among the three, (b) _1 and _2 are about
the same state, just expressed differently, and (c) _1 is LF
terminated, _2 is NUL terminated.  The same comment applies to the
remainder of the test.  For example:

> +##################################################################
> +## Create second commit.
> +##################################################################
> +
> +test_expect_success second_commit_0 '

This "_0" does not tell us anything, but you are testing "When fully
committed, we only see untracked files (if we ask) and branch info
(if we ask).

Having said all that, it is OK to fix their titles after the current
9-patch series lands on 'next'; incremental refinements are easier
on reviewers than having to review too many rerolls.

This is probably a good place to see what happens to these untracked
files and branch info if we do not ask the command to show them.
Otherwise, it tests exactly the same as "initial_commit_0" and is
not all that interesting, no?

> +##################################################################
> +## Ignore a file
> +##################################################################
> +
> +test_expect_success ignore_file_0 '
> +	echo x.ign >.gitignore &&
> +	echo "ignore me" >x.ign &&
> +	H1=$(git rev-parse HEAD) &&
> +
> +	## ignored file SHOULD NOT appear in output when --ignored is not used.
> + ...
> +	git status --porcelain=v2 --branch --untracked-files=all >actual &&
> +	test_cmp expect actual &&
> + ...
> +	git status --porcelain=v2 --branch --ignored --untracked-files=all >actual &&
> +	rm x.ign &&
> +	rm .gitignore &&

Arrange these files to be cleaned before you create them by having

	test_when_finished "rm -f x.ign .gitignore" &&

at the very beginning of this test before they are created.
Otherwise, if any step before these removal fail, later test that
assume they are gone will be affected.  You already do so correctly
in the upstream_fields_0 test below.

> +##################################################################
> +## Create some conflicts.
> +##################################################################
> +
> +test_expect_success conflict_AA '
> +	git branch AA_A master &&
> +	git checkout AA_A &&
> +	echo "Branch AA_A" >conflict.txt &&
> +	OID_AA_A=$(git hash-object -t blob -- conflict.txt) &&
> +	git add conflict.txt &&
> +	git commit -m "branch aa_a" &&
> +
> +	git branch AA_B master &&
> +	git checkout AA_B &&
> +	echo "Branch AA_B" >conflict.txt &&
> +	OID_AA_B=$(git hash-object -t blob -- conflict.txt) &&
> +	git add conflict.txt &&
> +	git commit -m "branch aa_b" &&
> +
> +	git branch AA_M AA_B &&
> +	git checkout AA_M &&
> +	test_must_fail git merge AA_A &&
> +
> +	HM=$(git rev-parse HEAD) &&
> +
> +	cat >expect <<-EOF &&
> +	# branch.oid $HM
> +	# branch.head AA_M
> +	u AA N... 000000 100644 100644 100644 $_z40 $OID_AA_B $OID_AA_A conflict.txt
> +	EOF

This is a small point, but doesn't the lowercase 'u' somehow look
ugly, especially because the status letters that immediately follow
it are all uppercase?

> +	git status --porcelain=v2 --branch --untracked-files=all >actual &&
> +	git reset --hard &&

This "reset" also may be a candidate for test_when_finished clean-up
(I won't repeat the comment but there probably are many others).

> +test_expect_success 'upstream_fields_0' '
> +	git checkout master &&
> +	test_when_finished rm -rf sub_repo &&

The "when-finished" argument are usually quoted like this, I think:

	test_when_finished "rm -rf sub_repo" &&


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 9/9] status: unit tests for --porcelain=v2
  2016-08-08 17:07   ` Junio C Hamano
@ 2016-08-10 21:28     ` Jeff Hostetler
  2016-08-10 22:41       ` Junio C Hamano
  0 siblings, 1 reply; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-10 21:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff Hostetler



On 08/08/2016 01:07 PM, Junio C Hamano wrote:
> Jeff Hostetler <git@jeffhostetler.com> writes:
>
>> +test_expect_success pre_initial_commit_0 '
>> +	...
>> +	git status --porcelain=v2 --branch --untracked-files=normal >actual &&
>> +	test_cmp expect actual
>> +'
>> +
>> +
>> +test_expect_success pre_initial_commit_1 '
>> +	git add file_x file_y file_z dir1 &&
>> +	...
>> +	cat >expect <<-EOF &&
>> +	# branch.oid (initial)
>> +	# branch.head master
>> +	1 A. N... 000000 100644 100644 $_z40 $OID_A dir1/file_a
>> +	...
>
> This makes one wonder what would/should be shown on the "A." column
> if one of these files are added with "-N" (intent-to-add).  I do not
> see any test for such entries in this patch, but I think it should.

I must admit that I don't use -N, so I'm open to recommendations here.
In my brief testing, the existing porcelain status reports it as "AM"
(for both a file with content and an empty file).

The V2 code outputs the following:
1 AM N... 000000 100644 100644 0000000000000000000000000000000000000000 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 intent.add

Which I think makes sense.  I'll add a test to exercise that.


> "Try -z on the above" can and should be in the test title to help
> ...
> Having said all that, it is OK to fix their titles after the current
> 9-patch series lands on 'next'; incremental refinements are easier
> on reviewers than having to review too many rerolls.

I'll change the test titles to have all that info.


> This is probably a good place to see what happens to these untracked
> files and branch info if we do not ask the command to show them.

I'll add some cases to cycle thru the options and confirm
there's no output when not requested.


>> +##################################################################
>> +## Ignore a file
>> +##################################################################
>> +
>> +test_expect_success ignore_file_0 '
>> +	echo x.ign >.gitignore &&
>> +	echo "ignore me" >x.ign &&
>> +	H1=$(git rev-parse HEAD) &&
>> +
>> +	## ignored file SHOULD NOT appear in output when --ignored is not used.
>> + ...
>> +	git status --porcelain=v2 --branch --untracked-files=all >actual &&
>> +	test_cmp expect actual &&
>> + ...
>> +	git status --porcelain=v2 --branch --ignored --untracked-files=all >actual &&
>> +	rm x.ign &&
>> +	rm .gitignore &&
>
> Arrange these files to be cleaned before you create them by having
>
> 	test_when_finished "rm -f x.ign .gitignore" &&
>
> at the very beginning of this test before they are created.
> Otherwise, if any step before these removal fail, later test that
> assume they are gone will be affected.  You already do so correctly
> in the upstream_fields_0 test below.

Missed a few.  Got it.  Thanks!


>> +
>> +	cat >expect <<-EOF &&
>> +	# branch.oid $HM
>> +	# branch.head AA_M
>> +	u AA N... 000000 100644 100644 100644 $_z40 $OID_AA_B $OID_AA_A conflict.txt
>> +	EOF
>
> This is a small point, but doesn't the lowercase 'u' somehow look
> ugly, especially because the status letters that immediately follow
> it are all uppercase?
>

Since we are inventing a new format and my column 1 is completely new
I didn't think it mattered.  And I used a lowercase 'u' to distinguish 
it from the "U" in the X and Y columns since they mean different things.

But we can change it if you'd prefer.


>> +	git status --porcelain=v2 --branch --untracked-files=all >actual &&
>> +	git reset --hard &&
>
> This "reset" also may be a candidate for test_when_finished clean-up
> (I won't repeat the comment but there probably are many others).

Got it. And I hopefully caught the rest.


>> +test_expect_success 'upstream_fields_0' '
>> +	git checkout master &&
>> +	test_when_finished rm -rf sub_repo &&
>
> The "when-finished" argument are usually quoted like this, I think:
>
> 	test_when_finished "rm -rf sub_repo" &&

Got it. Thanks.


I have the test changes ready.  As suggested, I'll send a single commit
patch after my 9-patch series lands on 'next'.

Thanks,
Jeff





^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 9/9] status: unit tests for --porcelain=v2
  2016-08-10 21:28     ` Jeff Hostetler
@ 2016-08-10 22:41       ` Junio C Hamano
  2016-08-10 23:23         ` Jeff Hostetler
  0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2016-08-10 22:41 UTC (permalink / raw)
  To: Jeff Hostetler; +Cc: git, Jeff Hostetler

Jeff Hostetler <git@jeffhostetler.com> writes:

> In my brief testing, the existing porcelain status reports it as "AM"
> (for both a file with content and an empty file).
>
> The V2 code outputs the following:
> 1 AM N... 000000 100644 100644

OK, so they are consistent, which is good.

>> Having said all that, it is OK to fix their titles after the current
>> 9-patch series lands on 'next'; incremental refinements are easier
>> on reviewers than having to review too many rerolls.
>
> I'll change the test titles to have all that info.

OK.  As I said, if the retitling is the only change, then doing so
as a follow-up to the existing 9-patch series would be easier to
review.  If there are other changes needed, a reroll of the full set
is probably better.

>> This is a small point, but doesn't the lowercase 'u' somehow look
>> ugly, especially because the status letters that immediately follow
>> it are all uppercase?
>>
>
> Since we are inventing a new format and my column 1 is completely new
> I didn't think it mattered.

Wrong assumption ;-)  We want the resulting code to be consistent.

> And I used a lowercase 'u' to distinguish
> it from the "U" in the X and Y columns since they mean different
> things.

I thought they both mean "Unmerged"; that is why I thought seeing a
lowercase there was very strange.

Thanks.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v5 9/9] status: unit tests for --porcelain=v2
  2016-08-10 22:41       ` Junio C Hamano
@ 2016-08-10 23:23         ` Jeff Hostetler
  0 siblings, 0 replies; 19+ messages in thread
From: Jeff Hostetler @ 2016-08-10 23:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff Hostetler



On 08/10/2016 06:41 PM, Junio C Hamano wrote:
> Jeff Hostetler <git@jeffhostetler.com> writes:
>>> Having said all that, it is OK to fix their titles after the current
>>> 9-patch series lands on 'next'; incremental refinements are easier
>>> on reviewers than having to review too many rerolls.
>>
>> I'll change the test titles to have all that info.
>
> OK.  As I said, if the retitling is the only change, then doing so
> as a follow-up to the existing 9-patch series would be easier to
> review.  If there are other changes needed, a reroll of the full set
> is probably better.

I've changed the titles and added 5 or 6 more tests to
cover your previous questions/comments, but no code changes.


>>> This is a small point, but doesn't the lowercase 'u' somehow look
>>> ugly, especially because the status letters that immediately follow
>>> it are all uppercase?
>>>
>>
>> Since we are inventing a new format and my column 1 is completely new
>> I didn't think it mattered.
>
> Wrong assumption ;-)  We want the resulting code to be consistent.
>
>> And I used a lowercase 'u' to distinguish
>> it from the "U" in the X and Y columns since they mean different
>> things.
>
> I thought they both mean "Unmerged"; that is why I thought seeing a
> lowercase there was very strange.

For unmerged items, the X & Y columns in the original porcelain
format, can have various combinations of "A", "D", and "U" values.
The implication being that "U" means modified/edited in some way.
Yes, it is labeled "unmerged", but in a "DU" or "UD" conflict, it
means that one side deleted it and one side modified it in some way.
A "UU" conflict means both sides edited it and auto-merge failed.

All I wanted the lowercase 'u' to indicate was that the file was
in an unmerged state (without specifying why).  That is, we would
have (the 7 valid combinations of) "u [ADU][ADU]" and it would be
clear(er?) that the little 'u' was different from the other 2.

I know it is a subtle point and we can change it if you want, but
that was the rationale.


Thanks,
Jeff


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-08-10 23:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-05 22:00 [PATCH v5 0/9] status: V2 porcelain status Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 1/9] status: rename long-format print routines Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 2/9] status: cleanup API to wt_status_print Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 3/9] status: support --porcelain[=<version>] Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 4/9] status: collect per-file data for --porcelain=v2 Jeff Hostetler
2016-08-05 22:48   ` Junio C Hamano
2016-08-07  8:34     ` Johannes Schindelin
2016-08-07 22:29       ` Eric Wong
2016-08-08  1:57         ` Junio C Hamano
2016-08-05 22:00 ` [PATCH v5 5/9] status: print per-file porcelain v2 status data Jeff Hostetler
2016-08-05 22:51   ` Junio C Hamano
2016-08-05 22:00 ` [PATCH v5 6/9] status: print branch info with --porcelain=v2 --branch Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 7/9] git-status.txt: describe --porcelain=v2 format Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 8/9] test-lib-functions.sh: Add lf_to_nul Jeff Hostetler
2016-08-05 22:00 ` [PATCH v5 9/9] status: unit tests for --porcelain=v2 Jeff Hostetler
2016-08-08 17:07   ` Junio C Hamano
2016-08-10 21:28     ` Jeff Hostetler
2016-08-10 22:41       ` Junio C Hamano
2016-08-10 23:23         ` Jeff Hostetler

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).