Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:58:32

Matthieu Moy [off-list ref] writes:
quoted hunk
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..dacf4b9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
 	Set to true to enable --branch by default in linkgit:git-status[1].
 	The option --no-branch takes precedence over this variable.
 
+status.displayCommentChar::
+	If set to false, linkgit:git-status[1] will not prefix each
+	line with the comment char (`core.commentchar`, i.e. `#` by
+	default). Defaults to true.
We prefix core.commentchar followed by a single space for non-empty
lines; is that worth mentioning, I wonder?

Also I envision that we would extend core.commentchar to be more
than a single character.  Is "displayCommentChar" still a good name
for this setting?

"status.omitCommentPrefix" that defaults to false might be a better
setting, I suspect.  I further suspect that in the longer term, we
may want to consider flipping its default to true (for Git 2.0, it
is too late).  I do not think we need these comment prefix in the
human readable "status" output---after all, there is no line that is
not commented (other than "nothing added to commit" at the end) in
the current output, so the comment prefix only wastes the screen
real estate.

What are our plans to help existing scripts people have written over
time, especially before "status -s" was invented, that will be
broken by use of this?
quoted hunk
@@ -663,17 +666,18 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	char summary_limit[64];
 	char index[PATH_MAX];
 	const char *env[] = { NULL, NULL };
-	const char *argv[8];
+	const char *argv[9];
 
 	env[0] =	index;
 	argv[0] =	"submodule";
 	argv[1] =	"summary";
 	argv[2] =	uncommitted ? "--files" : "--cached";
 	argv[3] =	"--for-status";
-	argv[4] =	"--summary-limit";
-	argv[5] =	summary_limit;
-	argv[6] =	uncommitted ? NULL : (s->amend ? "HEAD^" : "HEAD");
-	argv[7] =	NULL;
+	argv[4] =	s->display_comment_char ? "--display-comment-char" : "--no-display-comment-char";
+	argv[5] =	"--summary-limit";
+	argv[6] =	summary_limit;
+	argv[7] =	uncommitted ? NULL : (s->amend ? "HEAD^" : "HEAD");
+	argv[8] =	NULL;
Not strictly your fault, but we should lose these funny indent after
"=" and use argv_array API here.

Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #

From: Jeff King <hidden>
Date: 2016-06-15 22:58:32

On Wed, Aug 28, 2013 at 01:05:38PM -0700, Junio C Hamano wrote:
What are our plans to help existing scripts people have written over
time, especially before "status -s" was invented, that will be
broken by use of this?
I thought that our response to parsing the long output of "git status"
was always "you are doing it wrong". The right way has always been to
run the plumbing tools yourself, followed eventually by the --porcelain
mode to "git status" being blessed as a convenient plumbing.

I will not say that people might not do it anyway, but at what point do
we say "you were warned"?

-Peff

Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #

From: brian m. carlson <hidden>
Date: 2016-06-15 22:58:35

On Wed, Aug 28, 2013 at 04:18:03PM -0400, Jeff King wrote:
On Wed, Aug 28, 2013 at 01:05:38PM -0700, Junio C Hamano wrote:
quoted
What are our plans to help existing scripts people have written over
time, especially before "status -s" was invented, that will be
broken by use of this?
I thought that our response to parsing the long output of "git status"
was always "you are doing it wrong". The right way has always been to
run the plumbing tools yourself, followed eventually by the --porcelain
mode to "git status" being blessed as a convenient plumbing.

I will not say that people might not do it anyway, but at what point do
we say "you were warned"?
It already has changed.  At cPanel, we had code that broke with a new
version of git because the output of git status changed between 1.7.11
and 1.8.3.  We fixed it to use --porcelain and had no problems.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help