[PATCH] add-patch: edit the hunk again

["Rubén Justo" <rjusto@gmail.com>]

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

If the modified hunk returned is not an applicable patch, we give the
opportunity to try again.

For this new attempt we provide, again, the original hunk;  the user
has to repeat the modification from scratch.

Instead, let's give them the faulty 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 current hunk and moving on to the next one.

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

      Now makes perfect sense not to move to the next hunk when the
      user requests to discard their edits.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---

The message "saying 'no' discards!" comes from ac083c47ea
(git-add--interactive: manual hunk editing mode, 2008-07-03).

I think it was referring to discarding user modifications, not the
current hunk; which is what we were doing then (and now regaining).

However, we stopped behaving that way in 2b8ea7f3c7 (add -p:
calculate offset delta for edited patches, 2018-03-05), perhaps for
some reason I'm missing.

Therefore, this patch also modifies what we did, possibly
unintentionally, in 2b8ea7f3c7.

Thanks.


 add-patch.c                | 26 ++++++++++++++++----------
 t/t3701-add-interactive.sh | 14 ++++++++++++++
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 557903310d..125e79a5ae 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; ) {
@@ -1257,15 +1262,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 +1277,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]
@@ -1289,8 +1289,14 @@ static int edit_hunk_loop(struct add_p_state *s,
 					"Edit again (saying \"no\" discards!) "
 					"[y/n]? "));
 		if (res < 1)
-			return -1;
+			break;
 	}
+
+	/* Drop a possible edit */
+	strbuf_setlen(&s->plain, plain_len);
+	strbuf_setlen(&s->colored, colored_len);
+	*hunk = backup;
+	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..6af5636221 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -165,6 +165,20 @@ test_expect_success 'dummy edit works' '
 	diff_cmp expected diff
 '
 
+test_expect_success 'setup re-edit editor' '
+	write_script "fake_editor.sh" <<-\EOF &&
+	grep been-here "$1" && echo found >output
+	echo been-here > "$1"
+	EOF
+	test_set_editor "$(pwd)/fake_editor.sh"
+'
+
+test_expect_success 'editing again works' '
+	git reset &&
+	test_write_lines e y | GIT_TRACE=1 git add -p &&
+	grep found output
+'
+
 test_expect_success 'setup patch' '
 	cat >patch <<-\EOF
 	@@ -1,1 +1,4 @@
-- 
2.46.1.507.gbcf32d0979