| Message ID | 35ef0ee2b9925748ae3b6b6ac68e79c71782e5cf.1740149837.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add -p: a couple of hunk splitting fixes | expand |
On 25/02/21 02:57PM, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the users 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. If I'm understanding this correctly, the issue here is that, when a patch is editted, the number of hunks in can be split into may change, but is not reevaluated. This could lead to issue if the editted hunk is subsequently split. This issue would also apply to addition lines being changed to context lines as well correct? > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > add-patch.c | 12 +++++++++++- > t/t3701-add-interactive.sh | 21 +++++++++++++++++++++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/add-patch.c b/add-patch.c > index f44f98275cc..982745373df 100644 > --- a/add-patch.c > +++ b/add-patch.c > @@ -1182,19 +1182,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; > } Ok with this we now recount the potential number of hunks a hunk can be split into. Whenever there is a change from a context line to a addition or deletion line, a separate hunk could be formed. -Justin
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > When the users 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. Heh, when I did the original "add -i/-p", I said "it is doubtful that a user who has selected a hunk will split it afterwards" ;-) > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > add-patch.c | 12 +++++++++++- > t/t3701-add-interactive.sh | 21 +++++++++++++++++++++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/add-patch.c b/add-patch.c > index f44f98275cc..982745373df 100644 > --- a/add-patch.c > +++ b/add-patch.c > @@ -1182,19 +1182,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; > } OK. > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index 760f3d0d30f..cb81bfe64c8 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -1240,4 +1240,25 @@ test_expect_success 'splitting previous hunk marks split hunks as undecided' ' > 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 && Missing SP before ">file" on the earlier line. > + ( > + test_set_editor "$(pwd)/fake-editor.sh" && > + test_write_lines e K s j y n y q | 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
diff --git a/add-patch.c b/add-patch.c index f44f98275cc..982745373df 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1182,19 +1182,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 760f3d0d30f..cb81bfe64c8 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1240,4 +1240,25 @@ test_expect_success 'splitting previous hunk marks split hunks as undecided' ' 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" && + test_write_lines e K s j y n y q | 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