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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 16, 2014, 22:25 UTC
Message-ID: <xmqq38f4pk4t.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140616201037.GA37953@sirius.local>

Caleb Thompson <caleb@calebthompson.io> writes:

Show 20 quoted lines

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