git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/3] git-status: create config for ahead/behind calculation
@ 2019-06-18 20:21 Derrick Stolee via GitGitGadget
  2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Derrick Stolee via GitGitGadget @ 2019-06-18 20:21 UTC (permalink / raw)
  To: git; +Cc: git, Junio C Hamano

The git status builtin includes a calculation for how far the local ref is
ahead or behind the remote tracking branch. This check can be expensive, so
we already have a --[no-]ahead-behind command-line option.

This series adds two bits of functionality to the feature:

 1. Add a new status.aheadBehind config setting.
    
    
 2. Add a new advice.statusAheadBehind config setting associated with a
    warning that suggests status.aheadBehind when the calculation takes a
    long time.
    
    

We have been running with these commits in microsoft/git for a while now.
The only change I made in adapting Jeff's commits was to add the advice
config documentation.

The status.aheadBehind config setting is a candidate to be added to the
proposed "large repo" meta-config setting previously discussed [1]. I'm
putting this out for independent review as it is much smaller compared to
the potential of that series.

Thanks, -Stolee

[1] https://public-inbox.org/git/pull.254.git.gitgitgadget@gmail.com/

Jeff Hostetler (3):
  status: add status.aheadbehind setting
  status: warn when a/b calculation takes too long
  status: ignore status.aheadbehind in porcelain formats

 Documentation/config/advice.txt |  6 ++++++
 Documentation/config/status.txt |  5 +++++
 advice.c                        |  2 ++
 advice.h                        |  1 +
 builtin/commit.c                | 19 ++++++++++++++++++-
 t/t6040-tracking-info.sh        | 31 +++++++++++++++++++++++++++++++
 t/t7064-wtstatus-pv2.sh         |  8 ++++++++
 wt-status.c                     | 17 +++++++++++++++++
 8 files changed, 88 insertions(+), 1 deletion(-)


base-commit: b697d92f56511e804b8ba20ccbe7bdc85dc66810
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-272%2Fderrickstolee%2Fstatus-ahead-behind-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-272/derrickstolee/status-ahead-behind-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/272
-- 
gitgitgadget

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

* [PATCH 1/3] status: add status.aheadbehind setting
  2019-06-18 20:21 [PATCH 0/3] git-status: create config for ahead/behind calculation Derrick Stolee via GitGitGadget
@ 2019-06-18 20:21 ` Jeff Hostetler via GitGitGadget
  2019-06-21 16:33   ` Junio C Hamano
  2019-06-29 17:30   ` SZEDER Gábor
  2019-06-18 20:21 ` [PATCH 2/3] status: warn when a/b calculation takes too long Jeff Hostetler via GitGitGadget
  2019-06-18 20:21 ` [PATCH 3/3] status: ignore status.aheadbehind in porcelain formats Jeff Hostetler via GitGitGadget
  2 siblings, 2 replies; 7+ messages in thread
From: Jeff Hostetler via GitGitGadget @ 2019-06-18 20:21 UTC (permalink / raw)
  To: git; +Cc: git, Junio C Hamano, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

The --[no-]ahead-behind option was introduced in fd9b544a
(status: add --[no-]ahead-behind to status and commit for V2
format, 2018-01-09). This is a necessary change of behavior
in repos where the remote tracking branches can move very
quickly ahead of the local branches. However, users need to
remember to provide the command-line argument every time.

Add a new "status.aheadBehind" config setting to change the
default behavior of all git status formats.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/status.txt |  5 +++++
 builtin/commit.c                | 17 ++++++++++++++++-
 t/t6040-tracking-info.sh        | 31 +++++++++++++++++++++++++++++++
 t/t7064-wtstatus-pv2.sh         |  4 ++++
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
index ed72fa7dae..0fc704ab80 100644
--- a/Documentation/config/status.txt
+++ b/Documentation/config/status.txt
@@ -12,6 +12,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.aheadBehind::
+	Set to true to enable `--ahead-behind` and false to enable
+	`--no-ahead-behind` by default in linkgit:git-status[1] for
+	non-porcelain status formats.  Defaults to true.
+
 status.displayCommentPrefix::
 	If set to true, linkgit:git-status[1] will insert a comment
 	prefix before each output line (starting with
diff --git a/builtin/commit.c b/builtin/commit.c
index 1c9e8e2228..71305073ad 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1078,9 +1078,11 @@ static const char *read_commit_message(const char *name)
 static struct status_deferred_config {
 	enum wt_status_format status_format;
 	int show_branch;
+	enum ahead_behind_flags ahead_behind;
 } status_deferred_config = {
 	STATUS_FORMAT_UNSPECIFIED,
-	-1 /* unspecified */
+	-1, /* unspecified */
+	AHEAD_BEHIND_UNSPECIFIED,
 };
 
 static void finalize_deferred_config(struct wt_status *s)
@@ -1107,6 +1109,15 @@ static void finalize_deferred_config(struct wt_status *s)
 	if (s->show_branch < 0)
 		s->show_branch = 0;
 
+	/*
+	 * If the user did not give a "--[no]-ahead-behind" command
+	 * line argument, then we inherit the a/b config setting.
+	 * If is not set, then we inherit _FULL for backwards
+	 * compatibility.
+	 */
+	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+		s->ahead_behind_flags = status_deferred_config.ahead_behind;
+
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
 }
@@ -1246,6 +1257,10 @@ static int git_status_config(const char *k, const char *v, void *cb)
 		status_deferred_config.show_branch = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "status.aheadbehind")) {
+		status_deferred_config.ahead_behind = git_config_bool(k, v);
+		return 0;
+	}
 	if (!strcmp(k, "status.showstash")) {
 		s->show_stash = git_config_bool(k, v);
 		return 0;
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 716283b274..febf63f28a 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -159,6 +159,19 @@ test_expect_success 'status -s -b --no-ahead-behind (diverged from upstream)' '
 	test_i18ncmp expect actual
 '
 
+cat >expect <<\EOF
+## b1...origin/master [different]
+EOF
+
+test_expect_success 'status.aheadbehind=false status -s -b (diverged from upstream)' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=false status -s -b | head -1
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 On branch b1
 Your branch and 'origin/master' have diverged,
@@ -174,6 +187,15 @@ test_expect_success 'status --long --branch' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'status --long --branch' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=true status --long -b | head -3
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 On branch b1
 Your branch and 'origin/master' refer to different commits.
@@ -188,6 +210,15 @@ test_expect_success 'status --long --branch --no-ahead-behind' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'status.aheadbehind=false status --long --branch' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=false status --long -b | head -2
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 ## b5...brokenbase [gone]
 EOF
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 11eccc231a..a0baf6e8b0 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -436,6 +436,10 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		git status --no-ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual &&
 
+		# Confirmat that "status.aheadbehind" works on V2 format.
+		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
 		# Confirm --ahead-behind reports traditional branch.ab with 1/0.
 		cat >expect <<-EOF &&
 		# branch.oid $HUF
-- 
gitgitgadget


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

* [PATCH 2/3] status: warn when a/b calculation takes too long
  2019-06-18 20:21 [PATCH 0/3] git-status: create config for ahead/behind calculation Derrick Stolee via GitGitGadget
  2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
@ 2019-06-18 20:21 ` Jeff Hostetler via GitGitGadget
  2019-06-18 20:21 ` [PATCH 3/3] status: ignore status.aheadbehind in porcelain formats Jeff Hostetler via GitGitGadget
  2 siblings, 0 replies; 7+ messages in thread
From: Jeff Hostetler via GitGitGadget @ 2019-06-18 20:21 UTC (permalink / raw)
  To: git; +Cc: git, Junio C Hamano, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

The ahead/behind calculation in 'git status' can be slow in some
cases. Users may not realize that there are ways to avoid this
computation, especially if they are not using the information.

Add a warning that appears if this calculation takes more than
two seconds. The warning can be disabled through the new config
setting advice.statusAheadBehind.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/advice.txt |  6 ++++++
 advice.c                        |  2 ++
 advice.h                        |  1 +
 wt-status.c                     | 17 +++++++++++++++++
 4 files changed, 26 insertions(+)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index ec4f6ae658..4bc5b4c742 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -37,6 +37,12 @@ advice.*::
 		we can still suggest that the user push to either
 		refs/heads/* or refs/tags/* based on the type of the
 		source object.
+	statusAheadBehind::
+		Shown when linkgit:git-status[1] computes the ahead/behind
+		counts for a local ref compared to its remote tracking ref,
+		and that calculation takes longer than expected. Will not
+		appear if `status.aheadBehind` is false or the option
+		`--no-ahead-behind` is given.
 	statusHints::
 		Show directions on how to proceed from the current
 		state in the output of linkgit:git-status[1], in
diff --git a/advice.c b/advice.c
index ce5f374ecd..54f8dea30c 100644
--- a/advice.c
+++ b/advice.c
@@ -12,6 +12,7 @@ int advice_push_needs_force = 1;
 int advice_push_unqualified_ref_name = 1;
 int advice_status_hints = 1;
 int advice_status_u_option = 1;
+int advice_status_ahead_behind_warning = 1;
 int advice_commit_before_merge = 1;
 int advice_reset_quiet_warning = 1;
 int advice_resolve_conflict = 1;
@@ -68,6 +69,7 @@ static struct {
 	{ "pushUnqualifiedRefName", &advice_push_unqualified_ref_name },
 	{ "statusHints", &advice_status_hints },
 	{ "statusUoption", &advice_status_u_option },
+	{ "statusAheadBehindWarning", &advice_status_ahead_behind_warning },
 	{ "commitBeforeMerge", &advice_commit_before_merge },
 	{ "resetQuiet", &advice_reset_quiet_warning },
 	{ "resolveConflict", &advice_resolve_conflict },
diff --git a/advice.h b/advice.h
index e50f02cdfe..c86de9b9b8 100644
--- a/advice.h
+++ b/advice.h
@@ -12,6 +12,7 @@ extern int advice_push_needs_force;
 extern int advice_push_unqualified_ref_name;
 extern int advice_status_hints;
 extern int advice_status_u_option;
+extern int advice_status_ahead_behind_warning;
 extern int advice_commit_before_merge;
 extern int advice_reset_quiet_warning;
 extern int advice_resolve_conflict;
diff --git a/wt-status.c b/wt-status.c
index d2a1bec226..c94d43879a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -19,6 +19,8 @@
 #include "lockfile.h"
 #include "sequencer.h"
 
+#define AB_DELAY_WARNING_IN_MS (2 * 1000)
+
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";
 
@@ -1085,14 +1087,29 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	struct branch *branch;
 	char comment_line_string[3];
 	int i;
+	uint64_t t_begin = 0;
 
 	assert(s->branch && !s->is_initial);
 	if (!skip_prefix(s->branch, "refs/heads/", &branch_name))
 		return;
 	branch = branch_get(branch_name);
+
+	t_begin = getnanotime();
+
 	if (!format_tracking_info(branch, &sb, s->ahead_behind_flags))
 		return;
 
+	if (advice_status_ahead_behind_warning &&
+	    s->ahead_behind_flags == AHEAD_BEHIND_FULL) {
+		uint64_t t_delta_in_ms = (getnanotime() - t_begin) / 1000000;
+		if (t_delta_in_ms > AB_DELAY_WARNING_IN_MS) {
+			strbuf_addf(&sb, _("\n"
+					   "It took %.2f seconds to compute the branch ahead/behind values.\n"
+					   "You can use '--no-ahead-behind' to avoid this.\n"),
+				    t_delta_in_ms / 1000.0);
+		}
+	}
+
 	i = 0;
 	if (s->display_comment_prefix) {
 		comment_line_string[i++] = comment_line_char;
-- 
gitgitgadget


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

* [PATCH 3/3] status: ignore status.aheadbehind in porcelain formats
  2019-06-18 20:21 [PATCH 0/3] git-status: create config for ahead/behind calculation Derrick Stolee via GitGitGadget
  2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
  2019-06-18 20:21 ` [PATCH 2/3] status: warn when a/b calculation takes too long Jeff Hostetler via GitGitGadget
@ 2019-06-18 20:21 ` Jeff Hostetler via GitGitGadget
  2 siblings, 0 replies; 7+ messages in thread
From: Jeff Hostetler via GitGitGadget @ 2019-06-18 20:21 UTC (permalink / raw)
  To: git; +Cc: git, Junio C Hamano, Jeff Hostetler

From: Jeff Hostetler <jeffhost@microsoft.com>

Teach porcelain V[12] formats to ignore the status.aheadbehind
config setting. They only respect the --[no-]ahead-behind
command line argument.  This is for backwards compatibility
with existing scripts.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/commit.c        | 10 ++++++----
 t/t7064-wtstatus-pv2.sh | 12 ++++++++----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 71305073ad..79cb238d87 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1111,11 +1111,13 @@ static void finalize_deferred_config(struct wt_status *s)
 
 	/*
 	 * If the user did not give a "--[no]-ahead-behind" command
-	 * line argument, then we inherit the a/b config setting.
-	 * If is not set, then we inherit _FULL for backwards
-	 * compatibility.
+	 * line argument *AND* we will print in a human-readable format
+	 * (short, long etc.) then we inherit from the status.aheadbehind
+	 * config setting.  In all other cases (and porcelain V[12] formats
+	 * in particular), we inherit _FULL for backwards compatibility.
 	 */
-	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+	if (use_deferred_config &&
+	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = status_deferred_config.ahead_behind;
 
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index a0baf6e8b0..537787e598 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -436,10 +436,6 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		git status --no-ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual &&
 
-		# Confirmat that "status.aheadbehind" works on V2 format.
-		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
-		test_cmp expect actual &&
-
 		# Confirm --ahead-behind reports traditional branch.ab with 1/0.
 		cat >expect <<-EOF &&
 		# branch.oid $HUF
@@ -449,6 +445,14 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		EOF
 
 		git status --ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		# Confirm that "status.aheadbehind" DOES NOT work on V2 format.
+		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		# Confirm that "status.aheadbehind" DOES NOT work on V2 format.
+		git -c status.aheadbehind=true status --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual
 	)
 '
-- 
gitgitgadget

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

* Re: [PATCH 1/3] status: add status.aheadbehind setting
  2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
@ 2019-06-21 16:33   ` Junio C Hamano
  2019-06-26 20:02     ` Jeff Hostetler
  2019-06-29 17:30   ` SZEDER Gábor
  1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2019-06-21 16:33 UTC (permalink / raw)
  To: Jeff Hostetler via GitGitGadget; +Cc: git, git, Jeff Hostetler

"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> The --[no-]ahead-behind option was introduced in fd9b544a
> (status: add --[no-]ahead-behind to status and commit for V2
> format, 2018-01-09). This is a necessary change of behavior
> in repos where the remote tracking branches can move very
> quickly ahead of the local branches. However, users need to
> remember to provide the command-line argument every time.
>
> Add a new "status.aheadBehind" config setting to change the
> default behavior of all git status formats.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  Documentation/config/status.txt |  5 +++++
>  builtin/commit.c                | 17 ++++++++++++++++-
>  t/t6040-tracking-info.sh        | 31 +++++++++++++++++++++++++++++++
>  t/t7064-wtstatus-pv2.sh         |  4 ++++
>  4 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
> index ed72fa7dae..0fc704ab80 100644
> --- a/Documentation/config/status.txt
> +++ b/Documentation/config/status.txt
> @@ -12,6 +12,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.aheadBehind::
> +	Set to true to enable `--ahead-behind` and false to enable
> +	`--no-ahead-behind` by default in linkgit:git-status[1] for
> +	non-porcelain status formats.  Defaults to true.

Sensible.

> @@ -1078,9 +1078,11 @@ static const char *read_commit_message(const char *name)
>  static struct status_deferred_config {
>  	enum wt_status_format status_format;
>  	int show_branch;
> +	enum ahead_behind_flags ahead_behind;
>  } status_deferred_config = {
>  	STATUS_FORMAT_UNSPECIFIED,
> -	-1 /* unspecified */
> +	-1, /* unspecified */
> +	AHEAD_BEHIND_UNSPECIFIED,

This obviously is not a problem introduced by this patch, but is
there a plan to extend this beyond a boolean?

Otherwise, a separate enum is way overkill.  Naming the field so
that it is clear it is either true or false (e.g.  perhaps call it
"ahead_behind_detailed" as the current "QUICK" is merely "are they
equal?" which corresponds to "false", and "FULL" is to show the
detailed info), and then use the usual "-1 is unspecified, 0 and 1
are usual bools" convention would be more appropriate.


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

* Re: [PATCH 1/3] status: add status.aheadbehind setting
  2019-06-21 16:33   ` Junio C Hamano
@ 2019-06-26 20:02     ` Jeff Hostetler
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Hostetler @ 2019-06-26 20:02 UTC (permalink / raw)
  To: Junio C Hamano, Jeff Hostetler via GitGitGadget; +Cc: git, git, Jeff Hostetler



On 6/21/2019 12:33 PM, Junio C Hamano wrote:
> "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> The --[no-]ahead-behind option was introduced in fd9b544a
>> (status: add --[no-]ahead-behind to status and commit for V2
>> format, 2018-01-09). This is a necessary change of behavior
>> in repos where the remote tracking branches can move very
>> quickly ahead of the local branches. However, users need to
>> remember to provide the command-line argument every time.
>>
>> Add a new "status.aheadBehind" config setting to change the
>> default behavior of all git status formats.
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
>>   Documentation/config/status.txt |  5 +++++
>>   builtin/commit.c                | 17 ++++++++++++++++-
>>   t/t6040-tracking-info.sh        | 31 +++++++++++++++++++++++++++++++
>>   t/t7064-wtstatus-pv2.sh         |  4 ++++
>>   4 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
>> index ed72fa7dae..0fc704ab80 100644
>> --- a/Documentation/config/status.txt
>> +++ b/Documentation/config/status.txt
>> @@ -12,6 +12,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.aheadBehind::
>> +	Set to true to enable `--ahead-behind` and false to enable
>> +	`--no-ahead-behind` by default in linkgit:git-status[1] for
>> +	non-porcelain status formats.  Defaults to true.
> 
> Sensible.
> 
>> @@ -1078,9 +1078,11 @@ static const char *read_commit_message(const char *name)
>>   static struct status_deferred_config {
>>   	enum wt_status_format status_format;
>>   	int show_branch;
>> +	enum ahead_behind_flags ahead_behind;
>>   } status_deferred_config = {
>>   	STATUS_FORMAT_UNSPECIFIED,
>> -	-1 /* unspecified */
>> +	-1, /* unspecified */
>> +	AHEAD_BEHIND_UNSPECIFIED,
> 
> This obviously is not a problem introduced by this patch, but is
> there a plan to extend this beyond a boolean?
> 
> Otherwise, a separate enum is way overkill.  Naming the field so
> that it is clear it is either true or false (e.g.  perhaps call it
> "ahead_behind_detailed" as the current "QUICK" is merely "are they
> equal?" which corresponds to "false", and "FULL" is to show the
> detailed info), and then use the usual "-1 is unspecified, 0 and 1
> are usual bools" convention would be more appropriate.
> 

At one point[1] we talked about having an intermediate option
to try N or less commits before giving up.  The enum would let
us add that case if we wanted it.

So far, I've not heard any complaints with just having QUICK or FULL
from our Windows developers, so adding a 3rd mode hasn't been a
priority.  But the enum is here if we do decide to do so.

Jeff

[1] 
https://public-inbox.org/git/20180111093943.GC9190@sigill.intra.peff.net/

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

* Re: [PATCH 1/3] status: add status.aheadbehind setting
  2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
  2019-06-21 16:33   ` Junio C Hamano
@ 2019-06-29 17:30   ` SZEDER Gábor
  1 sibling, 0 replies; 7+ messages in thread
From: SZEDER Gábor @ 2019-06-29 17:30 UTC (permalink / raw)
  To: Jeff Hostetler via GitGitGadget; +Cc: git, git, Junio C Hamano, Jeff Hostetler

On Tue, Jun 18, 2019 at 01:21:25PM -0700, Jeff Hostetler via GitGitGadget wrote:
> diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
> index 716283b274..febf63f28a 100755
> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -159,6 +159,19 @@ test_expect_success 'status -s -b --no-ahead-behind (diverged from upstream)' '
>  	test_i18ncmp expect actual
>  '
>  
> +cat >expect <<\EOF
> +## b1...origin/master [different]
> +EOF
> +
> +test_expect_success 'status.aheadbehind=false status -s -b (diverged from upstream)' '
> +	(
> +		cd test &&
> +		git checkout b1 >/dev/null &&
> +		git -c status.aheadbehind=false status -s -b | head -1

These tests specifically check 'git status', but the pipe hides its
exit code.  Please use an intermediate file instead.

> +	) >actual &&

I found it odd to save the output of a whole subshell and redirect
'git checkout's stdout to /dev/null to prevent it from itnerfering
with the stdout of the subshell, instead of saving only the stdout of
the command the test focuses on.

> +	test_i18ncmp expect actual
> +'
> +
>  cat >expect <<\EOF
>  On branch b1
>  Your branch and 'origin/master' have diverged,
> @@ -174,6 +187,15 @@ test_expect_success 'status --long --branch' '
>  	test_i18ncmp expect actual
>  '
>  
> +test_expect_success 'status --long --branch' '
> +	(
> +		cd test &&
> +		git checkout b1 >/dev/null &&
> +		git -c status.aheadbehind=true status --long -b | head -3
> +	) >actual &&
> +	test_i18ncmp expect actual
> +'
> +
>  cat >expect <<\EOF
>  On branch b1
>  Your branch and 'origin/master' refer to different commits.
> @@ -188,6 +210,15 @@ test_expect_success 'status --long --branch --no-ahead-behind' '
>  	test_i18ncmp expect actual
>  '
>  
> +test_expect_success 'status.aheadbehind=false status --long --branch' '
> +	(
> +		cd test &&
> +		git checkout b1 >/dev/null &&
> +		git -c status.aheadbehind=false status --long -b | head -2
> +	) >actual &&
> +	test_i18ncmp expect actual
> +'
> +
>  cat >expect <<\EOF
>  ## b5...brokenbase [gone]
>  EOF

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

end of thread, other threads:[~2019-06-29 17:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 20:21 [PATCH 0/3] git-status: create config for ahead/behind calculation Derrick Stolee via GitGitGadget
2019-06-18 20:21 ` [PATCH 1/3] status: add status.aheadbehind setting Jeff Hostetler via GitGitGadget
2019-06-21 16:33   ` Junio C Hamano
2019-06-26 20:02     ` Jeff Hostetler
2019-06-29 17:30   ` SZEDER Gábor
2019-06-18 20:21 ` [PATCH 2/3] status: warn when a/b calculation takes too long Jeff Hostetler via GitGitGadget
2019-06-18 20:21 ` [PATCH 3/3] status: ignore status.aheadbehind in porcelain formats Jeff Hostetler via GitGitGadget

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

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).