Skip to content

git-status: create config for ahead/behind calculation #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

git-status: create config for ahead/behind calculation #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f0f2a2
status: add status.aheadbehind setting
jeffhostetler Jan 9, 2018
c105654
status: warn when a/b calculation takes too long
jeffhostetler Jan 10, 2018
feea3f6
status: ignore status.aheadbehind in porcelain formats
jeffhostetler Feb 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Documentation/config/advice.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions Documentation/config/status.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Jeff Hostetler via GitGitGadget" <[email protected]> writes:

> From: Jeff Hostetler <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Derrick Stolee <[email protected]>
> ---
>  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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jeff Hostetler wrote (reply to this):



On 6/21/2019 12:33 PM, Junio C Hamano wrote:
> "Jeff Hostetler via GitGitGadget" <[email protected]> writes:
> 
>> From: Jeff Hostetler <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Derrick Stolee <[email protected]>
>> ---
>>   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/[email protected]/

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, SZEDER Gábor wrote (reply to this):

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

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
Expand Down
2 changes: 2 additions & 0 deletions advice.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 },
Expand Down
1 change: 1 addition & 0 deletions advice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 18 additions & 1 deletion builtin/commit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -1107,6 +1109,17 @@ 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 *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 (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)
s->ahead_behind_flags = AHEAD_BEHIND_FULL;
}
Expand Down Expand Up @@ -1246,6 +1259,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;
Expand Down
31 changes: 31 additions & 0 deletions t/t6040-tracking-info.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions t/t7064-wtstatus-pv2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,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
)
'
Expand Down
17 changes: 17 additions & 0 deletions wt-status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand Down