All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Rubén Justo" <rjusto@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>,
	"Patrick Steinhardt" <ps@pks.im>
Subject: Re: [PATCH v4] add-patch: response to unknown command
Date: Mon, 29 Apr 2024 10:48:13 +0100	[thread overview]
Message-ID: <33bb69ad-ec79-4863-97b7-51605b94c9c5@gmail.com> (raw)
In-Reply-To: <xmqqfrva22py.fsf@gitster.g>

Hi Junio

On 24/04/2024 16:35, Junio C Hamano wrote:
> phillip.wood123@gmail.com writes:
> 
>> On 22/04/2024 16:50, Junio C Hamano wrote:
>>> Rubén Justo <rjusto@gmail.com> writes:
>>>
>>>> 1:  0317594bce ! 1:  b418b03f15 add-patch: response to unknown command
>>>>       @@ t/t3701-add-interactive.sh: test_expect_success 'warn about add.interactive.useB
>>>>        +	test_when_finished "git reset --hard; rm -f command" &&
>>>>        +	echo W >command &&
>>>>        +	git add -N command &&
>>>>       -+	cat >expect <<-EOF &&
>>>>       -+	diff --git a/command b/command
>>>>       -+	new file mode 100644
>>>>       -+	index 0000000..a42d8ff
>>>>       -+	--- /dev/null
>>>>       -+	+++ b/command
>>>>       -+	@@ -0,0 +1 @@
>>>>       -+	+W
>>>>       ++	git diff command >expect &&
>>>>       ++	cat >>expect <<-EOF &&
>>>>        +	(1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
>>>>        +	(1/1) Stage addition [y,n,q,a,d,e,p,?]?$SP
>>>>        +	EOF
>>> Interesting.
>>> My first reaction was "how is this different from checking just the
>>> last line of the actual output?  The early part of expect and actual
>>> both come from an internal invocation of 'git diff', and they must
>>> match by definition".
>>> But that may really be the point of this test.
>>
>> Yes - we want to make sure that we are not printing the help and the
>> only way to do that is to check the whole output
> 
> I was not questioning that part of the patch.  "My first reaction"
> was solely about use of "git diff" to replace the golden copy of
> expected result in the test itself, only to allow for use of
> different hash functions.  As you (or somebody else?) mentioned in
> an earlier review, diff_cmp is there for exactly that purpose.

Oh sorry I'd misunderstood

>>> That is, we may later decide to tweak the way "git diff" hunks are
>>> presented, and we expect that the way "git add -p" presents the
>>> hunks would change together with it automatically.
> 
> This argument cuts both ways, by the way.
> 
> Insisting that the output match the explicit expectation (not what
> "git diff" of the day produces) has a few advantages.  It makes the
> test more explicit and easy to see what output we are expecting, and
> more importantly, it forces us to update the test when we decide to
> tweak the output from the command being tested (i.e. hunk selection
> UI) and/or the output from "git diff" command.

There is also a practical argument against using "git diff" to generate 
the expected output as it only works if the diff contains a single hunk. 
If the diff contains more than one hunk "add -p" displays them separately.

Best Wishes

Phillip

  reply	other threads:[~2024-04-29  9:48 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 19:00 [PATCH] add-patch: response to invalid option Rubén Justo
2024-04-16  5:51 ` Patrick Steinhardt
2024-04-16 19:11   ` Rubén Justo
2024-04-16  9:41 ` phillip.wood123
2024-04-16 19:24   ` Rubén Justo
2024-04-17  9:37     ` phillip.wood123
2024-04-17 15:05       ` Junio C Hamano
2024-04-18 15:11         ` phillip.wood123
2024-04-16 10:26 ` Junio C Hamano
2024-04-16 13:56   ` Phillip Wood
2024-04-16 15:22     ` Junio C Hamano
2024-04-16 15:46       ` Phillip Wood
2024-04-16 16:10         ` Junio C Hamano
2024-04-16 19:31   ` Rubén Justo
2024-04-20 11:08 ` [PATCH v2] add-patch: response to invalid command Rubén Justo
2024-04-20 17:53   ` Junio C Hamano
2024-04-21  9:51   ` [PATCH v3] add-patch: response to unknown command Rubén Justo
2024-04-21 13:18     ` phillip.wood123
2024-04-21 19:37       ` Rubén Justo
2024-04-21 21:52     ` [PATCH v4] " Rubén Justo
2024-04-22 15:50       ` Junio C Hamano
2024-04-24 10:15         ` phillip.wood123
2024-04-24 15:35           ` Junio C Hamano
2024-04-29  9:48             ` Phillip Wood [this message]
2024-04-29 16:09               ` Junio C Hamano
2024-04-25  1:44       ` Jeff King
2024-04-25  2:15         ` Eric Sunshine
2024-04-25 20:23           ` Junio C Hamano
2024-04-25 21:00             ` Eric Sunshine
2024-04-25 21:13               ` Junio C Hamano
2024-04-25 21:05             ` Junio C Hamano
2024-04-25 22:09               ` Rubén Justo
2024-04-25 22:16                 ` Junio C Hamano
2024-04-25 23:46                   ` Rubén Justo
2024-04-26  5:39                     ` Junio C Hamano
2024-04-26 16:26                     ` Junio C Hamano
2024-04-26 20:21           ` Jeff King
2024-04-25  3:04         ` Rubén Justo
2024-04-26 20:23           ` Jeff King
2024-04-26 20:41             ` Rubén Justo
2024-04-25  8:53         ` phillip.wood123
2024-04-29 18:35       ` [PATCH v5 0/2] " Rubén Justo
2024-04-29 18:37         ` [PATCH v5 1/2] add-patch: do not show UI messages on stderr Rubén Justo
2024-04-29 19:24           ` Junio C Hamano
2024-04-30 10:52             ` Jeff King
2024-04-30 16:35               ` Rubén Justo
2024-04-30 17:17                 ` Junio C Hamano
2024-04-30 17:11               ` Junio C Hamano
2024-04-30 14:47           ` phillip.wood123
2024-04-30 16:38             ` Rubén Justo
2024-05-01 15:39               ` phillip.wood123
2024-05-01 16:14                 ` Junio C Hamano
2024-05-01 21:13                   ` Rubén Justo
2024-05-02 16:38                     ` Junio C Hamano
2024-04-29 18:37         ` [PATCH v5 2/2] add-patch: response to unknown command Rubén Justo

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=33bb69ad-ec79-4863-97b7-51605b94c9c5@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=rjusto@gmail.com \
    /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.