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

Re: [PATCH 1/2] add-patch: introduce 'p' in interactive-patch

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 25, 2024, 21:38 UTC
Message-ID
<xmqqcyrigfft.fsf@gitster.g>
In-Reply-To
<fa64a975-40e4-40f2-bdcf-fd2da4fc506e@gmail.com>
Rubén Justo <rjusto@gmail.com> writes:
Show 5 quoted lines
> Shortly we're going make interactive-patch stop printing automatically
> the hunk under certain circumstances.
>
> Let's introduce a new option to allow the user to explicitly request
> the printing.
That is good, but ...
Show 32 quoted lines
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
>  add-patch.c                |  4 ++++
>  t/t3701-add-interactive.sh | 22 +++++++++++-----------
>  2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/add-patch.c b/add-patch.c
> index 68f525b35c..52be1ddb15 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1388,6 +1388,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
>     "/ - search for a hunk matching the given regex\n"
>     "s - split the current hunk into smaller hunks\n"
>     "e - manually edit the current hunk\n"
> +   "p - print again the current hunk\n"
>     "? - print help\n");
>  
>  static int patch_update_file(struct add_p_state *s,
> @@ -1480,6 +1481,7 @@ static int patch_update_file(struct add_p_state *s,
>  				permitted |= ALLOW_EDIT;
>  				strbuf_addstr(&s->buf, ",e");
>  			}
> +			strbuf_addstr(&s->buf, ",p");
>  		}
>  		if (file_diff->deleted)
>  			prompt_mode_type = PROMPT_DELETION;
> @@ -1658,6 +1660,8 @@ static int patch_update_file(struct add_p_state *s,
>  				hunk->use = USE_HUNK;
>  				goto soft_increment;
>  			}
> +		} else if (s->answer.buf[0] == 'p') {
> +			/* nothing to do */

This is not good. If we are taking a new input, why doesn't the code already respond to it? "Showing it again" should be a separate feature even if some other codepaths still do show when [2/2] would prevent them to show, no?

Also, in addition to the changes to the patch to unbreak it, we'd need to update the git-add(1) manual page, especially its section on the interactive mode. I think a single-line addition should suffice, but it has to be there, added by the same patch as the one that starts accepting 'p' and acting on that input.

Thanks.
Show 73 quoted lines
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 0b5339ac6c..bc55255b0a 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -325,9 +325,9 @@ test_expect_success 'different prompts for mode change/deleted' '
>  	git -c core.filemode=true add -p >actual &&
>  	sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered &&
>  	cat >expect <<-\EOF &&
> -	(1/1) Stage deletion [y,n,q,a,d,?]?
> -	(1/2) Stage mode change [y,n,q,a,d,j,J,g,/,?]?
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]?
> +	(1/1) Stage deletion [y,n,q,a,d,p,?]?
> +	(1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]?
> +	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]?
>  	EOF
>  	test_cmp expect actual.filtered
>  '
> @@ -514,13 +514,13 @@ test_expect_success 'split hunk setup' '
>  test_expect_success 'goto hunk' '
>  	test_when_finished "git reset" &&
>  	tr _ " " >expect <<-EOF &&
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 1:  -1,2 +1,3          +15
> +	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1:  -1,2 +1,3          +15
>  	_ 2:  -2,4 +3,8          +21
>  	go to which hunk? @@ -1,2 +1,3 @@
>  	_10
>  	+15
>  	_20
> -	(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_
> +	(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
>  	EOF
>  	test_write_lines s y g 1 | git add -p >actual &&
>  	tail -n 7 <actual >actual.trimmed &&
> @@ -530,11 +530,11 @@ test_expect_success 'goto hunk' '
>  test_expect_success 'navigate to hunk via regex' '
>  	test_when_finished "git reset" &&
>  	tr _ " " >expect <<-EOF &&
> -	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -1,2 +1,3 @@
> +	(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@
>  	_10
>  	+15
>  	_20
> -	(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_
> +	(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_
>  	EOF
>  	test_write_lines s y /1,2 | git add -p >actual &&
>  	tail -n 5 <actual >actual.trimmed &&
> @@ -715,21 +715,21 @@ test_expect_success 'colors can be overridden' '
>  	<BLUE>+<RESET><BLUE>new<RESET>
>  	<CYAN> more-context<RESET>
>  	<BLUE>+<RESET><BLUE>another-one<RESET>
> -	<YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
> +	<YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,p,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
>  	<MAGENTA>@@ -1,3 +1,3 @@<RESET>
>  	<CYAN> context<RESET>
>  	<BOLD>-old<RESET>
>  	<BLUE>+<RESET><BLUE>new<RESET>
>  	<CYAN> more-context<RESET>
> -	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
> +	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
>  	<CYAN> more-context<RESET>
>  	<BLUE>+<RESET><BLUE>another-one<RESET>
> -	<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> +	<YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
>  	<CYAN> context<RESET>
>  	<BOLD>-old<RESET>
>  	<BLUE>+new<RESET>
>  	<CYAN> more-context<RESET>
> -	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? <RESET>
> +	<YELLOW>(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? <RESET>
>  	EOF
>  	test_cmp expect actual
>  '
Previous: Rubén JustoNext: Rubén Justo
Message 3 of 42 in “improve interactive-patch”
  1. 0/2 improve interactive-patchRubén Justo, Mar 25, 2024
  2. 1/2 add-patch: introduce 'p' in interactive-patchRubén Justo, Mar 25, 2024
  3. Junio C HamanoMar 25, 2024
  4. Rubén JustoMar 25, 2024
  5. Junio C HamanoMar 25, 2024
  6. 2/2 add-patch: do not print hunks repeatedlyRubén Justo, Mar 25, 2024
  7. Junio C HamanoMar 25, 2024
  8. 0/2 improve interactive-patchRubén Justo, Mar 26, 2024
  9. 1/2 add-patch: introduce 'p' in interactive-patchRubén Justo, Mar 26, 2024
  10. Phillip WoodMar 26, 2024
  11. Rubén JustoMar 26, 2024
  12. Phillip WoodMar 27, 2024
  13. 2/2 add-patch: do not print hunks repeatedlyRubén Justo, Mar 26, 2024
  14. Phillip WoodMar 26, 2024
  15. Rubén JustoMar 26, 2024
  16. Phillip WoodMar 27, 2024
  17. Rubén JustoMar 28, 2024
  18. Phillip WoodMar 26, 2024
  19. Junio C HamanoMar 26, 2024
  20. Rubén JustoMar 26, 2024
  21. Junio C HamanoMar 26, 2024
  22. Rubén JustoMar 26, 2024
  23. Rubén JustoMar 29, 2024
  24. Dragan SimicMar 29, 2024
  25. Rubén JustoMar 30, 2024
  26. Junio C HamanoMar 30, 2024
  27. Phillip WoodMar 27, 2024
  28. Junio C HamanoMar 27, 2024
  29. Phillip WoodMar 27, 2024
  30. Rubén JustoMar 28, 2024
  31. Rubén JustoMar 26, 2024
  32. 0/2 improve interactive-patchRubén Justo, Mar 28, 2024
  33. 1/2 add-patch: introduce 'p' in interactive-patchRubén Justo, Mar 28, 2024
  34. Junio C HamanoMar 28, 2024
  35. 2/2 add-patch: do not print hunks repeatedlyRubén Justo, Mar 28, 2024
  36. Junio C HamanoMar 28, 2024
  37. Rubén JustoMar 29, 2024
  38. 0/2 improve interactive-patchRubén Justo, Mar 29, 2024
  39. 1/2 add-patch: introduce 'p' in interactive-patchRubén Justo, Mar 29, 2024
  40. 2/2 add-patch: do not print hunks repeatedlyRubén Justo, Mar 29, 2024
  41. phillip.wood123@gmail.comMar 29, 2024
  42. Rubén JustoMar 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.