Re: [PATCH v4] for-each-ref: `:short` format for `refname`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 5, 2008, 22:20 UTC
- Message-ID
- <7vej3yutes.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1220649383-17916-1-git-send-email-bert.wesarg@googlemail.com>
Bert Wesarg <bert.wesarg@googlemail.com> writes:
Show 10 quoted lines
> ... > To integrate this new format into the bash completion to get > only non-ambiguous refs is beyond the scope of this patch. > > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> > > --- > Cc: git@vger.kernel.org > Cc: szeder@ira.uka.de > Cc: Shawn O. Pearce <spearce@spearce.org>
Nice writeup of the history of this patch, if on tad-too-verbose side.
Show 34 quoted lines
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 21e92bb..9b44092 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -546,6 +546,107 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
> +/*
> + * Shorten the refname to an non-ambiguous form
> + */
> +static char *get_short_ref(struct refinfo *ref)
> +{
> ...
> + /* skip first rule, it will always match */
> + for (i = nr_rules - 1; i > 0 ; --i) {
> + int j;
> + int short_name_len;
> +
> + if (1 != sscanf(ref->refname, scanf_fmts[i], short_name))
> + continue;
> +
> + short_name_len = strlen(short_name);
> +
> + /*
> + * 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++) {
> ...
> + }
> + /*
> + * short name is non-ambiguous if all previous rules
> + * haven't resolved to a valid ref
> + */
> + if (j == i)
> + return short_name;Is this inner loop essentially the same as calling dwim_ref(), while temporarily turning warn_ambiguous_refs on, and checking for return value of one?
Show 18 quoted lines
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh > index 8ced593..4f247dd 100755 > --- a/t/t6300-for-each-ref.sh > +++ b/t/t6300-for-each-ref.sh > @@ -262,6 +262,50 @@ for i in "--perl --shell" "-s --python" "--python --tcl" "--tcl --perl"; do > " > done > > +cat >expected <<\EOF > +master > +testtag > +EOF > + > +test_expect_success 'Check short refname format' ' > + (git for-each-ref --format="%(refname:short)" refs/heads && > + git for-each-ref --format="%(refname:short)" refs/tags) >actual && > + test_cmp expected actual > +'
Not a complaint nor objection but mere curiosity. Why does this run two for-each-ref, not just one with two patterns?
> +test_expect_success 'Check for invalid refname format' ' > + test_must_fail git for-each-ref --format="%(refname:INVALID)" > +'
Good.
Show 29 quoted lines
> +cat >expected <<\EOF > +heads/master > +master > +EOF > + > +test_expect_success 'Check ambiguous head and tag refs' ' > + git checkout -b newtag && > + echo "Using $datestamp" > one && > + git add one && > + git commit -m "Branch" && > + setdate_and_increment && > + git tag -m "Tagging at $datestamp" master && > + git for-each-ref --format "%(refname:short)" refs/heads/master refs/tags/master >actual && > + test_cmp expected actual > +' > + > +cat >expected <<\EOF > +heads/ambiguous > +ambiguous > +EOF > + > +test_expect_success 'Check ambiguous head and tag refs II' ' > + git checkout master && > + git tag ambiguous testtag^0 && > + git branch ambiguous testtag^0 && > + git for-each-ref --format "%(refname:short)" refs/heads/ambiguous refs/tags/ambiguous >actual && > + test_cmp expected actual > +' > +
Can we also try first creating a clone of some repo and run:
for-each-ref --format="%(refname:short)" refs/remotes
I am unsure how "remotes/origin" when "refs/remotes/origin/HEAD" points at their 'master' branch behaves with your code, and/or how it should behave.
Other than that, nicely done.