[v3] add-patch: edit the hunk again

Commit Message

Rubén Justo Sept. 28, 2024, 2:30 p.m. UTC

The "edit" option allows the user to directly modify the hunk to be
applied.

If the modified hunk returned by the user is not an applicable patch,
they will be given the opportunity to try again.

For this new attempt we give them the original hunk;  they have to
repeat the modification from scratch.

Instead, let's give them the modified patch back, so they can identify
and fix the problem.

If they really want to start over with a fresh patch they still can
say 'no', to cancel the "edit" and start anew [*].

    * In the old script-based version of "add -p", this "no" meant
      discarding the hunk and moving on to the next one.

      This changed, probably unintentionally, during its conversion to
      C in bcdd297b78 (built-in add -p: implement hunk editing,
      2019-12-13).

      It now makes more sense not to move to the next block when the
      user requests to discard their edits.

Signed-off-by: Rubén Justo <[e-mail]>
---

In this iteration, I'm modifying the message 'saying no discards' to
make its meaning more explicit, perhaps also gaining some clarity
along the way for the user who wants to restart editing from the
original patch.

In the test, I'm adding "n q" to the script as suggested by Phillip.

And, just a bit of mental peace by restoring the hunk from the backup
before trimming the two strbuf.

Thanks.

 add-patch.c                | 33 ++++++++++++++++++++-------------
 t/t3701-add-interactive.sh | 13 +++++++++++++
 2 files changed, 33 insertions(+), 13 deletions(-)


Range-diff against v2:
1:  2b55a759d5 ! 1:  7e76606751 add-patch: edit the hunk again
    @@ add-patch.c: static int edit_hunk_loop(struct add_p_state *s,
      		/*
      		 * TRANSLATORS: do not translate [y/n]
     @@ add-patch.c: static int edit_hunk_loop(struct add_p_state *s,
    - 					"Edit again (saying \"no\" discards!) "
    + 		 * of the word "no" does not start with n.
    + 		 */
    + 		res = prompt_yesno(s, _("Your edited hunk does not apply. "
    +-					"Edit again (saying \"no\" discards!) "
    ++					"Edit again (saying \"no\" discards your edits!) "
      					"[y/n]? "));
      		if (res < 1)
     -			return -1;
    @@ add-patch.c: static int edit_hunk_loop(struct add_p_state *s,
      	}
     +
     +	/* Drop a possible edit */
    ++	*hunk = backup;
     +	strbuf_setlen(&s->plain, plain_len);
     +	strbuf_setlen(&s->colored, colored_len);
    -+	*hunk = backup;
     +	return -1;
      }
      
    @@ t/t3701-add-interactive.sh: test_expect_success 'dummy edit works' '
     +	EOF
     +	(
     +		test_set_editor "$(pwd)/fake_editor.sh" &&
    -+		test_write_lines e y | GIT_TRACE=1 git add -p
    ++		test_write_lines e y n q | GIT_TRACE=1 git add -p
     +	) &&
     +	test_grep been-here output
     +'

Patch

diff --git a/add-patch.c b/add-patch.c
index 557903310d..c847b4a59d 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1111,7 +1111,8 @@  static void recolor_hunk(struct add_p_state *s, struct hunk *hunk)
 	hunk->colored_end = s->colored.len;
 }
 
-static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
+static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk,
+			      size_t plain_len, size_t colored_len)
 {
 	size_t i;
 
@@ -1146,6 +1147,10 @@  static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 				      "addp-hunk-edit.diff", NULL) < 0)
 		return -1; 
 
+	/* Drop possible previous edits */
+	strbuf_setlen(&s->plain, plain_len);
+	strbuf_setlen(&s->colored, colored_len);
+
 	/* strip out commented lines */
 	hunk->start = s->plain.len;
 	for (i = 0; i < s->buf.len; ) {
@@ -1157,12 +1162,13 @@  static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
 	}
 
 	hunk->end = s->plain.len;
+
+	recolor_hunk(s, hunk);
+
 	if (hunk->end == hunk->start)
 		/* The user aborted editing by deleting everything */
 		return 0;
 
-	recolor_hunk(s, hunk);
-
 	/*
 	 * If the hunk header is intact, parse it, otherwise simply use the
 	 * hunk header prior to editing (which will adjust `hunk->start` to
@@ -1257,15 +1263,14 @@  static int edit_hunk_loop(struct add_p_state *s,
 	backup = *hunk;
 
 	for (;;) {
-		int res = edit_hunk_manually(s, hunk);
+		int res = edit_hunk_manually(s, hunk, plain_len, colored_len);
 		if (res == 0) {
 			/* abandoned */
-			*hunk = backup;
-			return -1;
+			break;
 		}
 
 		if (res > 0) {
-			hunk->delta +=
+			hunk->delta = backup.delta +
 				recount_edited_hunk(s, hunk,
 						    backup.header.old_count,
 						    backup.header.new_count);
@@ -1273,10 +1278,6 @@  static int edit_hunk_loop(struct add_p_state *s,
 				return 0;
 		}
 
-		/* Drop edits (they were appended to s->plain) */
-		strbuf_setlen(&s->plain, plain_len);
-		strbuf_setlen(&s->colored, colored_len);
-		*hunk = backup;
 
 		/*
 		 * TRANSLATORS: do not translate [y/n]
@@ -1286,11 +1287,17 @@  static int edit_hunk_loop(struct add_p_state *s,
 		 * of the word "no" does not start with n.
 		 */
 		res = prompt_yesno(s, _("Your edited hunk does not apply. "
-					"Edit again (saying \"no\" discards!) "
+					"Edit again (saying \"no\" discards your edits!) "
 					"[y/n]? "));
 		if (res < 1)
-			return -1;
+			break;
 	}
+
+	/* Drop a possible edit */
+	*hunk = backup;
+	strbuf_setlen(&s->plain, plain_len);
+	strbuf_setlen(&s->colored, colored_len);
+	return -1;
 }
 
 static int apply_for_checkout(struct add_p_state *s, struct strbuf *diff,
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 718438ffc7..1ceefd96e6 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -165,6 +165,19 @@  test_expect_success 'dummy edit works' '
 	diff_cmp expected diff
 '
 
+test_expect_success 'editing again works' '
+	git reset &&
+	write_script "fake_editor.sh" <<-\EOF &&
+	grep been-here "$1" >output
+	echo been-here >"$1"
+	EOF
+	(
+		test_set_editor "$(pwd)/fake_editor.sh" &&
+		test_write_lines e y n q | GIT_TRACE=1 git add -p
+	) &&
+	test_grep been-here output
+'
+
 test_expect_success 'setup patch' '
 	cat >patch <<-\EOF
 	@@ -1,1 +1,4 @@