threads / patch / 30442

patchcorrect git-status Porcelain Format documentation

Subject: [PATCH] correct git-status Porcelain Format documentation

## tl;dr

16 messages between May 6, 2012 and May 8, 2012. Diffs are folded; open one to read it.

replies: 15people: 3as markdown or json

Jeff King· May 6, 2012, 13:29 UTC · lore
From: Zak Johnson <zakj@nox.cx>

The existing documentation implies that "git status --porcelain" has a branch line while "git status --porcelain -z" does not; in fact, neither includes a branch line.

Signed-off-by: Zak Johnson <zakj@nox.cx>
---
This was forward to me by the original author, who had trouble posting
to the list. So I'm trying it (I didn't see anything from the taboo list
in the patch, but we'll see...).

The patch itself looks obviously correct and describes the current behavior. But I have to wonder: the --short format will also not produce the branch line unless you provide "-b". So why is it that the porcelain format does not respect "-b", since anybody who asked for it would obviously be expecting to find and parse it?

Should this bit of documentation be dropped in favor of just making "-b" work properly?

 Documentation/git-status.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Show changes to Documentation/git-status.txt +3 −1
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 2883a28..3e12020 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -176,6 +176,8 @@ format, with a few exceptions:
 2. The user's status.relativePaths configuration is not respected; paths
    shown will always be relative to the repository root.
 
+3. There is no branch line.
+
 There is also an alternate -z format recommended for machine parsing. In
 that format, the status field is the same, but some other things
 change.  First, the '\->' is omitted from rename entries and the field
@@ -184,7 +186,7 @@ order is reversed (e.g 'from \-> to' becomes 'to from'). Second, a NUL
 and the terminating newline (but a space still separates the status
 field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
-backslash-escaping is performed. Fourth, there is no branch line.
+backslash-escaping is performed.
 
 CONFIGURATION
 -------------
-- 
1.7.10.1.14.gb97aca1
Jeff King· May 6, 2012, 13:51 UTC · re: Jeff King · lore

Re: [PATCH] correct git-status Porcelain Format documentation

[+cc Zak, who I forgot in the first email]
On Sun, May 06, 2012 at 09:29:59AM -0400, Jeff King wrote:
Show 20 quoted lines
> From: Zak Johnson <zakj@nox.cx>
> 
> The existing documentation implies that "git status --porcelain" has a branch
> line while "git status --porcelain -z" does not; in fact, neither includes a
> branch line.
> 
> Signed-off-by: Zak Johnson <zakj@nox.cx>
> ---
> This was forward to me by the original author, who had trouble posting
> to the list. So I'm trying it (I didn't see anything from the taboo list
> in the patch, but we'll see...).
> 
> The patch itself looks obviously correct and describes the current
> behavior. But I have to wonder: the --short format will also not produce
> the branch line unless you provide "-b". So why is it that the porcelain
> format does not respect "-b", since anybody who asked for it would
> obviously be expecting to find and parse it?
> 
> Should this bit of documentation be dropped in favor of just making "-b"
> work properly?

So that patch could look like the one below. But that brings up new confusion.

What should "git status --porcelain -z -b" look like? With my patch, it prints the branch-line with only a newline, not respecting the NUL termination. Which sounds like a bug, except that's what happens _now_ with "git status --short -z -b". Which seems like a bug to me, but maybe somebody is relying on that. It seems kind of broken to me.

Also, while looking at the documentation, we say of "-z": "This implies the --porcelain output format if no other format is given". But the only other format you could give is "--short", since there is no way to ask for the long output (and nor do we handle NUL-termination in that code, anyway). Should this be simplified to just "this implies porcelain"? That would technically break somebody who wanted their status.relativePaths config option respected. But it kind of seems crazy to me.

-- >8 --
Subject: [PATCH] status: respect "-b" for porcelain format

There is no reason not to, as the user has to explicitly ask for it, so we are not breaking compatibility by doing so.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-status.txt | 2 +-
 builtin/commit.c             | 4 ++--
 t/t7508-status.sh            | 8 ++++++--
 wt-status.c                  | 5 +++--
 wt-status.h                  | 2 +-
 5 files changed, 13 insertions(+), 8 deletions(-)
Show changes to 5 files +13 −8

Documentation/git-status.txt, builtin/commit.c, t/t7508-status.sh, wt-status.c, wt-status.h

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 2883a28..67e5f53 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -184,7 +184,7 @@ order is reversed (e.g 'from \-> to' becomes 'to from'). Second, a NUL
 and the terminating newline (but a space still separates the status
 field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
-backslash-escaping is performed. Fourth, there is no branch line.
+backslash-escaping is performed.
 
 CONFIGURATION
 -------------
diff --git a/builtin/commit.c b/builtin/commit.c
index a876a73..0e898e6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -504,7 +504,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 		wt_shortstatus_print(s, null_termination, status_show_branch);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(s, null_termination);
+		wt_porcelain_print(s, null_termination, status_show_branch);
 		break;
 	case STATUS_FORMAT_LONG:
 		wt_status_print(s);
@@ -1287,7 +1287,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		wt_shortstatus_print(&s, null_termination, status_show_branch);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(&s, null_termination);
+		wt_porcelain_print(&s, null_termination, status_show_branch);
 		break;
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 8f5cfac..695b6a8 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -671,11 +671,15 @@ test_expect_success 'status --porcelain ignores color.status' '
 git config --unset color.status
 git config --unset color.ui
 
-test_expect_success 'status --porcelain ignores -b' '
+test_expect_success 'status --porcelain respects -b' '
 
 	git status --porcelain -b >output &&
+	{
+		echo "## master" &&
+		cat expect
+	} >tmp &&
+	mv tmp expect &&
 	test_cmp expect output
-
 '
 
 cat >expect <<\EOF
diff --git a/wt-status.c b/wt-status.c
index eeef17e..7d2dbcd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -973,10 +973,11 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination, int show_br
 	}
 }
 
-void wt_porcelain_print(struct wt_status *s, int null_termination)
+void wt_porcelain_print(struct wt_status *s, int null_termination,
+			int show_branch)
 {
 	s->use_color = 0;
 	s->relative_paths = 0;
 	s->prefix = NULL;
-	wt_shortstatus_print(s, null_termination, 0);
+	wt_shortstatus_print(s, null_termination, show_branch);
 }
diff --git a/wt-status.h b/wt-status.h
index 6dd7207..b8e8758 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -74,7 +74,7 @@ void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 
 void wt_shortstatus_print(struct wt_status *s, int null_termination, int show_branch);
-void wt_porcelain_print(struct wt_status *s, int null_termination);
+void wt_porcelain_print(struct wt_status *s, int null_termination, int show_branch);
 
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...)
 	;
-- 
1.7.10.1.14.gb97aca1
Junio C Hamano· May 7, 2012, 18:10 UTC · re: Jeff King · lore

Re: [PATCH] correct git-status Porcelain Format documentation

Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> What should "git status --porcelain -z -b" look like? With my patch, it
> prints the branch-line with only a newline, not respecting the NUL
> termination. Which sounds like a bug, except that's what happens _now_
> with "git status --short -z -b".  Which seems like a bug to me, but
> maybe somebody is relying on that. It seems kind of broken to me.

It is broken; and it is not a problem if somebody relied on a broken output without giving --porcelain in a script and such a script needs to be updated. If we are updating the code to give "## <branch>" in the output under the "--porcelain", let's do that right from the beginning.

Show 5 quoted lines
> Also, while looking at the documentation, we say of "-z": "This implies
> the --porcelain output format if no other format is given". But the only
> other format you could give is "--short", since there is no way to ask
> for the long output (and nor do we handle NUL-termination in that code,
> anyway).
... nor do we define long porcelain format to begin with.
Show 5 quoted lines
> Should this be simplified to just "this implies porcelain"?
>
> That would technically break somebody who wanted their
> status.relativePaths config option respected. But it kind of seems crazy
> to me.

I do not think it is worth changing it. It is not too much touble to spell "status --porcelain -z <whatever else>" in a Porcelain script only once and forget about it.

Jeff King· May 7, 2012, 18:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] correct git-status Porcelain Format documentation

On Mon, May 07, 2012 at 11:10:23AM -0700, Junio C Hamano wrote:
Show 12 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > What should "git status --porcelain -z -b" look like? With my patch, it
> > prints the branch-line with only a newline, not respecting the NUL
> > termination. Which sounds like a bug, except that's what happens _now_
> > with "git status --short -z -b".  Which seems like a bug to me, but
> > maybe somebody is relying on that. It seems kind of broken to me.
> 
> It is broken; and it is not a problem if somebody relied on a broken
> output without giving --porcelain in a script and such a script needs to
> be updated.  If we are updating the code to give "## <branch>" in the
> output under the "--porcelain", let's do that right from the beginning.
OK, that matches my feelings.
Show 9 quoted lines
> > Should this be simplified to just "this implies porcelain"?
> >
> > That would technically break somebody who wanted their
> > status.relativePaths config option respected. But it kind of seems crazy
> > to me.
> 
> I do not think it is worth changing it.  It is not too much touble to
> spell "status --porcelain -z <whatever else>" in a Porcelain script only
> once and forget about it.

OK. I think it makes it conceptually simpler, but of the problems I listed, that is the least bug-like.

I'll prepare a series to fix "-b -z" and "-b --porcelain".
-Peff
Jeff King· May 7, 2012, 21:21 UTC · re: Jeff King · lore

Re: [PATCH] correct git-status Porcelain Format documentation

On Mon, May 07, 2012 at 02:13:48PM -0400, Jeff King wrote:
> I'll prepare a series to fix "-b -z" and "-b --porcelain".
Here's that series:
  [1/5]: commit: refactor option parsing
  [2/5]: status: refactor colopts handling
  [3/5]: status: refactor null_termination option
  [4/5]: status: fix null termination with "-b"
  [5/5]: status: respect "-b" for porcelain format

Patches 1 and 3 are cleanups to aid 4 and 5, which are the real fixes. Patch 2 is an optional cleanup I noticed while in the area.

-Peff
Jeff King· May 7, 2012, 21:22 UTC · re: Jeff King · lore

[PATCH 1/5] commit: refactor option parsing

The options are declared as a static global, but really they need only be accessible from cmd_commit. Putting them there will allow us to directly reference local variables in the options list, which will facilitate further cleanups.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 113 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 57 insertions(+), 56 deletions(-)
Show changes to builtin/commit.c +57 −56
diff --git a/builtin/commit.c b/builtin/commit.c
index a876a73..5f48e8a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -131,59 +131,6 @@ static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-static struct option builtin_commit_options[] = {
-	OPT__QUIET(&quiet, "suppress summary after successful commit"),
-	OPT__VERBOSE(&verbose, "show diff in commit message template"),
-
-	OPT_GROUP("Commit message options"),
-	OPT_FILENAME('F', "file", &logfile, "read message from file"),
-	OPT_STRING(0, "author", &force_author, "author", "override author for commit"),
-	OPT_STRING(0, "date", &force_date, "date", "override date for commit"),
-	OPT_CALLBACK('m', "message", &message, "message", "commit message", opt_parse_m),
-	OPT_STRING('c', "reedit-message", &edit_message, "commit", "reuse and edit message from specified commit"),
-	OPT_STRING('C', "reuse-message", &use_message, "commit", "reuse message from specified commit"),
-	OPT_STRING(0, "fixup", &fixup_message, "commit", "use autosquash formatted message to fixup specified commit"),
-	OPT_STRING(0, "squash", &squash_message, "commit", "use autosquash formatted message to squash specified commit"),
-	OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C/-c/--amend)"),
-	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
-	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
-	OPT_BOOL('e', "edit", &edit_flag, "force edit of commit"),
-	OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
-	OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
-	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
-	  "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
-	/* end commit message options */
-
-	OPT_GROUP("Commit contents options"),
-	OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
-	OPT_BOOLEAN('i', "include", &also, "add specified files to index for commit"),
-	OPT_BOOLEAN(0, "interactive", &interactive, "interactively add files"),
-	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactively add changes"),
-	OPT_BOOLEAN('o', "only", &only, "commit only specified files"),
-	OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-commit hook"),
-	OPT_BOOLEAN(0, "dry-run", &dry_run, "show what would be committed"),
-	OPT_SET_INT(0, "short", &status_format, "show status concisely",
-		    STATUS_FORMAT_SHORT),
-	OPT_BOOLEAN(0, "branch", &status_show_branch, "show branch information"),
-	OPT_SET_INT(0, "porcelain", &status_format,
-		    "machine-readable output", STATUS_FORMAT_PORCELAIN),
-	OPT_BOOLEAN('z', "null", &null_termination,
-		    "terminate entries with NUL"),
-	OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
-	OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"),
-	{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
-	/* end commit contents options */
-
-	{ OPTION_BOOLEAN, 0, "allow-empty", &allow_empty, NULL,
-	  "ok to record an empty change",
-	  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
-	{ OPTION_BOOLEAN, 0, "allow-empty-message", &allow_empty_message, NULL,
-	  "ok to record a change with an empty message",
-	  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
-
-	OPT_END()
-};
-
 static void determine_whence(struct wt_status *s)
 {
 	if (file_exists(git_path("MERGE_HEAD")))
@@ -1037,6 +984,7 @@ static const char *read_commit_message(const char *name)
 }
 
 static int parse_and_validate_options(int argc, const char *argv[],
+				      const struct option *options,
 				      const char * const usage[],
 				      const char *prefix,
 				      struct commit *current_head,
@@ -1044,8 +992,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 {
 	int f = 0;
 
-	argc = parse_options(argc, argv, prefix, builtin_commit_options, usage,
-			     0);
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
 	if (force_author && !strchr(force_author, '>'))
 		force_author = find_author_by_nickname(force_author);
@@ -1422,6 +1369,59 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
+	static struct option builtin_commit_options[] = {
+		OPT__QUIET(&quiet, "suppress summary after successful commit"),
+		OPT__VERBOSE(&verbose, "show diff in commit message template"),
+
+		OPT_GROUP("Commit message options"),
+		OPT_FILENAME('F', "file", &logfile, "read message from file"),
+		OPT_STRING(0, "author", &force_author, "author", "override author for commit"),
+		OPT_STRING(0, "date", &force_date, "date", "override date for commit"),
+		OPT_CALLBACK('m', "message", &message, "message", "commit message", opt_parse_m),
+		OPT_STRING('c', "reedit-message", &edit_message, "commit", "reuse and edit message from specified commit"),
+		OPT_STRING('C', "reuse-message", &use_message, "commit", "reuse message from specified commit"),
+		OPT_STRING(0, "fixup", &fixup_message, "commit", "use autosquash formatted message to fixup specified commit"),
+		OPT_STRING(0, "squash", &squash_message, "commit", "use autosquash formatted message to squash specified commit"),
+		OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C/-c/--amend)"),
+		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
+		OPT_FILENAME('t', "template", &template_file, "use specified template file"),
+		OPT_BOOL('e', "edit", &edit_flag, "force edit of commit"),
+		OPT_STRING(0, "cleanup", &cleanup_arg, "default", "how to strip spaces and #comments from message"),
+		OPT_BOOLEAN(0, "status", &include_status, "include status in commit message template"),
+		{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, "key id",
+		  "GPG sign commit", PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+		/* end commit message options */
+
+		OPT_GROUP("Commit contents options"),
+		OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
+		OPT_BOOLEAN('i', "include", &also, "add specified files to index for commit"),
+		OPT_BOOLEAN(0, "interactive", &interactive, "interactively add files"),
+		OPT_BOOLEAN('p', "patch", &patch_interactive, "interactively add changes"),
+		OPT_BOOLEAN('o', "only", &only, "commit only specified files"),
+		OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-commit hook"),
+		OPT_BOOLEAN(0, "dry-run", &dry_run, "show what would be committed"),
+		OPT_SET_INT(0, "short", &status_format, "show status concisely",
+			    STATUS_FORMAT_SHORT),
+		OPT_BOOLEAN(0, "branch", &status_show_branch, "show branch information"),
+		OPT_SET_INT(0, "porcelain", &status_format,
+			    "machine-readable output", STATUS_FORMAT_PORCELAIN),
+		OPT_BOOLEAN('z', "null", &null_termination,
+			    "terminate entries with NUL"),
+		OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
+		OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"),
+		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+		/* end commit contents options */
+
+		{ OPTION_BOOLEAN, 0, "allow-empty", &allow_empty, NULL,
+		  "ok to record an empty change",
+		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+		{ OPTION_BOOLEAN, 0, "allow-empty-message", &allow_empty_message, NULL,
+		  "ok to record a change with an empty message",
+		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+
+		OPT_END()
+	};
+
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf author_ident = STRBUF_INIT;
 	const char *index_file, *reflog_msg;
@@ -1449,7 +1449,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		if (!current_head || parse_commit(current_head))
 			die(_("could not parse HEAD commit"));
 	}
-	argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
+	argc = parse_and_validate_options(argc, argv, builtin_commit_options,
+					  builtin_commit_usage,
 					  prefix, current_head, &s);
 	if (dry_run)
 		return dry_run_commit(argc, argv, prefix, current_head, &s);
-- 
1.7.10.1.12.gd79f7ab
Jeff King· May 7, 2012, 21:23 UTC · re: Jeff King · lore

[PATCH 2/5] status: refactor colopts handling

The current code reads the config and command-line options into a separate "colopts" variable, and then copies the contents of that variable into the "struct wt_status". We can eliminate the extra variable and copy just write straight into the wt_status struct.

This simplifies the "status" code a little bit. Unfortunately, it makes the "commit" code one line more complex; a side effect of the separate variable was that "commit" did not copy the colopts variable, so any column.status configuration had no effect.

The result still ends up cleaner, though. In the previous version, it was unclear whether commit simply forgot to copy the colopt variable, or whether it was intentional. Now it explicitly turns off column options. Furthermore, if commit later learns to respect column.status, this will make the end result simpler. I punted on just adding that feature now, because it was sufficiently non-obvious that it should not go into a refactoring patch.

Signed-off-by: Jeff King <peff@peff.net>
---
For the reasons mentioned above, this ended up as a less impressive
cleanup than I had hoped. I think it's still worth doing, but I won't be
too sad if it gets dropped.
 builtin/commit.c | 13 ++++++-------
 wt-status.h      |  2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)
Show changes to 2 files +7 −8

builtin/commit.c, wt-status.h

diff --git a/builtin/commit.c b/builtin/commit.c
index 5f48e8a..1a69cb0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -89,7 +89,6 @@ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int no_post_rewrite, allow_empty_message;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
 static char *sign_commit;
-static unsigned int colopts;
 
 /*
  * The default commit message cleanup mode will remove the lines
@@ -1123,7 +1122,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!prefixcmp(k, "column."))
-		return git_column_config(k, v, "status", &colopts);
+		return git_column_config(k, v, "status", &s->colopts);
 	if (!strcmp(k, "status.submodulesummary")) {
 		int is_bool;
 		s->submodule_summary = git_config_bool_or_int(k, v, &is_bool);
@@ -1166,7 +1165,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
 
 int cmd_status(int argc, const char **argv, const char *prefix)
 {
-	struct wt_status s;
+	static struct wt_status s;
 	int fd;
 	unsigned char sha1[20];
 	static struct option builtin_status_options[] = {
@@ -1189,7 +1188,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		{ OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg, "when",
 		  "ignore changes to submodules, optional when: all, dirty, untracked. (Default: all)",
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
-		OPT_COLUMN(0, "column", &colopts, "list untracked files in columns"),
+		OPT_COLUMN(0, "column", &s.colopts, "list untracked files in columns"),
 		OPT_END(),
 	};
 
@@ -1203,8 +1202,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix,
 			     builtin_status_options,
 			     builtin_status_usage, 0);
-	finalize_colopts(&colopts, -1);
-	s.colopts = colopts;
+	finalize_colopts(&s.colopts, -1);
 
 	if (null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
@@ -1369,6 +1367,7 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
+	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, "suppress summary after successful commit"),
 		OPT__VERBOSE(&verbose, "show diff in commit message template"),
@@ -1431,7 +1430,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct commit_list *parents = NULL, **pptr = &parents;
 	struct stat statbuf;
 	int allow_fast_forward = 1;
-	struct wt_status s;
 	struct commit *current_head = NULL;
 	struct commit_extra_header *extra = NULL;
 
@@ -1441,6 +1439,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	wt_status_prepare(&s);
 	git_config(git_commit_config, &s);
 	determine_whence(&s);
+	s.colopts = 0;
 
 	if (get_sha1("HEAD", sha1))
 		current_head = NULL;
diff --git a/wt-status.h b/wt-status.h
index 6dd7207..5386077 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -56,7 +56,7 @@ struct wt_status {
 	enum untracked_status_type show_untracked_files;
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
-	int colopts;
+	unsigned colopts;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
-- 
1.7.10.1.12.gd79f7ab
Nguyen Thai Ngoc Duy· May 8, 2012, 13:27 UTC · re: Jeff King · lore

Re: [PATCH 2/5] status: refactor colopts handling

On Tue, May 8, 2012 at 4:23 AM, Jeff King <peff@peff.net> wrote:
Show 26 quoted lines
> The current code reads the config and command-line options
> into a separate "colopts" variable, and then copies the
> contents of that variable into the "struct wt_status". We
> can eliminate the extra variable and copy just write
> straight into the wt_status struct.
>
> This simplifies the "status" code a little bit.
> Unfortunately, it makes the "commit" code one line more
> complex; a side effect of the separate variable was that
> "commit" did not copy the colopts variable, so any
> column.status configuration had no effect.
>
> The result still ends up cleaner, though. In the previous
> version, it was unclear whether commit simply forgot to copy
> the colopt variable, or whether it was intentional. Now it
> explicitly turns off column options. Furthermore, if commit
> later learns to respect column.status, this will make the
> end result simpler. I punted on just adding that feature
> now, because it was sufficiently non-obvious that it should
> not go into a refactoring patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> For the reasons mentioned above, this ended up as a less impressive
> cleanup than I had hoped. I think it's still worth doing, but I won't be
> too sad if it gets dropped.
I'm neutral. But this patch reduces one line so it wins a "yes" from me.
>  builtin/commit.c | 13 ++++++-------
>  wt-status.h      |  2 +-
>  2 files changed, 7 insertions(+), 8 deletions(-)
-- 
Duy
Jeff King· May 7, 2012, 21:24 UTC · re: Jeff King · lore

[PATCH 3/5] status: refactor null_termination option

This option is passed separately to the wt_status printing functions, whereas every other formatting option is contained in the wt_status struct itself. Let's do the same here, so we can avoid passing it around through the call stack.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 17 ++++++++---------
 wt-status.c      | 26 +++++++++++++-------------
 wt-status.h      |  5 +++--
 3 files changed, 24 insertions(+), 24 deletions(-)
Show changes to 3 files +24 −24

builtin/commit.c, wt-status.c, wt-status.h

diff --git a/builtin/commit.c b/builtin/commit.c
index 1a69cb0..11271c0 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -110,7 +110,6 @@ static int show_ignored_in_status;
 static const char *only_include_assumed;
 static struct strbuf message = STRBUF_INIT;
 
-static int null_termination;
 static enum {
 	STATUS_FORMAT_LONG,
 	STATUS_FORMAT_SHORT,
@@ -447,10 +446,10 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(s, null_termination, status_show_branch);
+		wt_shortstatus_print(s, status_show_branch);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(s, null_termination);
+		wt_porcelain_print(s);
 		break;
 	case STATUS_FORMAT_LONG:
 		wt_status_print(s);
@@ -1076,7 +1075,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (all && argc > 0)
 		die(_("Paths with -a does not make sense."));
 
-	if (null_termination && status_format == STATUS_FORMAT_LONG)
+	if (s->null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
 	if (status_format != STATUS_FORMAT_LONG)
 		dry_run = 1;
@@ -1177,7 +1176,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "machine-readable output",
 			    STATUS_FORMAT_PORCELAIN),
-		OPT_BOOLEAN('z', "null", &null_termination,
+		OPT_BOOLEAN('z', "null", &s.null_termination,
 			    "terminate entries with NUL"),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
 		  "mode",
@@ -1204,7 +1203,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 			     builtin_status_usage, 0);
 	finalize_colopts(&s.colopts, -1);
 
-	if (null_termination && status_format == STATUS_FORMAT_LONG)
+	if (s.null_termination && status_format == STATUS_FORMAT_LONG)
 		status_format = STATUS_FORMAT_PORCELAIN;
 
 	handle_untracked_files_arg(&s);
@@ -1229,10 +1228,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(&s, null_termination, status_show_branch);
+		wt_shortstatus_print(&s, status_show_branch);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
-		wt_porcelain_print(&s, null_termination);
+		wt_porcelain_print(&s);
 		break;
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
@@ -1404,7 +1403,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "branch", &status_show_branch, "show branch information"),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "machine-readable output", STATUS_FORMAT_PORCELAIN),
-		OPT_BOOLEAN('z', "null", &null_termination,
+		OPT_BOOLEAN('z', "null", &s.null_termination,
 			    "terminate entries with NUL"),
 		OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
 		OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"),
diff --git a/wt-status.c b/wt-status.c
index eeef17e..284dc61 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -801,7 +801,7 @@ void wt_status_print(struct wt_status *s)
 	}
 }
 
-static void wt_shortstatus_unmerged(int null_termination, struct string_list_item *it,
+static void wt_shortstatus_unmerged(struct string_list_item *it,
 			   struct wt_status *s)
 {
 	struct wt_status_change_data *d = it->util;
@@ -817,7 +817,7 @@ static void wt_shortstatus_unmerged(int null_termination, struct string_list_ite
 	case 7: how = "UU"; break; /* both modified */
 	}
 	color_fprintf(s->fp, color(WT_STATUS_UNMERGED, s), "%s", how);
-	if (null_termination) {
+	if (s->null_termination) {
 		fprintf(stdout, " %s%c", it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
@@ -828,7 +828,7 @@ static void wt_shortstatus_unmerged(int null_termination, struct string_list_ite
 	}
 }
 
-static void wt_shortstatus_status(int null_termination, struct string_list_item *it,
+static void wt_shortstatus_status(struct string_list_item *it,
 			 struct wt_status *s)
 {
 	struct wt_status_change_data *d = it->util;
@@ -842,7 +842,7 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 	else
 		putchar(' ');
 	putchar(' ');
-	if (null_termination) {
+	if (s->null_termination) {
 		fprintf(stdout, "%s%c", it->string, 0);
 		if (d->head_path)
 			fprintf(stdout, "%s%c", d->head_path, 0);
@@ -870,10 +870,10 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 	}
 }
 
-static void wt_shortstatus_other(int null_termination, struct string_list_item *it,
+static void wt_shortstatus_other(struct string_list_item *it,
 				 struct wt_status *s, const char *sign)
 {
-	if (null_termination) {
+	if (s->null_termination) {
 		fprintf(stdout, "%s %s%c", sign, it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
@@ -941,7 +941,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	color_fprintf_ln(s->fp, header_color, "]");
 }
 
-void wt_shortstatus_print(struct wt_status *s, int null_termination, int show_branch)
+void wt_shortstatus_print(struct wt_status *s, int show_branch)
 {
 	int i;
 
@@ -955,28 +955,28 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination, int show_br
 		it = &(s->change.items[i]);
 		d = it->util;
 		if (d->stagemask)
-			wt_shortstatus_unmerged(null_termination, it, s);
+			wt_shortstatus_unmerged(it, s);
 		else
-			wt_shortstatus_status(null_termination, it, s);
+			wt_shortstatus_status(it, s);
 	}
 	for (i = 0; i < s->untracked.nr; i++) {
 		struct string_list_item *it;
 
 		it = &(s->untracked.items[i]);
-		wt_shortstatus_other(null_termination, it, s, "??");
+		wt_shortstatus_other(it, s, "??");
 	}
 	for (i = 0; i < s->ignored.nr; i++) {
 		struct string_list_item *it;
 
 		it = &(s->ignored.items[i]);
-		wt_shortstatus_other(null_termination, it, s, "!!");
+		wt_shortstatus_other(it, s, "!!");
 	}
 }
 
-void wt_porcelain_print(struct wt_status *s, int null_termination)
+void wt_porcelain_print(struct wt_status *s)
 {
 	s->use_color = 0;
 	s->relative_paths = 0;
 	s->prefix = NULL;
-	wt_shortstatus_print(s, null_termination, 0);
+	wt_shortstatus_print(s, 0);
 }
diff --git a/wt-status.h b/wt-status.h
index 5386077..26dd21a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -57,6 +57,7 @@ struct wt_status {
 	const char *ignore_submodule_arg;
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
 	unsigned colopts;
+	int null_termination;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
@@ -73,8 +74,8 @@ 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_shortstatus_print(struct wt_status *s, int null_termination, int show_branch);
-void wt_porcelain_print(struct wt_status *s, int null_termination);
+void wt_shortstatus_print(struct wt_status *s, int show_branch);
+void wt_porcelain_print(struct wt_status *s);
 
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...)
 	;
-- 
1.7.10.1.12.gd79f7ab
Jeff King· May 7, 2012, 21:24 UTC · re: Jeff King · lore

[PATCH 4/5] status: fix null termination with "-b"

When the "-z" option is given to status, we are supposed to NUL-terminate each record. However, the "-b" code to show the tracking branch did not respect this, and always ended with a newline.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t7508-status.sh | 9 +++++++++
 wt-status.c       | 7 ++++---
 2 files changed, 13 insertions(+), 3 deletions(-)
Show changes to 2 files +13 −3

t/t7508-status.sh, wt-status.c

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 8f5cfac..f60f49b 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -295,6 +295,15 @@ test_expect_success 'status -s -b' '
 
 '
 
+test_expect_success 'status -s -z -b' '
+	tr "\\n" Q <expect >expect.q &&
+	mv expect.q expect &&
+	git status -s -z -b >output &&
+	nul_to_q <output >output.q &&
+	mv output.q output &&
+	test_cmp expect output
+'
+
 test_expect_success 'setup dir3' '
 	mkdir dir3 &&
 	: >dir3/untracked1 &&
diff --git a/wt-status.c b/wt-status.c
index 284dc61..21c0d4d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -913,8 +913,8 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 	if (s->is_initial)
 		color_fprintf(s->fp, header_color, _("Initial commit on "));
 	if (!stat_tracking_info(branch, &num_ours, &num_theirs)) {
-		color_fprintf_ln(s->fp, branch_color_local,
-			"%s", branch_name);
+		color_fprintf(s->fp, branch_color_local, "%s", branch_name);
+		fputc(s->null_termination ? '\0' : '\n', s->fp);
 		return;
 	}
 
@@ -938,7 +938,8 @@ static void wt_shortstatus_print_tracking(struct wt_status *s)
 		color_fprintf(s->fp, branch_color_remote, "%d", num_theirs);
 	}
 
-	color_fprintf_ln(s->fp, header_color, "]");
+	color_fprintf(s->fp, header_color, "]");
+	fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
 void wt_shortstatus_print(struct wt_status *s, int show_branch)
-- 
1.7.10.1.12.gd79f7ab
Jeff King· May 7, 2012, 21:25 UTC · re: Jeff King · lore

[PATCH 5/5] status: respect "-b" for porcelain format

There is no reason not to, as the user has to explicitly ask for it, so we are not breaking compatibility by doing so. We can do this simply by moving the "show_branch" flag into the wt_status struct. As a bonus, this saves us from passing it explicitly, simplifying the code.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c  | 9 ++++-----
 t/t7508-status.sh | 7 ++++++-
 wt-status.c       | 6 +++---
 wt-status.h       | 3 ++-
 4 files changed, 15 insertions(+), 10 deletions(-)
Show changes to 4 files +15 −10

builtin/commit.c, t/t7508-status.sh, wt-status.c, wt-status.h

diff --git a/builtin/commit.c b/builtin/commit.c
index 11271c0..a2ec73d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -115,7 +115,6 @@ static enum {
 	STATUS_FORMAT_SHORT,
 	STATUS_FORMAT_PORCELAIN
 } status_format = STATUS_FORMAT_LONG;
-static int status_show_branch;
 
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
@@ -446,7 +445,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(s, status_show_branch);
+		wt_shortstatus_print(s);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
 		wt_porcelain_print(s);
@@ -1171,7 +1170,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT__VERBOSE(&verbose, "be verbose"),
 		OPT_SET_INT('s', "short", &status_format,
 			    "show status concisely", STATUS_FORMAT_SHORT),
-		OPT_BOOLEAN('b', "branch", &status_show_branch,
+		OPT_BOOLEAN('b', "branch", &s.show_branch,
 			    "show branch information"),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "machine-readable output",
@@ -1228,7 +1227,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	switch (status_format) {
 	case STATUS_FORMAT_SHORT:
-		wt_shortstatus_print(&s, status_show_branch);
+		wt_shortstatus_print(&s);
 		break;
 	case STATUS_FORMAT_PORCELAIN:
 		wt_porcelain_print(&s);
@@ -1400,7 +1399,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "dry-run", &dry_run, "show what would be committed"),
 		OPT_SET_INT(0, "short", &status_format, "show status concisely",
 			    STATUS_FORMAT_SHORT),
-		OPT_BOOLEAN(0, "branch", &status_show_branch, "show branch information"),
+		OPT_BOOLEAN(0, "branch", &s.show_branch, "show branch information"),
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "machine-readable output", STATUS_FORMAT_PORCELAIN),
 		OPT_BOOLEAN('z', "null", &s.null_termination,
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index f60f49b..28e1848 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -680,9 +680,14 @@ test_expect_success 'status --porcelain ignores color.status' '
 git config --unset color.status
 git config --unset color.ui
 
-test_expect_success 'status --porcelain ignores -b' '
+test_expect_success 'status --porcelain respects -b' '
 
 	git status --porcelain -b >output &&
+	{
+		echo "## master" &&
+		cat expect
+	} >tmp &&
+	mv tmp expect &&
 	test_cmp expect output
 
 '
diff --git a/wt-status.c b/wt-status.c
index 21c0d4d..dd6d8c4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -942,11 +942,11 @@ 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, int show_branch)
+void wt_shortstatus_print(struct wt_status *s)
 {
 	int i;
 
-	if (show_branch)
+	if (s->show_branch)
 		wt_shortstatus_print_tracking(s);
 
 	for (i = 0; i < s->change.nr; i++) {
@@ -979,5 +979,5 @@ void wt_porcelain_print(struct wt_status *s)
 	s->use_color = 0;
 	s->relative_paths = 0;
 	s->prefix = NULL;
-	wt_shortstatus_print(s, 0);
+	wt_shortstatus_print(s);
 }
diff --git a/wt-status.h b/wt-status.h
index 26dd21a..14aa9f7 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -58,6 +58,7 @@ struct wt_status {
 	char color_palette[WT_STATUS_MAXSLOT][COLOR_MAXLEN];
 	unsigned colopts;
 	int null_termination;
+	int show_branch;
 
 	/* These are computed during processing of the individual sections */
 	int commitable;
@@ -74,7 +75,7 @@ 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_shortstatus_print(struct wt_status *s, int show_branch);
+void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
 
 void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...)
-- 
1.7.10.1.12.gd79f7ab
Jeff King· May 7, 2012, 21:28 UTC · re: Jeff King · lore

Re: [PATCH 5/5] status: respect "-b" for porcelain format

On Mon, May 07, 2012 at 05:25:04PM -0400, Jeff King wrote:
Show 5 quoted lines
> There is no reason not to, as the user has to explicitly ask
> for it, so we are not breaking compatibility by doing so. We
> can do this simply by moving the "show_branch" flag into
> the wt_status struct. As a bonus, this saves us from passing
> it explicitly, simplifying the code.
I forgot this, which should probably be squashed in:
-- >8 --
Subject: status: fix "--porcelain -z -b" documentation
It is no longer the case that "-z" will not respect "-b".
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-status.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/git-status.txt +1 −1
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 2883a28..67e5f53 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -184,7 +184,7 @@ order is reversed (e.g 'from \-> to' becomes 'to from'). Second, a NUL
 and the terminating newline (but a space still separates the status
 field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
-backslash-escaping is performed. Fourth, there is no branch line.
+backslash-escaping is performed.
 
 CONFIGURATION
 -------------
-- 
1.7.10.1.12.gd79f7ab
Junio C Hamano· May 8, 2012, 04:55 UTC · re: Jeff King · lore

Re: [PATCH 5/5] status: respect "-b" for porcelain format

Jeff King <peff@peff.net> writes:
Show 16 quoted lines
> On Mon, May 07, 2012 at 05:25:04PM -0400, Jeff King wrote:
>
>> There is no reason not to, as the user has to explicitly ask
>> for it, so we are not breaking compatibility by doing so. We
>> can do this simply by moving the "show_branch" flag into
>> the wt_status struct. As a bonus, this saves us from passing
>> it explicitly, simplifying the code.
>
> I forgot this, which should probably be squashed in:
>
> -- >8 --
> Subject: status: fix "--porcelain -z -b" documentation
>
> It is no longer the case that "-z" will not respect "-b".
>
> Signed-off-by: Jeff King <peff@peff.net>

All patches looked good. I thought that this deserved to eventually land on 'maint', so I did my usual "am first to master primarily to populate the object database with exact blob objects, then rebase the result to 'maint' while resolving conflicts both textual and semantic, and finally attempt to merge it back to 'master'" dance to come up with a two topic branches. The result is queued in 'pu'.

Thanks.
Jeff King· May 8, 2012, 09:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 5/5] status: respect "-b" for porcelain format

On Mon, May 07, 2012 at 09:55:05PM -0700, Junio C Hamano wrote:
Show 6 quoted lines
> All patches looked good.  I thought that this deserved to eventually land
> on 'maint', so I did my usual "am first to master primarily to populate
> the object database with exact blob objects, then rebase the result to
> 'maint' while resolving conflicts both textual and semantic, and finally
> attempt to merge it back to 'master'" dance to come up with a two topic
> branches.  The result is queued in 'pu'.

I eyeballed the result. It all looks good except for patch 2 (refactor colopts handling), which now has a totally bogus commit message. Since colopts came much later, the only thing left in the patch is the movement of the local "struct wt_status s" in cmd_{commit,status} to a static at the top of each function (so that it can be referenced directly by the options list).

Probably that patch (94f98829) should just be squashed into its parent. And then the colopts bits can be moved to the very end, after the rest of it, and be a part of only the non-maint topic.

I've pushed that result out to the jk/{maint-,}status-porcelain-z-b branches at git://github.com/peff/git.git if that helps.

-Peff
Junio C Hamano· May 8, 2012, 17:52 UTC · re: Jeff King · lore

Re: [PATCH 5/5] status: respect "-b" for porcelain format

Jeff King <peff@peff.net> writes:
> Probably that patch (94f98829) should just be squashed into its parent.
> And then the colopts bits can be moved to the very end, after the rest
> of it, and be a part of only the non-maint topic.
Makes sense.  Thanks.
Show 5 quoted lines
>
> I've pushed that result out to the jk/{maint-,}status-porcelain-z-b
> branches at git://github.com/peff/git.git if that helps.
>
> -Peff
Junio C Hamano· May 7, 2012, 18:03 UTC · re: Jeff King · lore

Re: [PATCH] correct git-status Porcelain Format documentation

Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> The patch itself looks obviously correct and describes the current
> behavior. But I have to wonder: the --short format will also not produce
> the branch line unless you provide "-b". So why is it that the porcelain
> format does not respect "-b", since anybody who asked for it would
> obviously be expecting to find and parse it?
>
> Should this bit of documentation be dropped in favor of just making "-b"
> work properly?

I would say so. Also "branch line" was an undefined term and I had to read the whole thing to find out what the updated documentation was talking about; we may want to consistently say "branch and tracking info" as the earlier part of the document says.

← back to recent threads