[PATCH 1/3] for-each-ref: utilize core.warnambiguousrefs for strict refname:short format
From: Bert Wesarg <hidden>
Date: 2016-06-15 22:45:23
Subsystem:
documentation, the rest · Maintainers:
Jonathan Corbet, Linus Torvalds
core.warnambiguousrefs is used to enable strict mode for the abbreviation. In strict mode, the abbreviated ref will never trigger the 'warn_ambiguous_refs' warning. I.e. for these refs: refs/heads/xyzzy refs/tags/xyzzy the abbreviated forms are: heads/xyzzy tags/xyzzy Signed-off-by: Bert Wesarg <redacted> --- On Tue, Sep 9, 2008 at 10:05, Junio C Hamano [off-list ref] wrote:
"Bert Wesarg" [off-list ref] writes:quoted
Any opinions, whether we want the 'strict' mode? i.e.: for refs/heads/xyzzy and refs/tags/xyzzy: loose mode (current implementation): refs/heads/xyzzy => heads/xyzzy refs/tags/xyzzy => xyzzy there would be a ambiguous warning (if enabled) if you use xyzzy as a tag, but it resolves correctly to the tag. strict mode: refs/heads/xyzzy => heads/xyzzy refs/tags/xyzzy => tags/xyzzy will always produce a non-ambiguous short forms.I have no strong opinions either way, but if we want to pick only one, I suspect that the loose mode would be more appropriate for bash completion purposes exactly because: (1) the shorter form would match the users' expectations, and; (2) if it triggers ambiguity warning to use that result that matches users' expectations, it is a *good thing* --- it reminds the user that s/he is playing with fire _if_ the user is of careful type who enables the ambiguity warning. Thinking about it from a different angle, it would make more sense to use loose mode if the user does not have ambiguity warning configured, and use strict mode if the warning is enabled. Then people who will get warnings from ambiguity will not get an ambiguous completion, and people who won't will get shorter but still unambiguous completion.
Cc: git@vger.kernel.org Cc: szeder@ira.uka.de Cc: "Shawn O. Pearce" <redacted> Documentation/git-for-each-ref.txt | 2 + builtin-for-each-ref.c | 43 ++++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 5061d3e..265bbf3 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt@@ -75,6 +75,8 @@ For all objects, the following names can be used: 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 enable the strict mode + for the abbretiation. objecttype:: The type of the object (`blob`, `tree`, `commit`, `tag`).
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 9b44092..e7b7712 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c@@ -571,7 +571,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule) /* * Shorten the refname to an non-ambiguous form */ -static char *get_short_ref(struct refinfo *ref) +static void get_short_ref(struct refinfo *ref, int strict, char **short_ref) { int i; static char **scanf_fmts;
@@ -598,16 +598,16 @@ static char *get_short_ref(struct refinfo *ref) } } - /* bail out if there are no rules */ - if (!nr_rules) - return ref->refname; - /* buffer for scanf result, at most ref->refname must fit */ short_name = xstrdup(ref->refname); + *short_ref = short_name; - /* skip first rule, it will always match */ - for (i = nr_rules - 1; i > 0 ; --i) { - int j; + /* bail out if there are no rules */ + if (!nr_rules) + return; + + for (i = nr_rules - 1; i >= 0 ; --i) { + int j, rules_to_fail = i; int short_name_len; if (1 != sscanf(ref->refname, scanf_fmts[i], short_name))
@@ -616,13 +616,23 @@ static char *get_short_ref(struct refinfo *ref) short_name_len = strlen(short_name); /* + * in strict mode, all (except the matched one) rules + * must fail to resolve to a valid ref + */ + if (strict) + rules_to_fail = nr_rules; + /* * check if the short name resolves to a valid ref, * but use only rules prior to the matched one */ - for (j = 0; j < i; j++) { + for (j = 0; j < rules_to_fail; j++) { const char *rule = ref_rev_parse_rules[j]; unsigned char short_objectname[20]; + /* skip matched rule */ + if (i == j) + continue; + /* * the short name is ambiguous, if it resolves * (with this previous rule) to a valid ref
@@ -635,14 +645,14 @@ static char *get_short_ref(struct refinfo *ref) /* * short name is non-ambiguous if all previous rules - * haven't resolved to a valid ref + * doesn't resolved to a valid ref */ - if (j == i) - return short_name; + if (j == rules_to_fail) + return; } - free(short_name); - return ref->refname; + /* can't abbreviate refname, return full name */ + strcpy(short_name, ref->refname); }
@@ -678,13 +688,14 @@ static void populate_value(struct refinfo *ref) } if (!prefixcmp(name, "refname")) { const char *formatp = strchr(name, ':'); - const char *refname = ref->refname; + char *refname = ref->refname; /* look for "short" refname format */ if (formatp) { formatp++; if (!strcmp(formatp, "short")) - refname = get_short_ref(ref); + get_short_ref(ref, warn_ambiguous_refs, + &refname); else die("unknown refname format %s", formatp);
--
1.6.0.1