Re: [PATCH 8/8] check-whitespace: detect if no base_commit is provided

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> Yeah, makes sense. I think I'll simply add in
>
>     if ! git rev-parse --quiet --verify "${baseCommit}"
>     then
>         echo "Invalid <BASE_COMMIT> '${baseCommit}'"
>         exit 1
>     fi

It would make the intent a lot clearer.  Good.

>>> +if test $? -ne 0
>>> +then
>>> +	echo -n $gitLogOutput
>>
>> What is "-n" doing here?  Why are you squashing run of spaces in the
>> $gitLogOutput variable into a space by not "quoting" inside a dq-pair?
>>
>
> I actually didn't know about this. Thanks for informing.
>
>>> +	exit 1
>>> +fi
>>
>> Looking for "--check" in
>>
>> 	$ git log --help
>>
>> tells me that the command exits with non-zero status if problems are
>> found, so wouldn't that mean the cases with problems always exit
>> early, bypassing the while loop with full of bash-ism that comes
>> after this block?
>>
>
> It should exist with a non-zero code, but since we're capturing it in
> the while loop, it doesn't stop the slow.

Sorry, but I am confused.  The control would not even reach a "while
loop", I am afraid.

I was commenting on the exit status check done here:

    +gitLogOutput=$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..)
    +if test $? -ne 0
    +then
    +	echo -n $gitLogOutput
    +	exit 1
    +fi

Even though the output is captured in a variable, the exit status of
"git log --check" is still seen by the shell and "if test $? = 0"
next line say "ah, the thing exited with non-zero status so lets
echo the whole thing and exit with 1", before it gets to the while
loop we have below the above piece of code, no?