Karthik Nayak <karthik....@gmail.com> writes:
> @@ -458,7 +345,7 @@ static void add_verbose_info(struct strbuf *out, struct
> ref_array_item *item,
> }
>
> if (item->kind == REF_LOCAL_BRANCH)
> - fill_tracking_info(&stat, item->refname, filter->verbose > 1);
> + fill_tracking_info(&stat, refname, filter->verbose > 1);
Why can't you continue using item->refname?
(It's a real question)
> @@ -635,14 +495,21 @@ static void print_ref_list(struct ref_filter *filter)
> /* Print detached heads before sorting and printing the rest */
> if (filter->detached) {
> print_ref_item(array.items[index - 1], maxwidth, filter,
> remote_prefix);
> - index -= 1;
> + array.nr--;
> }
>
> - qsort(array.items, index, sizeof(struct ref_array_item *), ref_cmp);
> + if (!sorting) {
> + def_sorting.next = NULL;
> + def_sorting.atom = parse_ref_filter_atom(sort_type,
> + sort_type +
> strlen(sort_type));
> + sorting = &def_sorting;
> + }
> + ref_array_sort(sorting, &array);
Does this belong to print_ref_list()? Is it not possible to extract it
to get a code closer to the simple:
filter_refs(...);
ref_array_sort(...);
print_ref_list(...);
?
> - for (i = 0; i < index; i++)
> + for (i = 0; i < array.nr; i++)
> print_ref_item(array.items[i], maxwidth, filter, remote_prefix);
Now that we have show_ref_array_item, it may make sense to rename
print_ref_item to something that make the difference between these
functions more explicit. Well, ideally, you'd get rid of it an actually
use show_ref_array_item, but if you are to keep it, maybe
print_ref_item_default_branch_format (or something shorter)?
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -49,7 +49,6 @@ struct ref_sorting {
> struct ref_array_item {
> unsigned char objectname[20];
> int flag, kind;
> - int ignore : 1;
You should explain why you needed it and why you don't need it anymore
(I guess, because it was used to implement --merge and you now get it
from ref-filter).
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -38,7 +38,7 @@ test_expect_success 'fast-import: fail on invalid branch
> name "bad[branch]name"'
> test_must_fail git fast-import <input
> '
>
> -test_expect_success 'git branch shows badly named ref' '
> +test_expect_failure 'git branch does not show badly named ref' '
I'm not sure what's the convention, but I think the test description
should give the expected behavior even with test_expect_failure.
And please help the reviewers by saying what's the status wrt this test
(any plan on how to fix it?).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
