Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH v3 2/2] add-patch: do not print hunks repeatedly
Date: Thu, 28 Mar 2024 02:12:13 +0100	[thread overview]
Message-ID: <60c43160-1b60-42f6-9488-4cc332201b7e@gmail.com> (raw)
In-Reply-To: <60d978d0-f69a-4b65-b4ba-d30dac8f112a@gmail.com>

The interactive-patch is a sequential process where, on each step, we
print one hunk from a patch and then ask the user how to proceed.

There is a possibility of repeating a step, for example if the user
enters a non-applicable option, i.e: "s"

    $ git add -p
    diff --git a/add-patch.c b/add-patch.c
    index 52be1ddb15..8fb75e82e2 100644
    --- a/add-patch.c
    +++ b/add-patch.c
    @@ -1394,7 +1394,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
     static int patch_update_file(struct add_p_state *s,
     			     struct file_diff *file_diff)
     {
    -	size_t hunk_index = 0;
    +	size_t hunk_index = 0, prev_hunk_index = -1;
     	ssize_t i, undecided_previous, undecided_next;
     	struct hunk *hunk;
     	char ch;
    (1/4) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? s
    Sorry, cannot split this hunk
    @@ -1394,7 +1394,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
     static int patch_update_file(struct add_p_state *s,
     			     struct file_diff *file_diff)
     {
    -	size_t hunk_index = 0;
    +	size_t hunk_index = 0, prev_hunk_index = -1;
     	ssize_t i, undecided_previous, undecided_next;
     	struct hunk *hunk;
     	char ch;
    (1/4) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?

... or an invalid option, i.e: "U"

    $ git add -p
    diff --git a/add-patch.c b/add-patch.c
    index 52be1ddb15..8fb75e82e2 100644
    --- a/add-patch.c
    +++ b/add-patch.c
    @@ -1394,7 +1394,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
     static int patch_update_file(struct add_p_state *s,
     			     struct file_diff *file_diff)
     {
    -	size_t hunk_index = 0;
    +	size_t hunk_index = 0, prev_hunk_index = -1;
     	ssize_t i, undecided_previous, undecided_next;
     	struct hunk *hunk;
     	char ch;
    (1/4) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? U
    y - stage this hunk
    n - do not stage this hunk
    q - quit; do not stage this hunk or any of the remaining ones
    a - stage this hunk and all later hunks in the file
    d - do not stage this hunk or any of the later hunks in the file
    j - leave this hunk undecided, see next undecided hunk
    J - leave this hunk undecided, see next hunk
    g - select a hunk to go to
    / - search for a hunk matching the given regex
    e - manually edit the current hunk
    p - print again the current hunk
    ? - print help
    @@ -1394,7 +1394,7 @@ N_("j - leave this hunk undecided, see next undecided hunk\n"
     static int patch_update_file(struct add_p_state *s,
     			     struct file_diff *file_diff)
     {
    -	size_t hunk_index = 0;
    +	size_t hunk_index = 0, prev_hunk_index = -1;
     	ssize_t i, undecided_previous, undecided_next;
     	struct hunk *hunk;
     	char ch;
    (1/4) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?

Printing the chunk again followed by the question can be confusing as
the user has to pay special attention to notice that the same chunk is
being reconsidered.

It can also be problematic if the chunk is longer than one screen height
because the result of the previous iteration is lost off the screen (the
help guide in the previous example).

To avoid such problems, stop printing the chunk if the iteration does
not advance to a different chunk.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 add-patch.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 922c43378e..e0f4cd9b9b 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1395,7 +1395,7 @@ static int patch_update_file(struct add_p_state *s,
 			     struct file_diff *file_diff)
 {
 	size_t hunk_index = 0;
-	ssize_t i, undecided_previous, undecided_next;
+	ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1;
 	struct hunk *hunk;
 	char ch;
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -1448,10 +1448,15 @@ static int patch_update_file(struct add_p_state *s,
 
 		strbuf_reset(&s->buf);
 		if (file_diff->hunk_nr) {
-			render_hunk(s, hunk, 0, colored, &s->buf);
-			fputs(s->buf.buf, stdout);
+			if (rendered_hunk_index != hunk_index) {
+				render_hunk(s, hunk, 0, colored, &s->buf);
+				fputs(s->buf.buf, stdout);
+
+				rendered_hunk_index = hunk_index;
+			}
 
 			strbuf_reset(&s->buf);
+
 			if (undecided_previous >= 0) {
 				permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK;
 				strbuf_addstr(&s->buf, ",k");
@@ -1649,10 +1654,12 @@ static int patch_update_file(struct add_p_state *s,
 			if (!(permitted & ALLOW_SPLIT))
 				err(s, _("Sorry, cannot split this hunk"));
 			else if (!split_hunk(s, file_diff,
-					     hunk - file_diff->hunk))
+					     hunk - file_diff->hunk)) {
 				color_fprintf_ln(stdout, s->s.header_color,
 						 _("Split into %d hunks."),
 						 (int)splittable_into);
+				rendered_hunk_index = -1;
+			}
 		} else if (s->answer.buf[0] == 'e') {
 			if (!(permitted & ALLOW_EDIT))
 				err(s, _("Sorry, cannot edit this hunk"));
@@ -1661,7 +1668,7 @@ static int patch_update_file(struct add_p_state *s,
 				goto soft_increment;
 			}
 		} else if (s->answer.buf[0] == 'p') {
-			/* nothing special is needed */
+			rendered_hunk_index = -1;
 		} else {
 			const char *p = _(help_patch_remainder), *eol = p;
 
-- 
2.44.0.370.gd7ad5e5424

  parent reply	other threads:[~2024-03-28  1:12 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-25 20:59 [PATCH 0/2] improve interactive-patch Rubén Justo
2024-03-25 21:05 ` [PATCH 1/2] add-patch: introduce 'p' in interactive-patch Rubén Justo
2024-03-25 21:38   ` Junio C Hamano
2024-03-25 23:15     ` Rubén Justo
2024-03-25 23:42       ` Junio C Hamano
2024-03-25 21:07 ` [PATCH 2/2] add-patch: do not print hunks repeatedly Rubén Justo
2024-03-25 21:34   ` Junio C Hamano
2024-03-26  0:15 ` [PATCH v2 0/2] improve interactive-patch Rubén Justo
2024-03-26  0:17   ` [PATCH v2 1/2] add-patch: introduce 'p' in interactive-patch Rubén Justo
2024-03-26 14:38     ` Phillip Wood
2024-03-26 18:40       ` Rubén Justo
2024-03-27 10:55         ` Phillip Wood
2024-03-26  0:17   ` [PATCH v2 2/2] add-patch: do not print hunks repeatedly Rubén Justo
2024-03-26 14:39     ` Phillip Wood
2024-03-26 18:46       ` Rubén Justo
2024-03-27 11:06         ` Phillip Wood
2024-03-28  0:39           ` Rubén Justo
2024-03-26 14:37   ` [PATCH v2 0/2] improve interactive-patch Phillip Wood
2024-03-26 15:31     ` Junio C Hamano
2024-03-26 18:48       ` Rubén Justo
2024-03-26 19:13         ` Junio C Hamano
2024-03-26 20:26           ` Rubén Justo
2024-03-29 19:26           ` Rubén Justo
2024-03-29 19:48             ` Dragan Simic
2024-03-30 13:49               ` Rubén Justo
2024-03-30 17:06             ` Junio C Hamano
2024-03-27 11:14       ` Phillip Wood
2024-03-27 15:43         ` Junio C Hamano
2024-03-27 16:14           ` Phillip Wood
2024-03-28  1:03           ` Rubén Justo
2024-03-26 18:46     ` Rubén Justo
2024-03-28  1:10   ` [PATCH v3 " Rubén Justo
2024-03-28  1:12     ` [PATCH v3 1/2] add-patch: introduce 'p' in interactive-patch Rubén Justo
2024-03-28 14:45       ` Junio C Hamano
2024-03-28  1:12     ` Rubén Justo [this message]
2024-03-28 14:46       ` [PATCH v3 2/2] add-patch: do not print hunks repeatedly Junio C Hamano
2024-03-29  3:49         ` Rubén Justo
2024-03-29  3:56     ` [PATCH v4 0/2] improve interactive-patch Rubén Justo
2024-03-29  3:58       ` [PATCH v4 1/2] add-patch: introduce 'p' in interactive-patch Rubén Justo
2024-03-29  3:58       ` [PATCH v4 2/2] add-patch: do not print hunks repeatedly Rubén Justo
2024-03-29 10:41         ` phillip.wood123
2024-03-29 11:37           ` 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=60c43160-1b60-42f6-9488-4cc332201b7e@gmail.com \
    --to=rjusto@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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 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).