From 3be7c73df7ef389c1793d48989f777572a757e55 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 18 Jul 2024 21:37:16 +0100 Subject: [PATCH 1/2] add -p: mark split hunks as undecided When a hunk is split each of the new hunks inherits whether it is selected or not from the original hunk. If a selected hunk is split all of the new hunks are selected and the user is only prompted with the first of the split hunks, they are not asked whether or not they want to select the rest of the new hunks. If they want to deselect one of the other new hunks they have to navigate back to it to do that. This is unfortunate as the user is presumably splitting the original hunk because they only want to select some sub-set of it. Instead mark all the new hunks as "undecided" so that we prompt the user to decide whether to select them or not. In the case where the user only wants to change the selection of the first of the split hunks they will now have to do more work re-selecting the remaining split hunks. However changing the selection of any of the other newly created hunks is now much simpler as the user no-longer has to navigate back to them before being able to change their selected state. Due to concerns that users may be relying on the current behaviour [1] this change is guarded by WITH_BREAKING_CHANGES. [1] https://lore.kernel.org/git/xmqqjz9b6xr1.fsf@gitster.g Signed-off-by: Phillip Wood --- Documentation/BreakingChanges.adoc | 5 +++++ add-patch.c | 7 +++++++ t/t3701-add-interactive.sh | 10 ++++++++++ 3 files changed, 22 insertions(+) diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc index c6bd94986c5fcc..596f2a224b3df8 100644 --- a/Documentation/BreakingChanges.adoc +++ b/Documentation/BreakingChanges.adoc @@ -118,6 +118,11 @@ Cf. <2f5de416-04ba-c23d-1e0b-83bb655829a7@zombino.com>, <20170223155046.e7nxivfwqqoprsqj@LykOS.localdomain>, . +* The behavior of "git add -p" has been changed so that splitting a + hunk that has already been marked as selected or unselected will now + prompt the user to select each of the new hunks created by the + split instead of them inheriting their state from the original hunk. + === Removals * Support for grafting commits has long been superseded by git-replace(1). diff --git a/add-patch.c b/add-patch.c index 95c67d8c80c4f3..b1c104af3d5a38 100644 --- a/add-patch.c +++ b/add-patch.c @@ -953,6 +953,9 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, * sizeof(*hunk)); hunk = file_diff->hunk + hunk_index; hunk->splittable_into = 1; +#ifdef WITH_BREAKING_CHANGES + hunk->use = UNDECIDED_HUNK; +#endif memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk)); header = &hunk->header; @@ -1054,7 +1057,11 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, hunk++; hunk->splittable_into = 1; +#ifdef WITH_BREAKING_CHANGES + hunk->use = UNDECIDED_HUNK; +#else hunk->use = hunk[-1].use; +#endif header = &hunk->header; header->old_count = header->new_count = context_line_count; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index b8a05d95f3f130..57d12c89a56745 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1230,4 +1230,14 @@ test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' ' test_cmp expect actual ' +test_expect_success WITH_BREAKING_CHANGES 'splitting previous hunk marks split hunks as undecided' ' + test_write_lines a " " b c d e f g h i j k >file && + git add file && + test_write_lines x " " b y d e f g h i j x >file && + test_write_lines n K s n y q | git add -p file && + git cat-file blob :file >actual && + test_write_lines a " " b y d e f g h i j k >expect && + test_cmp expect actual +' + test_done From e0b52614f7dd1b181307339cda834b4c0d332abe Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Sat, 20 Jul 2024 14:49:25 +0100 Subject: [PATCH 2/2] add-patch: update hunk splitability after editing When the user edits a hunk if they change deletion lines to context lines or vice versa then the number of hunks that the edited hunk can be split into may differ from the unedited hunk and so we need to update hunk->splittable_into. In practice users are unlikely to hit this bug as it is doubtful that a user who has edited a hunk will split it afterwards. Signed-off-by: Phillip Wood --- add-patch.c | 12 +++++++++++- t/t3701-add-interactive.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index b1c104af3d5a38..dd166f7e075fcf 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1188,19 +1188,29 @@ static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk, { struct hunk_header *header = &hunk->header; size_t i; + char ch, marker = ' '; + hunk->splittable_into = 0; header->old_count = header->new_count = 0; for (i = hunk->start; i < hunk->end; ) { - switch(normalize_marker(&s->plain.buf[i])) { + ch = normalize_marker(&s->plain.buf[i]); + switch (ch) { case '-': header->old_count++; + if (marker == ' ') + hunk->splittable_into++; + marker = ch; break; case '+': header->new_count++; + if (marker == ' ') + hunk->splittable_into++; + marker = ch; break; case ' ': header->old_count++; header->new_count++; + marker = ch; break; } diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 57d12c89a56745..013bbb529f299c 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1240,4 +1240,30 @@ test_expect_success WITH_BREAKING_CHANGES 'splitting previous hunk marks split h test_cmp expect actual ' +test_expect_success 'splitting edited hunk' ' + # Before the first hunk is edited it can be split into two + # hunks, after editing it can be split into three hunks. + + write_script fake-editor.sh <<-\EOF && + sed "s/^ c/-c/" "$1" >"$1.tmp" && + mv "$1.tmp" "$1" + EOF + + test_write_lines a b c d e f g h i j k l m n >file && + git add file && + test_write_lines A b c d E f g h i j k l M n >file && + ( + test_set_editor "$(pwd)/fake-editor.sh" && + if test_have_prereq WITH_BREAKING_CHANGES + then + test_write_lines e K s j y n y q + else + test_write_lines e K s n K n y q + fi | git add -p file + ) && + git cat-file blob :file >actual && + test_write_lines a b d e f g h i j k l M n >expect && + test_cmp expect actual +' + test_done