tag: do not show ambiguous tag names as "tags/foo"

Since b7cc53e9 (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 b7cc53e9 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>

Jeff King committed Jan 25, 2016 at 22:00 UTC 0571979bd60837d3c0802ecc1a47c48b4a6114d0
7 files changed +62 -4
Documentation/git-for-each-ref.txt
+5 -1
@@ -92,7 +92,11 @@ refname::
92 The name of the ref (the part after $GIT_DIR/).
93 For a non-ambiguous short name of the ref append `:short`.
94 The option core.warnAmbiguousRefs is used to select the strict
95 - abbreviation mode.
95 + abbreviation mode. If `strip=<N>` is appended, strips `<N>`
96 + slash-separated path components from the front of the refname
97 + (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
98 + `<N>` must be a positive integer. If a displayed ref has fewer
99 + components than `<N>`, the command aborts with an error.
100
101 objecttype::
102 The type of the object (`blob`, `tree`, `commit`, `tag`).
Documentation/git-tag.txt
+1 -1
@@ -163,7 +163,7 @@ This option is only applicable when listing tags without annotation lines.
163 A string that interpolates `%(fieldname)` from the object
164 pointed at by a ref being shown. The format is the same as
165 that of linkgit:git-for-each-ref[1]. When unspecified,
166 - defaults to `%(refname:short)`.
166 + defaults to `%(refname:strip=2)`.
167
168 --[no-]merged [<commit>]::
169 Only list tags whose tips are reachable, or not reachable
builtin/tag.c
+2 -2
@@ -44,11 +44,11 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
44 if (!format) {
45 if (filter->lines) {
46 to_free = xstrfmt("%s %%(contents:lines=%d)",
47 - "%(align:15)%(refname:short)%(end)",
47 + "%(align:15)%(refname:strip=2)%(end)",
48 filter->lines);
49 format = to_free;
50 } else
51 - format = "%(refname:short)";
51 + format = "%(refname:strip=2)";
52 }
53
54 verify_ref_format(format);
ref-filter.c
+26
@@ -763,6 +763,29 @@ static inline char *copy_advance(char *dst, const char *src)
763 return dst;
764 }
765
766 +static const char *strip_ref_components(const char *refname, const char *nr_arg)
767 +{
768 + char *end;
769 + long nr = strtol(nr_arg, &end, 10);
770 + long remaining = nr;
771 + const char *start = refname;
772 +
773 + if (nr < 1 || *end != '\0')
774 + die(":strip= requires a positive integer argument");
775 +
776 + while (remaining) {
777 + switch (*start++) {
778 + case '\0':
779 + die("ref '%s' does not have %ld components to :strip",
780 + refname, nr);
781 + case '/':
782 + remaining--;
783 + break;
784 + }
785 + }
786 + return start;
787 +}
788 +
789 /*
790 * Parse the object referred by ref, and grab needed value.
791 */
@@ -909,11 +932,14 @@ static void populate_value(struct ref_array_item *ref)
932 formatp = strchr(name, ':');
933 if (formatp) {
934 int num_ours, num_theirs;
935 + const char *arg;
936
937 formatp++;
938 if (!strcmp(formatp, "short"))
939 refname = shorten_unambiguous_ref(refname,
940 warn_ambiguous_refs);
941 + else if (skip_prefix(formatp, "strip=", &arg))
942 + refname = strip_ref_components(refname, arg);
943 else if (!strcmp(formatp, "track") &&
944 (starts_with(name, "upstream") ||
945 starts_with(name, "push"))) {
t/t3203-branch-output.sh
+8
@@ -176,4 +176,12 @@ test_expect_success 'git branch --points-at option' '
176 test_cmp expect actual
177 '
178
179 +test_expect_success 'ambiguous branch/tag not marked' '
180 + git tag ambiguous &&
181 + git branch ambiguous &&
182 + echo " ambiguous" >expect &&
183 + git branch --list ambiguous >actual &&
184 + test_cmp expect actual
185 +'
186 +
187 test_done
t/t6300-for-each-ref.sh
+12
@@ -50,6 +50,8 @@ test_atom() {
50
51 test_atom head refname refs/heads/master
52 test_atom head refname:short master
53 +test_atom head refname:strip=1 heads/master
54 +test_atom head refname:strip=2 master
55 test_atom head upstream refs/remotes/origin/master
56 test_atom head upstream:short origin/master
57 test_atom head push refs/remotes/myfork/master
@@ -132,6 +134,16 @@ test_expect_success 'Check invalid atoms names are errors' '
134 test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
135 '
136
137 +test_expect_success 'arguments to :strip must be positive integers' '
138 + test_must_fail git for-each-ref --format="%(refname:strip=0)" &&
139 + test_must_fail git for-each-ref --format="%(refname:strip=-1)" &&
140 + test_must_fail git for-each-ref --format="%(refname:strip=foo)"
141 +'
142 +
143 +test_expect_success 'stripping refnames too far gives an error' '
144 + test_must_fail git for-each-ref --format="%(refname:strip=3)"
145 +'
146 +
147 test_expect_success 'Check format specifiers are ignored in naming date atoms' '
148 git for-each-ref --format="%(authordate)" refs/heads &&
149 git for-each-ref --format="%(authordate:default) %(authordate)" refs/heads &&
t/t7004-tag.sh
+8
@@ -1558,4 +1558,12 @@ test_expect_success '--no-merged show unmerged tags' '
1558 test_cmp expect actual
1559 '
1560
1561 +test_expect_success 'ambiguous branch/tags not marked' '
1562 + git tag ambiguous &&
1563 + git branch ambiguous &&
1564 + echo ambiguous >expect &&
1565 + git tag -l ambiguous >actual &&
1566 + test_cmp expect actual
1567 +'
1568 +
1569 test_done