Skip to content

Commit

Permalink
tag: do not show ambiguous tag names as "tags/foo"
Browse files Browse the repository at this point in the history
Since b7cc53e (tag.c: use 'ref-filter' APIs, 2015-07-11),
git-tag has started showing tags with ambiguous names (i.e.,
when both "heads/foo" and "tags/foo" exists) as "tags/foo"
instead of just "foo". This is both:

  - pointless; the output of "git tag" includes only
    refs/tags, so we know that "foo" means the one in
    "refs/tags".

and

  - ambiguous; in the original output, we know that the line
    "foo" means that "refs/tags/foo" exists. In the new
    output, it is unclear whether we mean "refs/tags/foo" or
    "refs/tags/tags/foo".

The reason this happens is that commit b7cc53e switched
git-tag to use ref-filter's "%(refname:short)" output
formatting, which was adapted from for-each-ref. This more
general code does not know that we care only about tags, and
uses shorten_unambiguous_ref to get the short-name. We need
to tell it that we care only about "refs/tags/", and it
should shorten with respect to that value.

In theory, the ref-filter code could figure this out by us
passing FILTER_REFS_TAGS. But there are two complications
there:

  1. The handling of refname:short is deep in formatting
     code that does not even have our ref_filter struct, let
     alone the arguments to the filter_ref struct.

  2. In git v2.7.0, we expose the formatting language to the
     user. If we follow this path, it will mean that
     "%(refname:short)" behaves differently for "tag" versus
     "for-each-ref" (including "for-each-ref refs/tags/"),
     which can lead to confusion.

Instead, let's add a new modifier to the formatting
language, "strip", to remove a specific set of prefix
components. This fixes "git tag", and lets users invoke the
same behavior from their own custom formats (for "tag" or
"for-each-ref") while leaving ":short" with its same
consistent meaning in all places.

We introduce a test in t7004 for "git tag", which fails
without this patch. We also add a similar test in t3203 for
"git branch", which does not actually fail. But since it is
likely that "branch" will eventually use the same formatting
code, the test helps defend against future regressions.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jan 26, 2016
1 parent 1d094db commit 0571979
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 4 deletions.
6 changes: 5 additions & 1 deletion Documentation/git-for-each-ref.txt
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ refname::
The name of the ref (the part after $GIT_DIR/).
For a non-ambiguous short name of the ref append `:short`.
The option core.warnAmbiguousRefs is used to select the strict
abbreviation mode.
abbreviation mode. If `strip=<N>` is appended, strips `<N>`
slash-separated path components from the front of the refname
(e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
`<N>` must be a positive integer. If a displayed ref has fewer
components than `<N>`, the command aborts with an error.

objecttype::
The type of the object (`blob`, `tree`, `commit`, `tag`).
Expand Down
2 changes: 1 addition & 1 deletion Documentation/git-tag.txt
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ This option is only applicable when listing tags without annotation lines.
A string that interpolates `%(fieldname)` from the object
pointed at by a ref being shown. The format is the same as
that of linkgit:git-for-each-ref[1]. When unspecified,
defaults to `%(refname:short)`.
defaults to `%(refname:strip=2)`.

--[no-]merged [<commit>]::
Only list tags whose tips are reachable, or not reachable
Expand Down
4 changes: 2 additions & 2 deletions builtin/tag.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
if (!format) {
if (filter->lines) {
to_free = xstrfmt("%s %%(contents:lines=%d)",
"%(align:15)%(refname:short)%(end)",
"%(align:15)%(refname:strip=2)%(end)",
filter->lines);
format = to_free;
} else
format = "%(refname:short)";
format = "%(refname:strip=2)";
}

verify_ref_format(format);
Expand Down
26 changes: 26 additions & 0 deletions ref-filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,29 @@ static inline char *copy_advance(char *dst, const char *src)
return dst;
}

static const char *strip_ref_components(const char *refname, const char *nr_arg)
{
char *end;
long nr = strtol(nr_arg, &end, 10);
long remaining = nr;
const char *start = refname;

if (nr < 1 || *end != '\0')
die(":strip= requires a positive integer argument");

while (remaining) {
switch (*start++) {
case '\0':
die("ref '%s' does not have %ld components to :strip",
refname, nr);
case '/':
remaining--;
break;
}
}
return start;
}

/*
* Parse the object referred by ref, and grab needed value.
*/
Expand Down Expand Up @@ -909,11 +932,14 @@ static void populate_value(struct ref_array_item *ref)
formatp = strchr(name, ':');
if (formatp) {
int num_ours, num_theirs;
const char *arg;

formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
warn_ambiguous_refs);
else if (skip_prefix(formatp, "strip=", &arg))
refname = strip_ref_components(refname, arg);
else if (!strcmp(formatp, "track") &&
(starts_with(name, "upstream") ||
starts_with(name, "push"))) {
Expand Down
8 changes: 8 additions & 0 deletions t/t3203-branch-output.sh
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,12 @@ test_expect_success 'git branch --points-at option' '
test_cmp expect actual
'

test_expect_success 'ambiguous branch/tag not marked' '
git tag ambiguous &&
git branch ambiguous &&
echo " ambiguous" >expect &&
git branch --list ambiguous >actual &&
test_cmp expect actual
'

test_done
12 changes: 12 additions & 0 deletions t/t6300-for-each-ref.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ test_atom() {

test_atom head refname refs/heads/master
test_atom head refname:short master
test_atom head refname:strip=1 heads/master
test_atom head refname:strip=2 master
test_atom head upstream refs/remotes/origin/master
test_atom head upstream:short origin/master
test_atom head push refs/remotes/myfork/master
Expand Down Expand Up @@ -132,6 +134,16 @@ test_expect_success 'Check invalid atoms names are errors' '
test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
'

test_expect_success 'arguments to :strip must be positive integers' '
test_must_fail git for-each-ref --format="%(refname:strip=0)" &&
test_must_fail git for-each-ref --format="%(refname:strip=-1)" &&
test_must_fail git for-each-ref --format="%(refname:strip=foo)"
'

test_expect_success 'stripping refnames too far gives an error' '
test_must_fail git for-each-ref --format="%(refname:strip=3)"
'

test_expect_success 'Check format specifiers are ignored in naming date atoms' '
git for-each-ref --format="%(authordate)" refs/heads &&
git for-each-ref --format="%(authordate:default) %(authordate)" refs/heads &&
Expand Down
8 changes: 8 additions & 0 deletions t/t7004-tag.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1558,4 +1558,12 @@ test_expect_success '--no-merged show unmerged tags' '
test_cmp expect actual
'

test_expect_success 'ambiguous branch/tags not marked' '
git tag ambiguous &&
git branch ambiguous &&
echo ambiguous >expect &&
git tag -l ambiguous >actual &&
test_cmp expect actual
'

test_done

0 comments on commit 0571979

Please sign in to comment.