[PATCH] correct git-status Porcelain Format documentation

Subsystems: documentation, the rest

DORMANTno replies

2 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH] correct git-status Porcelain Format documentation

From: Jeff King <hidden>
Date: 2016-06-15 22:53:45

From: Zak Johnson <redacted>

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 <redacted>
---
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(-)
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

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

From: Jeff King <hidden>
Date: 2016-06-15 22:53:45

[+cc Zak, who I forgot in the first email]

On Sun, May 06, 2012 at 09:29:59AM -0400, Jeff King wrote:
From: Zak Johnson <redacted>

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 <redacted>
---
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 <redacted>
---
 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(-)
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help