Re: [PATCHv7] pathspec: give better message for submodule related pathspec error
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 9, 2017, 22:53 UTC
- Message-ID
- <xmqqeg0bu9oq.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170109224330.28405-1-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 17 quoted lines
> Every once in a while someone complains to the mailing list to have
> run into this weird assertion[1]. The usual response from the mailing
> list is link to old discussions[2], and acknowledging the problem
> stating it is known.
>
> This patch accomplishes two things:
>
> 1. Switch assert() to die("BUG") to give a more readable message.
>
> 2. Take one of the cases where we hit a BUG and turn it into a normal
> "there was something wrong with the input" message.
>
> This assertion triggered for cases where there wasn't a programming
> bug, but just bogus input. In particular, if the user asks for a
> pathspec that is inside a submodule, we shouldn't assert() or
> die("BUG"); we should tell the user their request is bogus.
>Is it only me who sees funny black rectangles in front of these four lines instead of blanks, by the way?
Show 8 quoted lines
> This comes as a single patch again, replacing sb/pathspec-errors. > It goes directly on top of bw/pathspec-cleanup. > > v7: > do not rely on "test_commit -C" being there, nor the infrastructure > to request a "good" submodule upstream. Just create a submodule outselves > to test in. >
Thanks.
Show 11 quoted lines
> diff --git a/t/t6134-pathspec-in-submodule.sh b/t/t6134-pathspec-in-submodule.sh > new file mode 100755 > index 0000000000..d952ae2cae > --- /dev/null > +++ b/t/t6134-pathspec-in-submodule.sh > @@ -0,0 +1,40 @@ > +#!/bin/sh > + > +test_description='test case exclude pathspec' > + > +TEST_CREATE_SUBMODULE=yes
Did you mean to keep this?
Show 7 quoted lines
> +. ./test-lib.sh > + > +test_expect_success 'setup a submodule' ' > + test_create_repo pretzel && > + ( > + cd pretzel && > + touch a &&
This is better spelled as
: >a &&
because use of touch, when you do not care about the file timestamp, is misleading.
Thanks.