git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] add-patch: response to unknown command

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Apr 29, 2024, 09:48 UTC
Message-ID
<33bb69ad-ec79-4863-97b7-51605b94c9c5@gmail.com>
In-Reply-To
<xmqqfrva22py.fsf@gitster.g>
Hi Junio
On 24/04/2024 16:35, Junio C Hamano wrote:
Show 38 quoted lines
> 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
Show 12 quoted lines
>>> 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
Previous: Junio C HamanoNext: Junio C Hamano
Message 24 of 55 in “add-patch: response to invalid option”
  1. add-patch: response to invalid optionRubén Justo, Apr 15, 2024
  2. Patrick SteinhardtApr 16, 2024
  3. Rubén JustoApr 16, 2024
  4. phillip.wood123@gmail.comApr 16, 2024
  5. Rubén JustoApr 16, 2024
  6. phillip.wood123@gmail.comApr 17, 2024
  7. Junio C HamanoApr 17, 2024
  8. phillip.wood123@gmail.comApr 18, 2024
  9. Junio C HamanoApr 16, 2024
  10. Phillip WoodApr 16, 2024
  11. Junio C HamanoApr 16, 2024
  12. Phillip WoodApr 16, 2024
  13. Junio C HamanoApr 16, 2024
  14. Rubén JustoApr 16, 2024
  15. add-patch: response to invalid commandRubén Justo, Apr 20, 2024
  16. Junio C HamanoApr 20, 2024
  17. add-patch: response to unknown commandRubén Justo, Apr 21, 2024
  18. phillip.wood123@gmail.comApr 21, 2024
  19. Rubén JustoApr 21, 2024
  20. add-patch: response to unknown commandRubén Justo, Apr 21, 2024
  21. Junio C HamanoApr 22, 2024
  22. phillip.wood123@gmail.comApr 24, 2024
  23. Junio C HamanoApr 24, 2024
  24. Phillip WoodApr 29, 2024
  25. Junio C HamanoApr 29, 2024
  26. Jeff KingApr 25, 2024
  27. Eric SunshineApr 25, 2024
  28. Junio C HamanoApr 25, 2024
  29. Eric SunshineApr 25, 2024
  30. Junio C HamanoApr 25, 2024
  31. Junio C HamanoApr 25, 2024
  32. Rubén JustoApr 25, 2024
  33. Junio C HamanoApr 25, 2024
  34. Rubén JustoApr 25, 2024
  35. Junio C HamanoApr 26, 2024
  36. Junio C HamanoApr 26, 2024
  37. Jeff KingApr 26, 2024
  38. Rubén JustoApr 25, 2024
  39. Jeff KingApr 26, 2024
  40. Rubén JustoApr 26, 2024
  41. phillip.wood123@gmail.comApr 25, 2024
  42. 0/2 add-patch: response to unknown commandRubén Justo, Apr 29, 2024
  43. 1/2 add-patch: do not show UI messages on stderrRubén Justo, Apr 29, 2024
  44. Junio C HamanoApr 29, 2024
  45. Jeff KingApr 30, 2024
  46. Rubén JustoApr 30, 2024
  47. Junio C HamanoApr 30, 2024
  48. Junio C HamanoApr 30, 2024
  49. phillip.wood123@gmail.comApr 30, 2024
  50. Rubén JustoApr 30, 2024
  51. phillip.wood123@gmail.comMay 1, 2024
  52. Junio C HamanoMay 1, 2024
  53. Rubén JustoMay 1, 2024
  54. Junio C HamanoMay 2, 2024
  55. 2/2 add-patch: response to unknown commandRubén Justo, Apr 29, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.