All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Caleb Thompson <caleb@calebthompson.io>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 4/4] commit: Add commit.verbose configuration
Date: Mon, 16 Jun 2014 15:25:22 -0700	[thread overview]
Message-ID: <xmqq38f4pk4t.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140616201037.GA37953@sirius.local> (Caleb Thompson's message of "Mon, 16 Jun 2014 15:10:37 -0500")

Caleb Thompson <caleb@calebthompson.io> 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.

  reply	other threads:[~2014-06-16 22:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-12 19:38 [PATCH v5 0/4] commit: Add commit.verbose configuration Caleb Thompson
2014-06-12 19:38 ` [PATCH v5 1/4] commit test: Use test_config instead of git-config Caleb Thompson
2014-06-12 19:39 ` [PATCH v5 2/4] commit test: Use write_script Caleb Thompson
2014-06-13  6:50   ` Jeff King
2014-06-13 16:26     ` Caleb Thompson
2014-06-13 23:28       ` Jeff King
2014-06-12 19:39 ` [PATCH v5 3/4] commit test: test_set_editor in each test Caleb Thompson
2014-06-13  6:59   ` Jeff King
2014-06-13 16:36     ` Caleb Thompson
2014-06-13 17:16       ` Jakub Narębski
2014-06-13 17:47         ` Caleb Thompson
2014-06-13 18:52           ` Jakub Narębski
2014-06-13 23:39       ` Jeff King
2014-06-13 17:42     ` Junio C Hamano
2014-06-13 23:41       ` Jeff King
2014-06-16 17:46         ` Caleb Thompson
2014-06-16 18:58           ` Junio C Hamano
2014-06-12 20:00 ` [PATCH v5 4/4] commit: Add commit.verbose configuration Caleb Thompson
2014-06-13 17:48   ` Junio C Hamano
2014-06-16 19:50     ` Caleb Thompson
2014-06-16 20:05       ` Caleb Thompson
2014-06-16 20:06       ` Junio C Hamano
2014-06-16 20:10         ` Caleb Thompson
2014-06-16 22:25           ` Junio C Hamano [this message]
2014-06-12 20:30 ` [PATCH v5 0/4] " Jeremiah Mahler
2014-06-13 16:49   ` Caleb Thompson
2014-06-14  4:14     ` Jeremiah Mahler
2014-06-16 20:28       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq38f4pk4t.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=caleb@calebthompson.io \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.