Re: [PATCH] Clear --exclude list after 'git rev-parse --all'
- From
- Andreas Gruenbacher <agruenba@redhat.com>
- Date
- Oct 24, 2018, 09:01 UTC
- Message-ID
- <CAHc6FU6YLeTBU02k-Md7+H5kJDq2RXZmAXMaq8ZkiuSmZFXbwQ@mail.gmail.com>
- In-Reply-To
- <xmqqefcgotbk.fsf@gitster-ct.c.googlers.com>
On Wed, 24 Oct 2018 at 06:35, Junio C Hamano <gitster@pobox.com> wrote:
Show 23 quoted lines
> Andreas Gruenbacher <agruenba@redhat.com> writes:
>
> > Commit [1] added the --exclude option to revision.c. The --all,
> > --branches, --tags, --remotes, and --glob options clear the exclude
> > list. Shortly therafter, commit [2] added the same to 'git rev-parse',
> > but without clearing the exclude list for the --all option. Fix that.
> >
> > [1] e7b432c52 ("revision: introduce --exclude=<glob> to tame wildcards", 2013-08-30)
> > [2] 9dc01bf06 ("rev-parse: introduce --exclude=<glob> to tame wildcards", 2013-11-01)
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > ---
> > builtin/rev-parse.c | 1 +
> > 1 file changed, 1 insertion(+)
>
> All other glob options do show_reference with for_each_ref_in() and
> then calls clear_ref_exclusion(), and logically the patch makes
> sense.
>
> What is the "problem" this patch fixes, though? Is it easy to add a
> new test to t/6018-rev-list-glob.sh to demonstrate that "--glob" (or
> whatever that clears exclusion list without this patch) works
> correctly but "--all" misbehaves without this change?The test suite doesn't cover clearing the exclusion list for any of those rev-parse options and I also didn't write such a test case. I ran into this inconsistency during code review.
Andreas