Subject: Re: [PATCH v5 4/4] commit: Add commit.verbose configuration

From: Junio C Hamano <[email-masked]>
Date: Mon, 16 Jun 2014 15:25:22 -0700

Caleb Thompson <[email-masked]> writes:

>> Again, what are we testing, exactly?
>>
>> We do not want to see "^diff --git" in the output file, in other
>> words, we want to make sure "^diff --git" does not appear in the
>> output.
>>
>> So
>>
>>         write_script check-for-no-diff <<-\EOF
>>         ! grep '^diff --git' "$@"
>>	EOF
>>
>> should be the most natural way to express what we are testing, no?
>
> I did consider that. The reason I didn't propose that is that it doesn't catch
> the unlikely case that the $1 only contains a "diff --git" line or that $1 is
> empty.
>
> Those are rather unreasonable concerns, so I'm happy to use the much more
> readable version as you propose.

If it only has "diff --git", then the grep will find a hit, exits
with success, the script yields the opposite and "git commit" will
fail, which is what we want, so that is OK.  "$1 is empty" may or
may not be an error, depending on your settings, I guess (i.e. can't
we squelch the "# helpful instruction" lines altogether?)?

If the editor input is expected to be very stable, we could even do
something like:

	write_script check-editor-input <<-\EOF
	diff expect "$1" >&2
        EOF

and then catch any deviation from the norm with something like:

	cat >expect <<-\EOF &&
        ... expected editor input comes here ...
        EOF
        test_set_editor "$PWD/check-editor-input &&
	git commit --amend

but if the editor input may easily be affected by volatile things
like blob object names given in the diff output by "commit -v" or
untracked cruft in the working tree listed in the status output,
then it would add unnecessary maintenance burden (every time earlier
parts of the test scripts are updated, the expected output may have
to change to adjust to these non-essential details), so it is a
judgment call.

Thanks.