Re: [PATCH v3 4/6] for-each-ref: introduce %(upstream:track[short])

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Nov 15, 2013 at 5:59 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Introduce %(upstream:track) to display "[ahead M, behind N]" and
> %(upstream:trackshort) to display "=", ">", "<", or "<>"
> appropriately (inspired by contrib/completion/git-prompt.sh).
>
> Now you can use the following format in for-each-ref:
>
>   %(refname:short)%(upstream:trackshort)
>
> to display refs with terse tracking information.
>
> Note that :track and :trackshort only work with "upstream", and error
> out when used with anything else.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-for-each-ref.txt |  6 +++++-
>  builtin/for-each-ref.c             | 40 +++++++++++++++++++++++++++++++++++---
>  t/t6300-for-each-ref.sh            | 22 +++++++++++++++++++++
>  3 files changed, 64 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index ab3da0e..c9b192e 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -91,7 +91,11 @@ objectname::
>  upstream::
>         The name of a local ref which can be considered ``upstream''
>         from the displayed ref. Respects `:short` in the same way as
> -       `refname` above.
> +       `refname` above.  Additionally respects `:track` to show
> +       "[ahead N, behind M]" and `:trackshort` to show the terse
> +       version (like the prompt) ">", "<", "<>", or "=".  Has no

The "prompt" is not mentioned elsewhere in for-each-ref documentation,
and a person not familiar with contrib/completion/ may be confused by
this reference. It might make sense instead to explain the meanings of
">", "<", "<>", and "=" directly since they are not necessarily
obvious to the casual reader.

> +       effect if the ref does not have tracking information
> +       associated with it.
>
>  HEAD::
>         Used to indicate the currently checked out branch.  Is '*' if
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 5f1842f..ed81407 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -689,13 +690,46 @@ static void populate_value(struct refinfo *ref)
>                         continue;
>
>                 formatp = strchr(name, ':');
> -               /* look for "short" refname format */
>                 if (formatp) {
> +                       int num_ours, num_theirs;
> +
>                         formatp++;
>                         if (!strcmp(formatp, "short"))
>                                 refname = shorten_unambiguous_ref(refname,
>                                                       warn_ambiguous_refs);
> -                       else
> +                       else if (!strcmp(formatp, "track") &&
> +                               !prefixcmp(name, "upstream")) {
> +                               char buf[40];
> +
> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
> +                               if (!num_ours && !num_theirs)
> +                                       v->s = "";
> +                               else if (!num_ours) {
> +                                       sprintf(buf, "[behind %d]", num_theirs);
> +                                       v->s = xstrdup(buf);
> +                               } else if (!num_theirs) {
> +                                       sprintf(buf, "[ahead %d]", num_ours);
> +                                       v->s = xstrdup(buf);
> +                               } else {
> +                                       sprintf(buf, "[ahead %d, behind %d]",

Is the intention that these strings ("[ahead %d]", etc.) will be
internationalized in the future? If so, the allocated 40-character
buffer may be insufficient.

> +                                               num_ours, num_theirs);
> +                                       v->s = xstrdup(buf);
> +                               }
> +                               continue;
> +                       } else if (!strcmp(formatp, "trackshort") &&
> +                               !prefixcmp(name, "upstream")) {
> +
> +                               stat_tracking_info(branch, &num_ours, &num_theirs);
> +                               if (!num_ours && !num_theirs)
> +                                       v->s = "=";
> +                               else if (!num_ours)
> +                                       v->s = "<";
> +                               else if (!num_theirs)
> +                                       v->s = ">";
> +                               else
> +                                       v->s = "<>";
> +                               continue;
> +                       } else
>                                 die("unknown %.*s format %s",
>                                     (int)(formatp - name), name, formatp);

Is it still accurate to call this a "format" in the error message?
'track' and 'trackshort' seem more like decorations.

>                 }
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 5e29ffc..9d874fd 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -303,6 +303,28 @@ test_expect_success 'Check short upstream format' '
>         test_cmp expected actual
>  '
>
> +test_expect_success 'setup for upstream:track[short]' '
> +       test_commit two
> +'
> +
> +cat >expected <<EOF
> +[ahead 1]
> +EOF
> +
> +test_expect_success 'Check upstream:track format' '
> +       git for-each-ref --format="%(upstream:track)" refs/heads >actual &&
> +       test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> +>
> +EOF
> +
> +test_expect_success 'Check upstream:trackshort format' '
> +       git for-each-ref --format="%(upstream:trackshort)" refs/heads >actual &&
> +       test_cmp expected actual
> +'
> +
>  cat >expected <<EOF
>  $(git rev-parse --short HEAD)
>  EOF

Would it make sense also to add tests verifying that :track and
:trackshort correctly fail when applied to a key other than
"upstream"?