threads / rfc / 34796

RFC v3, 4 partsDisable "git status" comment prefix

Subject: [RFC/PATCH v3 0/4] Disable "git status" comment prefix

## tl;dr

16 messages between Aug 29, 2013 and Sep 3, 2013. Diffs are folded; open one to read it.

replies: 15people: 5as markdown or json

Matthieu Moy· Aug 29, 2013, 13:05 UTC · lore
So, here's a reroll that makes the code cleaner.

First patches are cleanups without behavioral changes, only the last one does something new.

I'm waiting for more comments to decide what to do with the configuration option. Right now, my preference would be to call it status.oldStyle and default it to false (i.e. change the behavior, but allow old-timers to get back the old one).

Matthieu Moy (4):
  builtin/stripspace.c: fix broken indentation
  wt-status: use argv_array API
  get rid of "git submodule summary --for-status"
  status: introduce status.displayCommentChar to disable display of #
 Documentation/config.txt     |  5 +++
 builtin/commit.c             |  9 +++++
 builtin/stripspace.c         |  8 ++---
 git-submodule.sh             | 17 +--------
 t/t7401-submodule-summary.sh | 13 -------
 t/t7508-status.sh            | 43 +++++++++++++++++++++++
 wt-status.c                  | 82 +++++++++++++++++++++++++++++++++-----------
 wt-status.h                  |  1 +
 8 files changed, 125 insertions(+), 53 deletions(-)
-- 
1.8.4.12.gf9d53a3.dirty
Matthieu Moy· Aug 29, 2013, 13:05 UTC · re: Matthieu Moy · lore

[PATCH v3 1/4] builtin/stripspace.c: fix broken indentation

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/stripspace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Show changes to builtin/stripspace.c +4 −4
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index e981dfb..1259ed7 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -89,11 +89,11 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)
 
 	if (argc == 2) {
 		if (!strcmp(argv[1], "-s") ||
-			!strcmp(argv[1], "--strip-comments")) {
-			 strip_comments = 1;
+		    !strcmp(argv[1], "--strip-comments")) {
+			strip_comments = 1;
 		} else if (!strcmp(argv[1], "-c") ||
-					 !strcmp(argv[1], "--comment-lines")) {
-			 mode = COMMENT_LINES;
+			   !strcmp(argv[1], "--comment-lines")) {
+			mode = COMMENT_LINES;
 		} else {
 			mode = INVAL;
 		}
-- 
1.8.4.12.gf9d53a3.dirty
Matthieu Moy· Aug 29, 2013, 13:05 UTC · re: Matthieu Moy · lore

[PATCH v3 2/4] wt-status: use argv_array API

No behavior change, but two slight code reorganization: argv_array_push doesn't accept NULL strings, and duplicates its argument hence summary_limit must be written to before being inserted into argv.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 wt-status.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)
Show changes to wt-status.c +14 −12
diff --git a/wt-status.c b/wt-status.c
index cb24f1f..958a53c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -8,6 +8,7 @@
 #include "diffcore.h"
 #include "quote.h"
 #include "run-command.h"
+#include "argv-array.h"
 #include "remote.h"
 #include "refs.h"
 #include "submodule.h"
@@ -663,29 +664,30 @@ 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];
-
-	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;
+	struct argv_array argv = ARGV_ARRAY_INIT;
 
 	sprintf(summary_limit, "%d", s->submodule_summary);
 	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
 
+	env[0] = index;
+	argv_array_push(&argv, "submodule");
+	argv_array_push(&argv, "summary");
+	argv_array_push(&argv, uncommitted ? "--files" : "--cached");
+	argv_array_push(&argv, "--for-status");
+	argv_array_push(&argv, "--summary-limit");
+	argv_array_push(&argv, summary_limit);
+	if (!uncommitted)
+		argv_array_push(&argv, s->amend ? "HEAD^" : "HEAD");
+
 	memset(&sm_summary, 0, sizeof(sm_summary));
-	sm_summary.argv = argv;
+	sm_summary.argv = argv.argv;
 	sm_summary.env = env;
 	sm_summary.git_cmd = 1;
 	sm_summary.no_stdin = 1;
 	fflush(s->fp);
 	sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
 	run_command(&sm_summary);
+	argv_array_clear(&argv);
 }
 
 static void wt_status_print_other(struct wt_status *s,
-- 
1.8.4.12.gf9d53a3.dirty
Matthieu Moy· Aug 29, 2013, 13:05 UTC · re: Matthieu Moy · lore

[PATCH v3 3/4] get rid of "git submodule summary --for-status"

The --for-status option was an undocumented option used only by wt-status.c, which inserted a header and commented out the output. We can achieve the same result within wt-status.c, without polluting the submodule command-line options.

This will make it easier to disable the comments from wt-status.c later.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 git-submodule.sh             | 17 +----------------
 t/t7401-submodule-summary.sh | 13 -------------
 wt-status.c                  | 29 +++++++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 31 deletions(-)
Show changes to 3 files +28 −31

git-submodule.sh, t/t7401-submodule-summary.sh, wt-status.c

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..fccdec9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -965,7 +965,6 @@ set_name_rev () {
 #
 cmd_summary() {
 	summary_limit=-1
-	for_status=
 	diff_cmd=diff-index
 
 	# parse $args after "submodule ... summary".
@@ -978,9 +977,6 @@ cmd_summary() {
 		--files)
 			files="$1"
 			;;
-		--for-status)
-			for_status="$1"
-			;;
 		-n|--summary-limit)
 			summary_limit="$2"
 			isnumber "$summary_limit" || usage
@@ -1149,18 +1145,7 @@ cmd_summary() {
 			echo
 		fi
 		echo
-	done |
-	if test -n "$for_status"; then
-		if [ -n "$files" ]; then
-			gettextln "Submodules changed but not updated:" | git stripspace -c
-		else
-			gettextln "Submodule changes to be committed:" | git stripspace -c
-		fi
-		printf "\n" | git stripspace -c
-		git stripspace -c
-	else
-		cat
-	fi
+	done
 }
 #
 # List all submodules, prefixed with:
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index ac2434c..b435d03 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -262,19 +262,6 @@ EOF
 	test_cmp expected actual
 "
 
-test_expect_success '--for-status' "
-	git submodule summary --for-status HEAD^ >actual &&
-	test_i18ncmp actual - <<EOF
-# Submodule changes to be committed:
-#
-# * sm1 $head6...0000000:
-#
-# * sm2 0000000...$head7 (2):
-#   > Add foo9
-#
-EOF
-"
-
 test_expect_success 'fail when using --files together with --cached' "
 	test_must_fail git submodule summary --files --cached
 "
diff --git a/wt-status.c b/wt-status.c
index 958a53c..d91661d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -665,6 +665,10 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	char index[PATH_MAX];
 	const char *env[] = { NULL, NULL };
 	struct argv_array argv = ARGV_ARRAY_INIT;
+	struct strbuf cmd_stdout = STRBUF_INIT;
+	struct strbuf summary = STRBUF_INIT;
+	char *summary_content;
+	size_t len;
 
 	sprintf(summary_limit, "%d", s->submodule_summary);
 	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
@@ -673,7 +677,6 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	argv_array_push(&argv, "submodule");
 	argv_array_push(&argv, "summary");
 	argv_array_push(&argv, uncommitted ? "--files" : "--cached");
-	argv_array_push(&argv, "--for-status");
 	argv_array_push(&argv, "--summary-limit");
 	argv_array_push(&argv, summary_limit);
 	if (!uncommitted)
@@ -685,9 +688,31 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	sm_summary.git_cmd = 1;
 	sm_summary.no_stdin = 1;
 	fflush(s->fp);
-	sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
+	sm_summary.out = -1;
+
 	run_command(&sm_summary);
 	argv_array_clear(&argv);
+
+	len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);
+
+	/* prepend header, only if there's an actual output */
+	if (len) {
+		if (uncommitted)
+			strbuf_addstr(&summary, _("Submodules changed but not updated:"));
+		else
+			strbuf_addstr(&summary, _("Submodule changes to be committed:"));
+		strbuf_addstr(&summary, "\n\n");
+	}
+	strbuf_addbuf(&summary, &cmd_stdout);
+	strbuf_release(&cmd_stdout);
+
+	summary_content = strbuf_detach(&summary, &len);
+	strbuf_add_commented_lines(&summary, summary_content, len);
+	free(summary_content);
+
+	summary_content = strbuf_detach(&summary, &len);
+	fprintf(s->fp, summary_content);
+	free(summary_content);
 }
 
 static void wt_status_print_other(struct wt_status *s,
-- 
1.8.4.12.gf9d53a3.dirty
Jens Lehmann· Aug 29, 2013, 19:54 UTC · re: Matthieu Moy · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 29.08.2013 15:05, schrieb Matthieu Moy:
Show 6 quoted lines
> The --for-status option was an undocumented option used only by
> wt-status.c, which inserted a header and commented out the output. We can
> achieve the same result within wt-status.c, without polluting the
> submodule command-line options.
> 
> This will make it easier to disable the comments from wt-status.c later.
Cool, thanks for implementing this!

But unfortunately this change collides with bc/submodule-status-ignored (I added Brian to the CC) which is currently on its way to next. Your patch will break the fix in the second commit, because that's only enabled when the submodule script sees the --for-status option.

A solution for that would be to rebase your patches on top of pu, drop the first two hunks of the change to git-submodule.sh and still pass the --for-status option to git-submodule.sh. This would move adding the comment characters into wt-status.c but will still enable the script to honor the ignore=all setting when called by status.

Junio, what is our take on changing behavior of undocumented internally used options? Do we have to add a new --for-status2 (which doesn't add the comment characters) or is it ok to just change the behavior of the existing option?

Show 130 quoted lines
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  git-submodule.sh             | 17 +----------------
>  t/t7401-submodule-summary.sh | 13 -------------
>  wt-status.c                  | 29 +++++++++++++++++++++++++++--
>  3 files changed, 28 insertions(+), 31 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..fccdec9 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -965,7 +965,6 @@ set_name_rev () {
>  #
>  cmd_summary() {
>  	summary_limit=-1
> -	for_status=
>  	diff_cmd=diff-index
>  
>  	# parse $args after "submodule ... summary".
> @@ -978,9 +977,6 @@ cmd_summary() {
>  		--files)
>  			files="$1"
>  			;;
> -		--for-status)
> -			for_status="$1"
> -			;;
>  		-n|--summary-limit)
>  			summary_limit="$2"
>  			isnumber "$summary_limit" || usage
> @@ -1149,18 +1145,7 @@ cmd_summary() {
>  			echo
>  		fi
>  		echo
> -	done |
> -	if test -n "$for_status"; then
> -		if [ -n "$files" ]; then
> -			gettextln "Submodules changed but not updated:" | git stripspace -c
> -		else
> -			gettextln "Submodule changes to be committed:" | git stripspace -c
> -		fi
> -		printf "\n" | git stripspace -c
> -		git stripspace -c
> -	else
> -		cat
> -	fi
> +	done
>  }
>  #
>  # List all submodules, prefixed with:
> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
> index ac2434c..b435d03 100755
> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
> @@ -262,19 +262,6 @@ EOF
>  	test_cmp expected actual
>  "
>  
> -test_expect_success '--for-status' "
> -	git submodule summary --for-status HEAD^ >actual &&
> -	test_i18ncmp actual - <<EOF
> -# Submodule changes to be committed:
> -#
> -# * sm1 $head6...0000000:
> -#
> -# * sm2 0000000...$head7 (2):
> -#   > Add foo9
> -#
> -EOF
> -"
> -
>  test_expect_success 'fail when using --files together with --cached' "
>  	test_must_fail git submodule summary --files --cached
>  "
> diff --git a/wt-status.c b/wt-status.c
> index 958a53c..d91661d 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -665,6 +665,10 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>  	char index[PATH_MAX];
>  	const char *env[] = { NULL, NULL };
>  	struct argv_array argv = ARGV_ARRAY_INIT;
> +	struct strbuf cmd_stdout = STRBUF_INIT;
> +	struct strbuf summary = STRBUF_INIT;
> +	char *summary_content;
> +	size_t len;
>  
>  	sprintf(summary_limit, "%d", s->submodule_summary);
>  	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
> @@ -673,7 +677,6 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>  	argv_array_push(&argv, "submodule");
>  	argv_array_push(&argv, "summary");
>  	argv_array_push(&argv, uncommitted ? "--files" : "--cached");
> -	argv_array_push(&argv, "--for-status");
>  	argv_array_push(&argv, "--summary-limit");
>  	argv_array_push(&argv, summary_limit);
>  	if (!uncommitted)
> @@ -685,9 +688,31 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>  	sm_summary.git_cmd = 1;
>  	sm_summary.no_stdin = 1;
>  	fflush(s->fp);
> -	sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
> +	sm_summary.out = -1;
> +
>  	run_command(&sm_summary);
>  	argv_array_clear(&argv);
> +
> +	len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);
> +
> +	/* prepend header, only if there's an actual output */
> +	if (len) {
> +		if (uncommitted)
> +			strbuf_addstr(&summary, _("Submodules changed but not updated:"));
> +		else
> +			strbuf_addstr(&summary, _("Submodule changes to be committed:"));
> +		strbuf_addstr(&summary, "\n\n");
> +	}
> +	strbuf_addbuf(&summary, &cmd_stdout);
> +	strbuf_release(&cmd_stdout);
> +
> +	summary_content = strbuf_detach(&summary, &len);
> +	strbuf_add_commented_lines(&summary, summary_content, len);
> +	free(summary_content);
> +
> +	summary_content = strbuf_detach(&summary, &len);
> +	fprintf(s->fp, summary_content);
> +	free(summary_content);
>  }
>  
>  static void wt_status_print_other(struct wt_status *s,
> 
Matthieu Moy· Aug 29, 2013, 21:23 UTC · re: Jens Lehmann · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 12 quoted lines
> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>> The --for-status option was an undocumented option used only by
>> wt-status.c, which inserted a header and commented out the output. We can
>> achieve the same result within wt-status.c, without polluting the
>> submodule command-line options.
>> 
>> This will make it easier to disable the comments from wt-status.c later.
>
> Cool, thanks for implementing this!
>
> But unfortunately this change collides with bc/submodule-status-ignored
> (I added Brian to the CC) which is currently on its way to next.
Thanks for pointing that out. The patch looks buggy:
Show changes to git-submodule.sh +11 −0
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1036,6 +1036,13 @@ cmd_summary() {
                do
                        # Always show modules deleted or type-changed (blob<->module)
                        test $status = D -o $status = T && echo "$sm_path" && continue
+                       # Respect the ignore setting for --for-status.
+                       if test -n $for_status
+                       then
+                               name=$(module_name "$sm_path")
+                               ignore_config=$(get_submodule_config "$name" ignore none)
+                               test $status != A -a $ignore_config = all && continue
+                       fi

Because of the missing quotes around $for_status, it seems the test is
unconditionnaly true:

$ test -n t ; echo $?
0
$ test -n   ; echo $?
0

This makes me wonder why the ignore configuration should be considered
only with --for-status. Why not turn that into

--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1036,6 +1036,13 @@ cmd_summary() {
                do
                        # Always show modules deleted or type-changed (blob<->module)
                        test $status = D -o $status = T && echo "$sm_path" && continue
+			# Respect the ignore setting
+			name=$(module_name "$sm_path")
+			ignore_config=$(get_submodule_config "$name" ignore none)
+			test $status != A -a $ignore_config = all && continue

?
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Jens Lehmann· Aug 30, 2013, 19:40 UTC · re: Matthieu Moy · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 29.08.2013 23:23, schrieb Matthieu Moy:
Show 16 quoted lines
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>>> The --for-status option was an undocumented option used only by
>>> wt-status.c, which inserted a header and commented out the output. We can
>>> achieve the same result within wt-status.c, without polluting the
>>> submodule command-line options.
>>>
>>> This will make it easier to disable the comments from wt-status.c later.
>>
>> Cool, thanks for implementing this!
>>
>> But unfortunately this change collides with bc/submodule-status-ignored
>> (I added Brian to the CC) which is currently on its way to next.
> 
> Thanks for pointing that out. The patch looks buggy:
Ok, I'll tak
Show 38 quoted lines
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1036,6 +1036,13 @@ cmd_summary() {
>                 do
>                         # Always show modules deleted or type-changed (blob<->module)
>                         test $status = D -o $status = T && echo "$sm_path" && continue
> +                       # Respect the ignore setting for --for-status.
> +                       if test -n $for_status
> +                       then
> +                               name=$(module_name "$sm_path")
> +                               ignore_config=$(get_submodule_config "$name" ignore none)
> +                               test $status != A -a $ignore_config = all && continue
> +                       fi
> 
> Because of the missing quotes around $for_status, it seems the test is
> unconditionnaly true:
> 
> $ test -n t ; echo $?
> 0
> $ test -n   ; echo $?
> 0
> 
> This makes me wonder why the ignore configuration should be considered
> only with --for-status. Why not turn that into
> 
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1036,6 +1036,13 @@ cmd_summary() {
>                 do
>                         # Always show modules deleted or type-changed (blob<->module)
>                         test $status = D -o $status = T && echo "$sm_path" && continue
> +			# Respect the ignore setting
> +			name=$(module_name "$sm_path")
> +			ignore_config=$(get_submodule_config "$name" ignore none)
> +			test $status != A -a $ignore_config = all && continue
> 
> ?
> 
Jens Lehmann· Aug 30, 2013, 19:51 UTC · re: Jens Lehmann · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 30.08.2013 21:40, schrieb Jens Lehmann:
Show 19 quoted lines
> Am 29.08.2013 23:23, schrieb Matthieu Moy:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>>>> The --for-status option was an undocumented option used only by
>>>> wt-status.c, which inserted a header and commented out the output. We can
>>>> achieve the same result within wt-status.c, without polluting the
>>>> submodule command-line options.
>>>>
>>>> This will make it easier to disable the comments from wt-status.c later.
>>>
>>> Cool, thanks for implementing this!
>>>
>>> But unfortunately this change collides with bc/submodule-status-ignored
>>> (I added Brian to the CC) which is currently on its way to next.
>>
>> Thanks for pointing that out. The patch looks buggy:
> 
> Ok, I'll tak
Sorry, I accidentally hit "Send" ... :-(
Ok, I'll take a look and will comment on that soon.
Show 24 quoted lines
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -1036,6 +1036,13 @@ cmd_summary() {
>>                 do
>>                         # Always show modules deleted or type-changed (blob<->module)
>>                         test $status = D -o $status = T && echo "$sm_path" && continue
>> +                       # Respect the ignore setting for --for-status.
>> +                       if test -n $for_status
>> +                       then
>> +                               name=$(module_name "$sm_path")
>> +                               ignore_config=$(get_submodule_config "$name" ignore none)
>> +                               test $status != A -a $ignore_config = all && continue
>> +                       fi
>>
>> Because of the missing quotes around $for_status, it seems the test is
>> unconditionnaly true:
>>
>> $ test -n t ; echo $?
>> 0
>> $ test -n   ; echo $?
>> 0
>>
>> This makes me wonder why the ignore configuration should be considered
>> only with --for-status. Why not turn that into

Please don't. That changes the default behavior of submodule summary, which never ignores any submodules. The ignore logic was added to core git after commands like diff and status learned to check submodules for modifications too. That was bad for people who used submodules to store many and/or huge files in a way that wouldn't slow down diff or status, as it slowed them down again. The ignore option allowed them to continue using submodules for that purpose. They still need to have the submodule script ignore the ignore settings, because running them is the point in time they want to take the extra effort to look into those submodules they normally ignore. And that's why the submodule totally lacks any option to control the ignore behavior, which we would also have to add if we would follow your proposal.

So I think it's either changing the default behavior of --for-status or adding another option (--for-status-wo-comment or such) which will honor the ignore setting only when called from status.

Show 14 quoted lines
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -1036,6 +1036,13 @@ cmd_summary() {
>>                 do
>>                         # Always show modules deleted or type-changed (blob<->module)
>>                         test $status = D -o $status = T && echo "$sm_path" && continue
>> +			# Respect the ignore setting
>> +			name=$(module_name "$sm_path")
>> +			ignore_config=$(get_submodule_config "$name" ignore none)
>> +			test $status != A -a $ignore_config = all && continue
>>
>> ?
>>
> 
Jens Lehmann· Aug 30, 2013, 20:08 UTC · re: Jens Lehmann · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 30.08.2013 21:51, schrieb Jens Lehmann:
Show 46 quoted lines
> Am 30.08.2013 21:40, schrieb Jens Lehmann:
>> Am 29.08.2013 23:23, schrieb Matthieu Moy:
>>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>
>>>> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>>>>> The --for-status option was an undocumented option used only by
>>>>> wt-status.c, which inserted a header and commented out the output. We can
>>>>> achieve the same result within wt-status.c, without polluting the
>>>>> submodule command-line options.
>>>>>
>>>>> This will make it easier to disable the comments from wt-status.c later.
>>>>
>>>> Cool, thanks for implementing this!
>>>>
>>>> But unfortunately this change collides with bc/submodule-status-ignored
>>>> (I added Brian to the CC) which is currently on its way to next.
>>>
>>> Thanks for pointing that out. The patch looks buggy:
>>
>> Ok, I'll tak
> 
> Sorry, I accidentally hit "Send" ... :-(
> 
> Ok, I'll take a look and will comment on that soon.
> 
>>> --- a/git-submodule.sh
>>> +++ b/git-submodule.sh
>>> @@ -1036,6 +1036,13 @@ cmd_summary() {
>>>                 do
>>>                         # Always show modules deleted or type-changed (blob<->module)
>>>                         test $status = D -o $status = T && echo "$sm_path" && continue
>>> +                       # Respect the ignore setting for --for-status.
>>> +                       if test -n $for_status
>>> +                       then
>>> +                               name=$(module_name "$sm_path")
>>> +                               ignore_config=$(get_submodule_config "$name" ignore none)
>>> +                               test $status != A -a $ignore_config = all && continue
>>> +                       fi
>>>
>>> Because of the missing quotes around $for_status, it seems the test is
>>> unconditionnaly true:
>>>
>>> $ test -n t ; echo $?
>>> 0
>>> $ test -n   ; echo $?
>>> 0

Right you are, I did not notice the missing "" in my review. Looks like we also should add one or more tests making sure that submodule summary and status never honor the ignore settings.

brian m. carlson· Aug 31, 2013, 17:08 UTC · re: Jens Lehmann · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

On Fri, Aug 30, 2013 at 10:08:53PM +0200, Jens Lehmann wrote:
Show 17 quoted lines
> Am 30.08.2013 21:51, schrieb Jens Lehmann:
> > Am 30.08.2013 21:40, schrieb Jens Lehmann:
> >> Am 29.08.2013 23:23, schrieb Matthieu Moy:
> >>> Jens Lehmann <Jens.Lehmann@web.de> writes:
> >>>
> >>>> Am 29.08.2013 15:05, schrieb Matthieu Moy:
> >>> Because of the missing quotes around $for_status, it seems the test is
> >>> unconditionnaly true:
> >>>
> >>> $ test -n t ; echo $?
> >>> 0
> >>> $ test -n   ; echo $?
> >>> 0
> 
> Right you are, I did not notice the missing "" in my review. Looks like
> we also should add one or more tests making sure that submodule summary
> and status never honor the ignore settings.

How do we want to handle this? I can send a reroll and include some new tests, but if this code is going away, then there's no point.

-- 
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
Jens Lehmann· Sep 1, 2013, 13:47 UTC · re: brian m. carlson · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 31.08.2013 19:08, schrieb brian m. carlson:
Show 21 quoted lines
> On Fri, Aug 30, 2013 at 10:08:53PM +0200, Jens Lehmann wrote:
>> Am 30.08.2013 21:51, schrieb Jens Lehmann:
>>> Am 30.08.2013 21:40, schrieb Jens Lehmann:
>>>> Am 29.08.2013 23:23, schrieb Matthieu Moy:
>>>>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>>>>
>>>>>> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>>>>> Because of the missing quotes around $for_status, it seems the test is
>>>>> unconditionnaly true:
>>>>>
>>>>> $ test -n t ; echo $?
>>>>> 0
>>>>> $ test -n   ; echo $?
>>>>> 0
>>
>> Right you are, I did not notice the missing "" in my review. Looks like
>> we also should add one or more tests making sure that submodule summary
>> and status never honor the ignore settings.
> 
> How do we want to handle this?  I can send a reroll and include some
> new tests, but if this code is going away, then there's no point.

A reroll would be great, as I think your patch is a bugfix that should go in rather soonish no matter how we continue with the comment signs. Two new tests (one for submodule summary and one for submodule status) with both the global ignore setting and a submodule specific one set to "all" showing no impact on the output would suffice (and trigger the then also fixed missing "" bug ;-).

Jens Lehmann· Sep 3, 2013, 19:32 UTC · re: Jens Lehmann · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Am 29.08.2013 21:54, schrieb Jens Lehmann:
Show 20 quoted lines
> Am 29.08.2013 15:05, schrieb Matthieu Moy:
>> The --for-status option was an undocumented option used only by
>> wt-status.c, which inserted a header and commented out the output. We can
>> achieve the same result within wt-status.c, without polluting the
>> submodule command-line options.
>>
>> This will make it easier to disable the comments from wt-status.c later.
> 
> Cool, thanks for implementing this!
> 
> But unfortunately this change collides with bc/submodule-status-ignored
> (I added Brian to the CC) which is currently on its way to next. Your
> patch will break the fix in the second commit, because that's only
> enabled when the submodule script sees the --for-status option.
> 
> A solution for that would be to rebase your patches on top of pu, drop
> the first two hunks of the change to git-submodule.sh and still pass
> the --for-status option to git-submodule.sh. This would move adding the
> comment characters into wt-status.c but will still enable the script to
> honor the ignore=all setting when called by status.

I think we should go that route, --for-status is an internal option and nobody should rely on its behavior.

Show 29 quoted lines
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>>  git-submodule.sh             | 17 +----------------
>>  t/t7401-submodule-summary.sh | 13 -------------
>>  wt-status.c                  | 29 +++++++++++++++++++++++++++--
>>  3 files changed, 28 insertions(+), 31 deletions(-)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 2979197..fccdec9 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -965,7 +965,6 @@ set_name_rev () {
>>  #
>>  cmd_summary() {
>>  	summary_limit=-1
>> -	for_status=
>>  	diff_cmd=diff-index
>>  
>>  	# parse $args after "submodule ... summary".
>> @@ -978,9 +977,6 @@ cmd_summary() {
>>  		--files)
>>  			files="$1"
>>  			;;
>> -		--for-status)
>> -			for_status="$1"
>> -			;;
>>  		-n|--summary-limit)
>>  			summary_limit="$2"
>>  			isnumber "$summary_limit" || usage
Please drop the two hunks above ...
Show 41 quoted lines
>> @@ -1149,18 +1145,7 @@ cmd_summary() {
>>  			echo
>>  		fi
>>  		echo
>> -	done |
>> -	if test -n "$for_status"; then
>> -		if [ -n "$files" ]; then
>> -			gettextln "Submodules changed but not updated:" | git stripspace -c
>> -		else
>> -			gettextln "Submodule changes to be committed:" | git stripspace -c
>> -		fi
>> -		printf "\n" | git stripspace -c
>> -		git stripspace -c
>> -	else
>> -		cat
>> -	fi
>> +	done
>>  }
>>  #
>>  # List all submodules, prefixed with:
>> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
>> index ac2434c..b435d03 100755
>> --- a/t/t7401-submodule-summary.sh
>> +++ b/t/t7401-submodule-summary.sh
>> @@ -262,19 +262,6 @@ EOF
>>  	test_cmp expected actual
>>  "
>>  
>> -test_expect_success '--for-status' "
>> -	git submodule summary --for-status HEAD^ >actual &&
>> -	test_i18ncmp actual - <<EOF
>> -# Submodule changes to be committed:
>> -#
>> -# * sm1 $head6...0000000:
>> -#
>> -# * sm2 0000000...$head7 (2):
>> -#   > Add foo9
>> -#
>> -EOF
>> -"
>> -

... and just remove the "# " from the expected output here. This test can be removed when we use

Show 23 quoted lines
>>  test_expect_success 'fail when using --files together with --cached' "
>>  	test_must_fail git submodule summary --files --cached
>>  "
>> diff --git a/wt-status.c b/wt-status.c
>> index 958a53c..d91661d 100644
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -665,6 +665,10 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>>  	char index[PATH_MAX];
>>  	const char *env[] = { NULL, NULL };
>>  	struct argv_array argv = ARGV_ARRAY_INIT;
>> +	struct strbuf cmd_stdout = STRBUF_INIT;
>> +	struct strbuf summary = STRBUF_INIT;
>> +	char *summary_content;
>> +	size_t len;
>>  
>>  	sprintf(summary_limit, "%d", s->submodule_summary);
>>  	snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
>> @@ -673,7 +677,6 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>>  	argv_array_push(&argv, "submodule");
>>  	argv_array_push(&argv, "summary");
>>  	argv_array_push(&argv, uncommitted ? "--files" : "--cached");
>> -	argv_array_push(&argv, "--for-status");
And the line above has to stay.
Show 36 quoted lines
>>  	argv_array_push(&argv, "--summary-limit");
>>  	argv_array_push(&argv, summary_limit);
>>  	if (!uncommitted)
>> @@ -685,9 +688,31 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>>  	sm_summary.git_cmd = 1;
>>  	sm_summary.no_stdin = 1;
>>  	fflush(s->fp);
>> -	sm_summary.out = dup(fileno(s->fp));    /* run_command closes it */
>> +	sm_summary.out = -1;
>> +
>>  	run_command(&sm_summary);
>>  	argv_array_clear(&argv);
>> +
>> +	len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);
>> +
>> +	/* prepend header, only if there's an actual output */
>> +	if (len) {
>> +		if (uncommitted)
>> +			strbuf_addstr(&summary, _("Submodules changed but not updated:"));
>> +		else
>> +			strbuf_addstr(&summary, _("Submodule changes to be committed:"));
>> +		strbuf_addstr(&summary, "\n\n");
>> +	}
>> +	strbuf_addbuf(&summary, &cmd_stdout);
>> +	strbuf_release(&cmd_stdout);
>> +
>> +	summary_content = strbuf_detach(&summary, &len);
>> +	strbuf_add_commented_lines(&summary, summary_content, len);
>> +	free(summary_content);
>> +
>> +	summary_content = strbuf_detach(&summary, &len);
>> +	fprintf(s->fp, summary_content);
>> +	free(summary_content);
>>  }
>>  
>>  static void wt_status_print_other(struct wt_status *s,
Junio already commented on this part.
Junio C Hamano· Aug 29, 2013, 19:56 UTC · re: Matthieu Moy · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 18 quoted lines
> +	/* prepend header, only if there's an actual output */
> +	if (len) {
> +		if (uncommitted)
> +			strbuf_addstr(&summary, _("Submodules changed but not updated:"));
> +		else
> +			strbuf_addstr(&summary, _("Submodule changes to be committed:"));
> +		strbuf_addstr(&summary, "\n\n");
> +	}
> +	strbuf_addbuf(&summary, &cmd_stdout);
> +	strbuf_release(&cmd_stdout);
> +
> +	summary_content = strbuf_detach(&summary, &len);
> +	strbuf_add_commented_lines(&summary, summary_content, len);
> +	free(summary_content);
> +
> +	summary_content = strbuf_detach(&summary, &len);
> +	fprintf(s->fp, summary_content);
> +	free(summary_content);
This "fprintf()" looks bogus to me.  How about adding this on top?
 wt-status.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Show changes to wt-status.c +2 −3
diff --git a/wt-status.c b/wt-status.c
index d91661d..1f17652 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -710,9 +710,8 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	strbuf_add_commented_lines(&summary, summary_content, len);
 	free(summary_content);
 
-	summary_content = strbuf_detach(&summary, &len);
-	fprintf(s->fp, summary_content);
-	free(summary_content);
+	fputs(summary.buf, s->fp);
+	strbuf_release(&summary);
 }
 
 static void wt_status_print_other(struct wt_status *s,
Matthieu Moy· Aug 29, 2013, 21:05 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 3/4] get rid of "git submodule summary --for-status"

Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> +	/* prepend header, only if there's an actual output */
>> +	if (len) {
>> +		if (uncommitted)
>> +			strbuf_addstr(&summary, _("Submodules changed but not updated:"));
>> +		else
>> +			strbuf_addstr(&summary, _("Submodule changes to be committed:"));
>> +		strbuf_addstr(&summary, "\n\n");
>> +	}
>> +	strbuf_addbuf(&summary, &cmd_stdout);
>> +	strbuf_release(&cmd_stdout);
>> +
>> +	summary_content = strbuf_detach(&summary, &len);
>> +	strbuf_add_commented_lines(&summary, summary_content, len);
>> +	free(summary_content);
>> +
>> +	summary_content = strbuf_detach(&summary, &len);
>> +	fprintf(s->fp, summary_content);
>> +	free(summary_content);
>
> This "fprintf()" looks bogus to me.
Oops, indeed. I forgot the "%s".
> How about adding this on top?
Your solution is better, yes.
Show 19 quoted lines
>  wt-status.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index d91661d..1f17652 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -710,9 +710,8 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
>  	strbuf_add_commented_lines(&summary, summary_content, len);
>  	free(summary_content);
>  
> -	summary_content = strbuf_detach(&summary, &len);
> -	fprintf(s->fp, summary_content);
> -	free(summary_content);
> +	fputs(summary.buf, s->fp);
> +	strbuf_release(&summary);
>  }
>  
>  static void wt_status_print_other(struct wt_status *s,
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Matthieu Moy· Aug 29, 2013, 13:05 UTC · re: Matthieu Moy · lore

[PATCH v3 4/4] status: introduce status.displayCommentChar to disable display of #

Historically, "git status" needed to prefix each output line with '#' so that the output could be added as comment to the commit message. This prefix comment has no real purpose when "git status" is ran from the command-line, and this may distract users from the real content.

Allow the user to disable this prefix comment. In the long run, if users like the non-prefix output, it may make sense to flip the default value to true.

Obviously, status.displayCommentChar applies to "git status" but is ignored by "git commit", so the status is still commented in COMMIT_EDITMSG.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |  5 +++++
 builtin/commit.c         |  9 +++++++++
 t/t7508-status.sh        | 43 +++++++++++++++++++++++++++++++++++++++++++
 wt-status.c              | 35 +++++++++++++++++++++++++----------
 wt-status.h              |  1 +
 5 files changed, 83 insertions(+), 10 deletions(-)
Show changes to 5 files +83 −10

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..0afa531 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.
+
 status.showUntrackedFiles::
 	By default, linkgit:git-status[1] and linkgit:git-commit[1] show
 	files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..d4cfced 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1182,6 +1182,10 @@ static int git_status_config(const char *k, const char *v, void *cb)
 		s->use_color = git_config_colorbool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "status.displaycommentchar")) {
+		s->display_comment_char = git_config_bool(k, v);
+		return 0;
+	}
 	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
 		int slot = parse_status_slot(k, 13);
 		if (slot < 0)
@@ -1496,6 +1500,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	gitmodules_config();
 	git_config(git_commit_config, &s);
 	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
+	/*
+	 * Ignore status.displayCommentChar: we do need comments in
+	 * COMMIT_EDITMSG.
+	 */
+	s.display_comment_char = 1;
 	determine_whence(&s);
 	s.colopts = 0;
 
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index ac3d0fe..d0d7c4a 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -113,6 +113,39 @@ test_expect_success 'status (2)' '
 	test_i18ncmp expect output
 '
 
+strip_comments () {
+	sed "s/^\# //; s/^\#$//; s/^#\t/\t/" <"$1" >"$1".tmp &&
+	rm "$1" && mv "$1".tmp "$1"
+}
+
+test_expect_success 'status with status.displayCommentChar=false' '
+	strip_comments expect &&
+	git -c status.displayCommentChar=false status >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'setup fake editor' '
+	cat >.git/editor <<-\EOF &&
+	#! /bin/sh
+	cp "$1" output
+EOF
+	chmod 755 .git/editor
+'
+
+commit_template_commented () {
+	(
+		EDITOR=.git/editor &&
+		export EDITOR &&
+		# Fails due to empty message
+		test_must_fail git commit
+	) &&
+	! grep '^[^#]' output
+}
+
+test_expect_success 'commit ignores status.displayCommentChar=false in COMMIT_EDITMSG' '
+	commit_template_commented
+'
+
 cat >expect <<\EOF
 # On branch master
 # Changes to be committed:
@@ -872,6 +905,16 @@ test_expect_success 'status submodule summary' '
 	test_i18ncmp expect output
 '
 
+test_expect_success 'status submodule summary with status.displayCommentChar=false' '
+	strip_comments expect &&
+	git -c status.displayCommentChar=false status >output &&
+	test_i18ncmp expect output
+'
+
+test_expect_success 'commit with submodule summary ignores displayCommentChar' '
+	commit_template_commented
+'
+
 cat >expect <<EOF
  M dir1/modified
 A  dir2/added
diff --git a/wt-status.c b/wt-status.c
index d91661d..cdbcd6f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -46,9 +46,11 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
 
 	strbuf_vaddf(&sb, fmt, ap);
 	if (!sb.len) {
-		strbuf_addch(&sb, comment_line_char);
-		if (!trail)
-			strbuf_addch(&sb, ' ');
+		if (s->display_comment_char) {
+			strbuf_addch(&sb, comment_line_char);
+			if (!trail)
+				strbuf_addch(&sb, ' ');
+		}
 		color_print_strbuf(s->fp, color, &sb);
 		if (trail)
 			fprintf(s->fp, "%s", trail);
@@ -59,7 +61,7 @@ static void status_vprintf(struct wt_status *s, int at_bol, const char *color,
 		eol = strchr(line, '\n');
 
 		strbuf_reset(&linebuf);
-		if (at_bol) {
+		if (at_bol && s->display_comment_char) {
 			strbuf_addch(&linebuf, comment_line_char);
 			if (*line != '\n' && *line != '\t')
 				strbuf_addch(&linebuf, ' ');
@@ -129,6 +131,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->untracked.strdup_strings = 1;
 	s->ignored.strdup_strings = 1;
 	s->show_branch = -1;  /* unspecified */
+	s->display_comment_char = 1;
 }
 
 static void wt_status_print_unmerged_header(struct wt_status *s)
@@ -706,9 +709,11 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	strbuf_addbuf(&summary, &cmd_stdout);
 	strbuf_release(&cmd_stdout);
 
-	summary_content = strbuf_detach(&summary, &len);
-	strbuf_add_commented_lines(&summary, summary_content, len);
-	free(summary_content);
+	if (s->display_comment_char) {
+		summary_content = strbuf_detach(&summary, &len);
+		strbuf_add_commented_lines(&summary, summary_content, len);
+		free(summary_content);
+	}
 
 	summary_content = strbuf_detach(&summary, &len);
 	fprintf(s->fp, summary_content);
@@ -793,6 +798,8 @@ static void wt_status_print_tracking(struct wt_status *s)
 	struct strbuf sb = STRBUF_INIT;
 	const char *cp, *ep;
 	struct branch *branch;
+	char comment_line_string[3];
+	int i;
 
 	assert(s->branch && !s->is_initial);
 	if (prefixcmp(s->branch, "refs/heads/"))
@@ -801,12 +808,20 @@ static void wt_status_print_tracking(struct wt_status *s)
 	if (!format_tracking_info(branch, &sb))
 		return;
 
+	i = 0;
+	if (s->display_comment_char) {
+		comment_line_string[i++] = comment_line_char;
+		comment_line_string[i++] = ' ';
+	}
+	comment_line_string[i] = '\0';
+
 	for (cp = sb.buf; (ep = strchr(cp, '\n')) != NULL; cp = ep + 1)
 		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s),
-				 "%c %.*s", comment_line_char,
+				 "%s%.*s", comment_line_string,
 				 (int)(ep - cp), cp);
-	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
-			 comment_line_char);
+	if (s->display_comment_char)
+		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
+				 comment_line_char);
 }
 
 static int has_unmerged(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index fb7152e..ac02c64 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -50,6 +50,7 @@ struct wt_status {
 	enum commit_whence whence;
 	int nowarn;
 	int use_color;
+	int display_comment_char;
 	int relative_paths;
 	int submodule_summary;
 	int show_ignored_files;
-- 
1.8.4.12.gf9d53a3.dirty
Junio C Hamano· Aug 29, 2013, 16:19 UTC · re: Matthieu Moy · lore

Re: [RFC/PATCH v3 0/4] Disable "git status" comment prefix

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> ... Right now, my preference would be to call it
> status.oldStyle and default it to false (i.e. change the behavior, but
> allow old-timers to get back the old one).
Sounds sensible, at least to me.

← back to recent threads