git-status: create config for ahead/behind calculation #272
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The --[no-]ahead-behind option was introduced in fd9b544 (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]>
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 <[email protected]> Signed-off-by: Derrick Stolee <[email protected]>
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 <[email protected]> Signed-off-by: Derrick Stolee <[email protected]>
derrickstolee
force-pushed
the
status-ahead-behind
branch
from
65466eb to
feea3f6
Compare
|
/submit |
|
Submitted as [email protected] |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -12,6 +12,11 @@ status.branch:: | |||
| Set to true to enable --branch by default in linkgit:git-status[1]. | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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]/
|
This branch is now known as |
|
This patch series was integrated into pu via git@f5ece82. |
|
This patch series was integrated into pu via git@efd4549. |
|
This patch series was integrated into pu via git@8995472. |
|
This patch series was integrated into pu via git@d098601. |
|
This patch series was integrated into pu via git@5cf1056. |
|
This patch series was integrated into pu via git@e9a9096. |
|
This patch series was integrated into next via git@362ee6b. |
|
This patch series was integrated into pu via git@fd439ed. |
| @@ -12,6 +12,11 @@ status.branch:: | |||
| Set to true to enable --branch by default in linkgit:git-status[1]. | |||
There was a problem hiding this comment.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
git statusbuiltin 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-behindcommand-line option.This series adds two bits of functionality to the feature:
Add a new
status.aheadBehindconfig setting.Add a new
advice.statusAheadBehindconfig setting associated with a warning that suggestsstatus.aheadBehindwhen 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.aheadBehindconfig 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/[email protected]/
Cc: git@jeffhostetler