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

[Karthik Nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1908 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> 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?

My bad, I thought you were referring to the code before my changes. Yes,
here you're right, we don't need the check since the shell would capture
the non-zero status.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]