Rubén JustoThe "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(-)
Show changes to 2 files +30 −10
add-patch.c, t/t3701-add-interactive.sh
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
