Re: [PATCH 3/4] status: update short status to use --no-ahead-behind

From: Jeff King <peff@peff.net>
Date: Dec 20, 2017, 16:26 UTC
Message-ID: <20171220162637.GC31149@sigill.intra.peff.net>
In-Reply-To: <20171220144245.39401-4-git@jeffhostetler.com>

On Wed, Dec 20, 2017 at 02:42:44PM +0000, Jeff Hostetler wrote:

Show 5 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Teach "git status --short --branch" to use "--no-ahead-behind"
> flag to skip computing ahead/behind counts for the branch and
> its upstream and just report '[different]'.

How come "--short" and "--long" get this smaller bit of data, but "--porcelain=v2" just omits the line entirely?

I don't have a real preference for or against the "[different]" message myself, but if we can get the information cheaply, it seems odd not to provide it in all cases.

Show 11 quoted lines

> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 6ce8cf8..ea029ad 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -117,6 +117,9 @@ configuration variable documented in linkgit:git-config[1].
>  	expensive computation on extremely large repositories.
>  +
>  	In porcelain V2 format, the 'branch.ab' line will not be present.
> ++
> +	In short format with --branch, '[different]' will printed rather
> +	than detailed ahead/behind counts.

s/will/will be/ ?

Show 14 quoted lines

> diff --git a/remote.c b/remote.c
> index a38b42e..0a63ac1 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1978,9 +1978,12 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid)
>  
>  /*
>   * Compare a branch with its upstream, and save their differences (number
> - * of commits) in *num_ours and *num_theirs. The name of the upstream branch
> - * (or NULL if no upstream is defined) is returned via *upstream_name, if it
> - * is not itself NULL.
> + * of commits) in *num_ours and *num_theirs.  If either num_ours or num_theirs
> + * are NULL, we skip counting the commits and just return whether they are
> + * different.

OK, this makes sense. I wondered in the last one why the caller could not simply check "num_ours != num_theirs" themselves. And this is why: we want to be able to signal to stat_tracking_info() that we want the "cheap" version.

Show 12 quoted lines

> diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
> index 8f17fd9..0190220 100755
> --- a/t/t6040-tracking-info.sh
> +++ b/t/t6040-tracking-info.sh
> @@ -147,6 +147,19 @@ test_expect_success 'status -s -b (diverged from upstream)' '
>  '
>  
>  cat >expect <<\EOF
> +## b1...origin/master [different]
> +EOF
> +
> +test_expect_success 'status -s -b --no-ahead-behind (diverged from upstream)' '

This patch will affect "git status --porcelain", too. That's not supposed to change in incompatible ways. I guess it's up for debate whether callers are meant to handle any arbitrary string inside the [] (we already show "[gone]" for some cases), since AFAICT the format of the tracking info is left completely vague in the documentation.

(I'd also hope that everybody is using --porcelain=v2 if they can, but we should still avoid breaking v1).

-Peff