git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / 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

  List information: http://vger.kernel.org/majordomo-info.html

* 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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).