{"thread":{"id":"47825","subject":"[PATCH 1/4] add -i: add function to format hunk header","startedAt":"2018-02-13T10:44:29Z","lastAt":"2018-03-06T10:25:48Z","messageCount":79,"participants":["Phillip Wood","Junio C Hamano","brian m. carlson","Eric Sunshine","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"339183","messageId":"20180213104408.9887-2-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH 1/4] add -i: add function to format hunk header","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:44:05Z","receivedAt":"2018-02-13T10:44:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis code is duplicated in a couple of places so make it into a\nfunction as we're going to add some more callers shortly.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -751,6 +751,15 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" .\n+\t\t(($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\" +$n_ofs\" .\n+\t\t(($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -838,11 +847,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -912,11 +917,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n-- \n2.16.1\n\n"},{"id":"339184","messageId":"20180213104408.9887-1-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":null,"subject":"[PATCH 0/4] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:44:04Z","receivedAt":"2018-02-13T10:44:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhile working on a patch series to stage selected lines from a hunk\nwithout having to edit it I got worried that subsequent patches would\nbe applied in the wrong place which lead to this series to correct the\noffsets of hunks following those that are skipped or edited.\n\nPhillip Wood (4):\n add -i: add function to format hunk header\n t3701: add failing test for pathological context lines\n add -p: Adjust offsets of subsequent hunks when one is skipped\n add -p: calculate offset delta for edited patches\n\n git-add--interactive.perl | 93 +++++++++++++++++++++++++++++++++++-----------\n t/t3701-add-interactive.sh | 30 +++++++++++++++\n 2 files changed, 102 insertions(+), 21 deletions(-)\n\n-- \n2.16.1\n\n"},{"id":"339185","messageId":"20180213104408.9887-4-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH 3/4] add -p: Adjust offsets of subsequent hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:44:07Z","receivedAt":"2018-02-13T10:44:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince commit 8cbd431082 (\"git-add--interactive: replace hunk\nrecounting with apply --recount\", 2008-7-2) if a hunk is skipped then\nwe rely on the context lines to apply subsequent hunks in the right\nplace. While this works most of the time it is possible for hunks to\nend up being applied in the wrong place. To fix this adjust the offset\nof subsequent hunks to correct for any change in the number of\ninsertions or deletions due to the skipped hunk. The change in offset\ndue to edited hunks that have the number of insertions or deletions\nchanged is ignored here, it will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 +++++++++++++--\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8ababa6453ac4f57a925aacbb8b9bb1c973e4a54..7a0a5896bb8fa063ace29b85840849c867b874f5 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {\n \tmy @out = ();\n \n \tmy ($last_o_ctx, $last_was_dirty);\n+\tmy $ofs_delta = 0;\n \n-\tfor (grep { $_->{USE} } @in) {\n+\tfor (@in) {\n \t\tif ($_->{TYPE} ne 'hunk') {\n \t\t\tpush @out, $_;\n \t\t\tnext;\n \t\t}\n \t\tmy $text = $_->{TEXT};\n-\t\tmy ($o_ofs) = parse_hunk_header($text->[0]);\n+\t\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =\n+\t\t\t\t\t\tparse_hunk_header($text->[0]);\n+\t\tunless ($_->{USE}) {\n+\t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($ofs_delta) {\n+\t\t\t$n_ofs += $ofs_delta;\n+\t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t $n_ofs, $n_cnt);\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 6838698a1382b24724cfbd3be04a7054489b94af..5401d74d5804b5e43286d08a905fb96c68b02e42 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -555,7 +555,7 @@ test_expect_success 'set up pathological context' '\n \tEOF\n '\n \n-test_expect_failure 'add -p works with pathological context lines' '\n+test_expect_success 'add -p works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" n y |\n \tgit add -p &&\n-- \n2.16.1\n\n"},{"id":"339186","messageId":"20180213104408.9887-5-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH 4/4] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:44:08Z","receivedAt":"2018-02-13T10:44:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRecount the number of preimage and postimage lines in a hunk after it\nhas been edited so any change in the number of insertions or deletions\ncan be used to adjust the offsets of subsequent hunks. If an edited\nhunk is subsequently split then the offset correction will be lost. It\nwould be possible to fix this if it is a problem, however the code\nhere is still an improvement on the status quo for the common case\nwhere an edited hunk is applied without being split.\n\nThis is also a necessary step to removing '--recount' and\n'--allow-overlap' from the invocation of 'git apply'. Before\n'--recount' can be removed the splitting and coalescing counting needs\nto be fixed to handle a missing newline at the end of a file. In order\nto remove '--allow-overlap' there needs to be i) some way of verifying\nthe offset data in the edited hunk (probably by correlating the\npreimage (or postimage if the patch is going to be applied in reverse)\nlines of the edited and unedited versions to see if they are offset or\nif any leading/trailing context lines have been removed) and ii) a way of\ndealing with edited hunks that change context lines that are shared\nwith neighbouring hunks.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 57 ++++++++++++++++++++++++++++++++++++++--------\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 49 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7a0a5896bb8fa063ace29b85840849c867b874f5..f139e545a4fb9cbaa757208a44cb2b5c3ad3678a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -938,6 +938,9 @@ sub coalesce_overlapping_hunks {\n \t\t\t\t\t\tparse_hunk_header($text->[0]);\n \t\tunless ($_->{USE}) {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\t# If this hunk has been edited then subtract\n+\t\t\t# the delta that is due to the edit.\n+\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n@@ -945,6 +948,9 @@ sub coalesce_overlapping_hunks {\n \t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n \t\t\t\t\t\t\t $n_ofs, $n_cnt);\n \t\t}\n+\t\t# If this hunk was edited then adjust the offset delta\n+\t\t# to reflect the edit.\n+\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n@@ -1016,6 +1022,30 @@ marked for discarding.\"),\n marked for applying.\"),\n );\n \n+sub recount_edited_hunk {\n+\tlocal $_;\n+\tmy ($oldtext, $newtext) = @_;\n+\tmy ($o_cnt, $n_cnt) = (0, 0);\n+\tfor (@{$newtext}[1..$#{$newtext}]) {\n+\t\tmy $mode = substr($_, 0, 1);\n+\t\tif ($mode eq '-') {\n+\t\t\t$o_cnt++;\n+\t\t} elsif ($mode eq '+') {\n+\t\t\t$n_cnt++;\n+\t\t} elsif ($mode eq ' ') {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, undef, $n_ofs, undef) =\n+\t\t\t\t\tparse_hunk_header($newtext->[0]);\n+\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n+\t# Return the change in the number of lines inserted by this hunk\n+\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n+}\n+\n sub edit_hunk_manually {\n \tmy ($oldtext) = @_;\n \n@@ -1114,25 +1144,34 @@ sub prompt_yesno {\n }\n \n sub edit_hunk_loop {\n-\tmy ($head, $hunk, $ix) = @_;\n-\tmy $text = $hunk->[$ix]->{TEXT};\n+\tmy ($head, $hunks, $ix) = @_;\n+\tmy $hunk = $hunks->[$ix];\n+\tmy $text = $hunk->{TEXT};\n \n \twhile (1) {\n-\t\t$text = edit_hunk_manually($text);\n-\t\tif (!defined $text) {\n+\t\tmy $newtext = edit_hunk_manually($text);\n+\t\tif (!defined $newtext) {\n \t\t\treturn undef;\n \t\t}\n \t\tmy $newhunk = {\n-\t\t\tTEXT => $text,\n-\t\t\tTYPE => $hunk->[$ix]->{TYPE},\n+\t\t\tTEXT => $newtext,\n+\t\t\tTYPE => $hunk->{TYPE},\n \t\t\tUSE => 1,\n \t\t\tDIRTY => 1,\n \t\t};\n \t\tif (diff_applies($head,\n-\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t @{$hunks}[0..$ix-1],\n \t\t\t\t $newhunk,\n-\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n-\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\t\t @{$hunks}[$ix+1..$#{$hunks}])) {\n+\t\t\t$newhunk->{OFS_DELTA} =\n+\t\t\t\t\trecount_edited_hunk($text, $newtext);\n+\t\t\t# If this hunk has already been edited then\n+\t\t\t# add the offset delta of the previous edit to\n+\t\t\t# get the real delta from the original\n+\t\t\t# unedited hunk.\n+\t\t\t$hunk->{OFS_DELTA} and\n+\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$newtext})];\n \t\t\treturn $newhunk;\n \t\t}\n \t\telse {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5401d74d5804b5e43286d08a905fb96c68b02e42..5681bb2bb7e9a9c0702f8ebbcd97efbb5c5d1d58 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -563,7 +563,7 @@ test_expect_success 'add -p works with pathological context lines' '\n \ttest_cmp expected-1 actual\n '\n \n-test_expect_failure 'add -p patch editing works with pathological context lines' '\n+test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" e y |\n \tGIT_EDITOR=./editor git add -p &&\n-- \n2.16.1\n\n"},{"id":"339187","messageId":"20180213104408.9887-3-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH 2/4] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:44:06Z","receivedAt":"2018-02-13T10:44:57Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a hunk is skipped by add -i the offsets of subsequent hunks are\nnot adjusted to account for any missing insertions due to the skipped\nhunk. Most of the time this does not matter as apply uses the context\nlines to apply the subsequent hunks in the correct place, however in\npathological cases the context lines will match at the now incorrect\noffset and the hunk will be applied in the wrong place. The offsets of\nhunks following an edited hunk that has had the number of insertions\nor deletions changed also need to be updated in the same way. Add\nfailing tests to demonstrate this.\n\nSigned-off-by: Phillip Wood \n---\n t/t3701-add-interactive.sh | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 058698df6a4a9811b9db84fb5900472c47c61798..6838698a1382b24724cfbd3be04a7054489b94af 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -541,4 +541,34 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n \t! grep dirty-otherwise output\n '\n \n+test_expect_success 'set up pathological context' '\n+\tgit reset --hard &&\n+\ttest_write_lines a a a a a a a a a a a >a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\ttest_write_lines c b a a a a a a a b a a a a >a &&\n+\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n+\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n+\twrite_script editor <<-\\EOF\n+\tsed /^+c/d \"$1\" >\"$1.tmp\"\n+\tmv \"$1.tmp\" \"$1\"\n+\tEOF\n+'\n+\n+test_expect_failure 'add -p works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" n y |\n+\tgit add -p &&\n+\tgit cat-file blob :a > actual &&\n+\ttest_cmp expected-1 actual\n+'\n+\n+test_expect_failure 'add -p patch editing works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" e y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a > actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.1\n\n"},{"id":"339232","messageId":"xmqqr2popqzx.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180213104408.9887-3-phillip.wood@talktalk.net","subject":"Re: [PATCH 2/4] t3701: add failing test for pathological context lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-13T20:35:30Z","receivedAt":"2018-02-13T20:35:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> When a hunk is skipped by add -i the offsets of subsequent hunks are\n> not adjusted to account for any missing insertions due to the skipped\n> hunk. Most of the time this does not matter as apply uses the context\n> lines to apply the subsequent hunks in the correct place, however in\n> pathological cases the context lines will match at the now incorrect\n> offset and the hunk will be applied in the wrong place. The offsets of\n\nGood. The --recount \"feature\" on the receiving end does not have\nenough information to do a job as good as the code sitting at the\nside of producing a patch to be applied, and this goes in the right\ndirection.\n\n"},{"id":"339243","messageId":"20180213235642.GD1022467@genre.crustytoothpaste.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"Re: [PATCH 0/4] Correct offsets of hunks when one is skipped","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-02-13T23:56:42Z","receivedAt":"2018-02-13T23:56:55Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Feb 13, 2018 at 10:44:04AM +0000, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> While working on a patch series to stage selected lines from a hunk\n> without having to edit it I got worried that subsequent patches would\n> be applied in the wrong place which lead to this series to correct the\n> offsets of hunks following those that are skipped or edited.\n> \n> Phillip Wood (4):\n> add -i: add function to format hunk header\n> t3701: add failing test for pathological context lines\n> add -p: Adjust offsets of subsequent hunks when one is skipped\n> add -p: calculate offset delta for edited patches\n> \n> git-add--interactive.perl | 93 +++++++++++++++++++++++++++++++++++-----------\n> t/t3701-add-interactive.sh | 30 +++++++++++++++\n> 2 files changed, 102 insertions(+), 21 deletions(-)\n\nThis looks reasonably sane to me. I really like that you managed to\nproduce failing tests for this situation. I know pathological cases\nlike this have bit GCC in the past, so it's good that you fixed this.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"339599","messageId":"20180219112910.24471-1-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH v2 0/9] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:01Z","receivedAt":"2018-02-19T11:29:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince v1 I've added some test cleanups for t3701, fixed the counting\nwhen splitting and coalescing hunks containing \"\\ No newline at end of\nfile\" lines and added a patch to remove '--recount' from the\ninvocation of 'git apply'. There are minor changes to patches 5\n(previously patch 2) and patch 7 (previously patch 4) which I've\nexplained in the comments on those patches. Otherwise the original\npatches are unchanged.\n\nCover letter to v1:\n\nWhile working on a patch series to stage selected lines from a hunk\nwithout having to edit it I got worried that subsequent patches would\nbe applied in the wrong place which lead to this series to correct the\noffsets of hunks following those that are skipped or edited.\n\n\nPhillip Wood (9):\n add -i: add function to format hunk header\n t3701: indent here documents\n t3701: use test_write_lines and write_script\n t3701: don't hard code sha1 hash values\n t3701: add failing test for pathological context lines\n add -p: Adjust offsets of subsequent hunks when one is skipped\n add -p: calculate offset delta for edited patches\n add -p: fix counting when splitting and coalescing\n add -p: don't rely on apply's '--recount' option\n\n git-add--interactive.perl | 106 ++++++++++++-----\n t/t3701-add-interactive.sh | 281 ++++++++++++++++++++++++---------------------\n 2 files changed, 229 insertions(+), 158 deletions(-)\n\n-- \n2.16.1\n\n"},{"id":"339600","messageId":"20180219112910.24471-2-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 1/9] add -i: add function to format hunk header","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:02Z","receivedAt":"2018-02-19T11:29:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis code is duplicated in a couple of places so make it into a\nfunction as we're going to add some more callers shortly.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -751,6 +751,15 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" .\n+\t\t(($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\" +$n_ofs\" .\n+\t\t(($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -838,11 +847,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -912,11 +917,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n-- \n2.16.1\n\n"},{"id":"339601","messageId":"20180219112910.24471-3-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:03Z","receivedAt":"2018-02-19T11:29:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIndent here documents in line with the current style for tests.\n\nSigned-off-by: Phillip Wood \n---\n t/t3701-add-interactive.sh | 174 ++++++++++++++++++++++-----------------------\n 1 file changed, 87 insertions(+), 87 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 058698df6a4a9811b9db84fb5900472c47c61798..861ea2e08cce750515f59fc424b3f8336fd9b1a9 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tnew file mode 100644\n+\tindex 0000000..d95f3ad\n+\t--- /dev/null\n+\t+++ b/file\n+\t@@ -0,0 +1 @@\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (initial)' '\n@@ -59,14 +59,14 @@ test_expect_success 'status works (commit)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,2 @@\n+\t baseline\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (commit)' '\n@@ -83,8 +83,8 @@ test_expect_success 'revert works (commit)' '\n \n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n@@ -100,21 +100,21 @@ test_expect_success 'dummy edit works' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\t@@ -1,1 +1,4 @@\n+\t this\n+\t+patch\n+\t-does not\n+\t apply\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n \techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<\\EOF &&\n-mv -f \"$1\" oldpatch &&\n-mv -f patch \"$1\"\n-EOF\n+\tcat >>fake_editor.sh <<-\\EOF &&\n+\tmv -f \"$1\" oldpatch &&\n+\tmv -f patch \"$1\"\n+\tEOF\n \tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n@@ -126,10 +126,10 @@ test_expect_success 'bad edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\tthis patch\n+\tis garbage\n+\tEOF\n '\n \n test_expect_success 'garbage edit rejected' '\n@@ -139,28 +139,28 @@ test_expect_success 'garbage edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\t@@ -1,0 +1,0 @@\n+\t baseline\n+\t+content\n+\t+newcontent\n+\t+lines\n+\tEOF\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/file b/file\n+\tindex b5dd6c9..f910ae9 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,4 +1,4 @@\n+\t baseline\n+\t content\n+\t-newcontent\n+\t+more\n+\t lines\n+\tEOF\n '\n \n test_expect_success 'real edit works' '\n@@ -222,31 +222,31 @@ test_expect_success 'setup again' '\n \n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Expected output, similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/file b/file\n+\tindex b6f2c08..61b9053 100755\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Test splitting the first patch, then adding both\n@@ -259,15 +259,15 @@ test_expect_success 'add first line works' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/non-empty b/non-empty\n+\tdeleted file mode 100644\n+\tindex d95f3ad..0000000\n+\t--- a/non-empty\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-content\n+\tEOF\n '\n \n test_expect_success 'deleting a non-empty file' '\n@@ -282,11 +282,11 @@ test_expect_success 'deleting a non-empty file' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/empty b/empty\n+\tdeleted file mode 100644\n+\tindex e69de29..0000000\n+\tEOF\n '\n \n test_expect_success 'deleting an empty file' '\n-- \n2.16.1\n\n"},{"id":"339602","messageId":"20180219112910.24471-8-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 7/9] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:08Z","receivedAt":"2018-02-19T11:29:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRecount the number of preimage and postimage lines in a hunk after it\nhas been edited so any change in the number of insertions or deletions\ncan be used to adjust the offsets of subsequent hunks. If an edited\nhunk is subsequently split then the offset correction will be lost. It\nwould be possible to fix this if it is a problem, however the code\nhere is still an improvement on the status quo for the common case\nwhere an edited hunk is applied without being split.\n\nThis is also a necessary step to removing '--recount' and\n'--allow-overlap' from the invocation of 'git apply'. Before\n'--recount' can be removed the splitting and coalescing counting needs\nto be fixed to handle a missing newline at the end of a file. In order\nto remove '--allow-overlap' there needs to be i) some way of verifying\nthe offset data in the edited hunk (probably by correlating the\npreimage (or postimage if the patch is going to be applied in reverse)\nlines of the edited and unedited versions to see if they are offset or\nif any leading/trailing context lines have been removed) and ii) a way of\ndealing with edited hunks that change context lines that are shared\nwith neighbouring hunks.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v1\n - edited hunks are now recounted before seeing if they apply in\n preparation for removing '--recount' when invoking 'git apply'\n - added sentence to commit message about the offset data being lost\n if an edited hunk is split\n\n git-add--interactive.perl | 55 ++++++++++++++++++++++++++++++++++++++--------\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 47 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7a0a5896bb8fa063ace29b85840849c867b874f5..0df0c2aa065af88e159f8e9a2febe12f4ef8ee75 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {\n \t\t\t\t\t\tparse_hunk_header($text->[0]);\n \t\tunless ($_->{USE}) {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\t# If this hunk has been edited then subtract\n+\t\t\t# the delta that is due to the edit.\n+\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n \t\t\t$n_ofs += $ofs_delta;\n \t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n \t\t\t\t\t\t\t $n_ofs, $n_cnt);\n \t\t}\n+\t\t# If this hunk was edited then adjust the offset delta\n+\t\t# to reflect the edit.\n+\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n@@ -1016,6 +1022,30 @@ marked for discarding.\"),\n marked for applying.\"),\n );\n \n+sub recount_edited_hunk {\n+\tlocal $_;\n+\tmy ($oldtext, $newtext) = @_;\n+\tmy ($o_cnt, $n_cnt) = (0, 0);\n+\tfor (@{$newtext}[1..$#{$newtext}]) {\n+\t\tmy $mode = substr($_, 0, 1);\n+\t\tif ($mode eq '-') {\n+\t\t\t$o_cnt++;\n+\t\t} elsif ($mode eq '+') {\n+\t\t\t$n_cnt++;\n+\t\t} elsif ($mode eq ' ') {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, undef, $n_ofs, undef) =\n+\t\t\t\t\tparse_hunk_header($newtext->[0]);\n+\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n+\t# Return the change in the number of lines inserted by this hunk\n+\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n+}\n+\n sub edit_hunk_manually {\n \tmy ($oldtext) = @_;\n \n@@ -1114,25 +1144,32 @@ sub prompt_yesno {\n }\n \n sub edit_hunk_loop {\n-\tmy ($head, $hunk, $ix) = @_;\n-\tmy $text = $hunk->[$ix]->{TEXT};\n+\tmy ($head, $hunks, $ix) = @_;\n+\tmy $hunk = $hunks->[$ix];\n+\tmy $text = $hunk->{TEXT};\n \n \twhile (1) {\n-\t\t$text = edit_hunk_manually($text);\n-\t\tif (!defined $text) {\n+\t\tmy $newtext = edit_hunk_manually($text);\n+\t\tif (!defined $newtext) {\n \t\t\treturn undef;\n \t\t}\n \t\tmy $newhunk = {\n-\t\t\tTEXT => $text,\n-\t\t\tTYPE => $hunk->[$ix]->{TYPE},\n+\t\t\tTEXT => $newtext,\n+\t\t\tTYPE => $hunk->{TYPE},\n \t\t\tUSE => 1,\n \t\t\tDIRTY => 1,\n \t\t};\n+\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n+\t\t# If this hunk has already been edited then add the\n+\t\t# offset delta of the previous edit to get the real\n+\t\t# delta from the original unedited hunk.\n+\t\t$hunk->{OFS_DELTA} and\n+\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n \t\tif (diff_applies($head,\n-\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t @{$hunks}[0..$ix-1],\n \t\t\t\t $newhunk,\n-\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n-\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\t\t @{$hunks}[$ix+1..$#{$hunks}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$newtext})];\n \t\t\treturn $newhunk;\n \t\t}\n \t\telse {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex e153a02605df25ea40e49dd48b7c9fd981713b9d..bbda771ba7e516aa37a204beffba7eeb0c85a2f4 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -532,7 +532,7 @@ test_expect_success 'add -p works with pathological context lines' '\n \ttest_cmp expected-1 actual\n '\n \n-test_expect_failure 'add -p patch editing works with pathological context lines' '\n+test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n \ttest_set_editor \"$FAKE_EDITOR\" &&\n \t# n q q below is in case edit fails\n-- \n2.16.1\n\n"},{"id":"339603","messageId":"20180219112910.24471-10-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:10Z","receivedAt":"2018-02-19T11:29:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nNow that add -p counts patches properly it should be possible to turn\noff the '--recount' option when invoking 'git apply'\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n I can't think of a reason why this shouldn't be OK but I can't help\n feeling slightly nervous about it. I've made it a separate patch so it\n can be easily dropped or reverted if I've missed something.\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3226c2c4f02d5f8679d77b8eede984fc727b422d..a64c0db57d62ab02ef718b8c8f821105132d9920 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -677,7 +677,7 @@ sub add_untracked_cmd {\n sub run_git_apply {\n \tmy $cmd = shift;\n \tmy $fh;\n-\topen $fh, '| git ' . $cmd . \" --recount --allow-overlap\";\n+\topen $fh, '| git ' . $cmd . \" --allow-overlap\";\n \tprint $fh @_;\n \treturn close $fh;\n }\n-- \n2.16.1\n\n"},{"id":"339604","messageId":"20180219112910.24471-9-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 8/9] add -p: fix counting when splitting and coalescing","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:09Z","receivedAt":"2018-02-19T11:29:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a file has no trailing new line at the end diff records this by\nappending \"\\ No newline at end of file\" below the last line of the\nfile. This line should not be counted in the hunk header. Fix the\nsplitting and coalescing code to count files without a trailing new line\nproperly and change one of the tests to test splitting without a\ntrailing new line.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 13 +++++++++----\n t/t3701-add-interactive.sh | 28 ++++++++++++++++++++++------\n 2 files changed, 31 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 0df0c2aa065af88e159f8e9a2febe12f4ef8ee75..3226c2c4f02d5f8679d77b8eede984fc727b422d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -793,6 +793,11 @@ sub split_hunk {\n \t\twhile (++$i < @$text) {\n \t\t\tmy $line = $text->[$i];\n \t\t\tmy $display = $display->[$i];\n+\t\t\tif ($line =~ /^\\\\/) {\n+\t\t\t\tpush @{$this->{TEXT}}, $line;\n+\t\t\t\tpush @{$this->{DISPLAY}}, $display;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\tif ($line =~ /^ /) {\n \t\t\t\tif ($this->{ADDDEL} &&\n \t\t\t\t !defined $next_hunk_start) {\n@@ -887,8 +892,8 @@ sub merge_hunk {\n \t$o_cnt = $n_cnt = 0;\n \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n \t\tmy $line = $prev->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\n@@ -905,8 +910,8 @@ sub merge_hunk {\n \n \tfor ($i = 1; $i < @{$this->{TEXT}}; $i++) {\n \t\tmy $line = $this->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex bbda771ba7e516aa37a204beffba7eeb0c85a2f4..0fb9c0e0f140e21ef7ad467c40b9211d29f53db6 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -221,30 +221,46 @@ test_expect_success 'setup patch' '\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n-# Expected output, similar to the patch but w/ diff at the top\n+# Expected output, diff is similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n-\tdiff --git a/file b/file\n+\techo diff --git a/file b/file >expected &&\n+\tcat patch >>expected &&\n+\tcat >expected-output <<-EOF\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n \t+firstline\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n+\t@@ -1,2 +1,3 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t@@ -1,2 +2,3 @@\n+\t baseline\n+\t content\n+\t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n # Test splitting the first patch, then adding both\n-test_expect_success 'add first line works' '\n+test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n \tgit commit -am \"clear local changes\" &&\n \tgit apply patch &&\n-\t(echo s; echo y; echo y) | git add -p file &&\n+\tprintf \"%s\\n\" s y y | git add -p file 2>error |\n+\t\tsed -n -e \"s/^Stage this hunk[^@]*\\(@@ .*\\)/\\1/\" \\\n+\t\t -e \"/^[-+@ \\\\\\\\]\"/p >output &&\n+\ttest_must_be_empty error &&\n \tgit diff --cached | sed /^index/d >diff &&\n-\ttest_cmp expected diff\n+\ttest_cmp expected diff &&\n+\ttest_cmp expected-output output\n '\n \n test_expect_success 'setup expected' '\n-- \n2.16.1\n\n"},{"id":"339605","messageId":"20180219112910.24471-5-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:05Z","receivedAt":"2018-02-19T11:29:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nPurge the index lines from diffs so we're not hard coding sha1 hash\nvalues in the expected output.\n\nSigned-off-by: Phillip Wood \n---\n t/t3701-add-interactive.sh | 32 ++++++++++++--------------------\n 1 file changed, 12 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex b73a9598ab3eaed074735e99f3dcbc8f88d86f4c..70748393f28c93f4bc5d43b07bd96bd208aba3e9 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -24,7 +24,6 @@ test_expect_success 'status works (initial)' '\n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tnew file mode 100644\n-\tindex 0000000..d95f3ad\n \t--- /dev/null\n \t+++ b/file\n \t@@ -0,0 +1 @@\n@@ -34,7 +33,7 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'diff works (initial)' '\n \t(echo d; echo 1) | git add -i >output &&\n-\tsed -ne \"/new file/,/content/p\" diff &&\n+\tsed -ne /^index/d -e \"/new file/,/content/p\" diff &&\n \ttest_cmp expected diff\n '\n test_expect_success 'revert works (initial)' '\n@@ -60,7 +59,6 @@ test_expect_success 'status works (commit)' '\n \n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n-\tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1 +1,2 @@\n@@ -71,7 +69,7 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'diff works (commit)' '\n \t(echo d; echo 1) | git add -i >output &&\n-\tsed -ne \"/^index/,/content/p\" diff &&\n+\tsed -ne \"/^---/,/content/p\" diff &&\n \ttest_cmp expected diff\n '\n test_expect_success 'revert works (commit)' '\n@@ -90,7 +88,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'dummy edit works' '\n \ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n-\tgit diff > diff &&\n+\tgit diff | sed /^index/d >diff &&\n \ttest_cmp expected diff\n '\n \n@@ -145,7 +143,6 @@ test_expect_success 'setup patch' '\n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/file b/file\n-\tindex b5dd6c9..f910ae9 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1,4 +1,4 @@\n@@ -159,7 +156,7 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'real edit works' '\n \t(echo e; echo n; echo d) | git add -p &&\n-\tgit diff >output &&\n+\tgit diff | sed /^index/d >output &&\n \ttest_cmp expected output\n '\n \n@@ -168,10 +165,10 @@ test_expect_success 'skip files similarly as commit -a' '\n \techo file >.gitignore &&\n \techo changed >file &&\n \techo y | git add -p file &&\n-\tgit diff >output &&\n+\tgit diff | sed /^index/d >output &&\n \tgit reset &&\n \tgit commit -am commit &&\n-\tgit diff >expected &&\n+\tgit diff | sed /^index/d >expected &&\n \ttest_cmp expected output &&\n \tgit reset --hard HEAD^\n '\n@@ -217,7 +214,6 @@ test_expect_success 'setup again' '\n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n \tcat >patch <<-EOF\n-\tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n@@ -232,7 +228,6 @@ test_expect_success 'setup patch' '\n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/file b/file\n-\tindex b6f2c08..61b9053 100755\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n@@ -248,15 +243,14 @@ test_expect_success 'add first line works' '\n \tgit commit -am \"clear local changes\" &&\n \tgit apply patch &&\n \t(echo s; echo y; echo y) | git add -p file &&\n-\tgit diff --cached > diff &&\n+\tgit diff --cached | sed /^index/d >diff &&\n \ttest_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/non-empty b/non-empty\n \tdeleted file mode 100644\n-\tindex d95f3ad..0000000\n \t--- a/non-empty\n \t+++ /dev/null\n \t@@ -1 +0,0 @@\n@@ -271,15 +265,14 @@ test_expect_success 'deleting a non-empty file' '\n \tgit commit -m non-empty &&\n \trm non-empty &&\n \techo y | git add -p non-empty &&\n-\tgit diff --cached >diff &&\n+\tgit diff --cached | sed /^index/d >diff &&\n \ttest_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/empty b/empty\n \tdeleted file mode 100644\n-\tindex e69de29..0000000\n \tEOF\n '\n \n@@ -290,7 +283,7 @@ test_expect_success 'deleting an empty file' '\n \tgit commit -m empty &&\n \trm empty &&\n \techo y | git add -p empty &&\n-\tgit diff --cached >diff &&\n+\tgit diff --cached | sed /^index/d >diff &&\n \ttest_cmp expected diff\n '\n \n@@ -317,7 +310,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n \t# exits.\n \tprintf \"%s\\n\" s e q n q q |\n \tEDITOR=: git add -p &&\n-\tgit diff >actual &&\n+\tgit diff | sed /^index/d >actual &&\n \t! grep \"^+15\" actual\n '\n \n@@ -329,7 +322,7 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \tprintf \"%s\\n\" s n y e q n q q |\n \tEDITOR=: git add -p 2>error &&\n \ttest_must_be_empty error &&\n-\tgit diff >actual &&\n+\tgit diff | sed /^index/d >actual &&\n \t! grep \"^+31\" actual\n '\n \n@@ -348,14 +341,13 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \tcat >expected <<-\\EOF &&\n \t* Unmerged path conflict.t\n \tdiff --git a/non-conflict.t b/non-conflict.t\n-\tindex f766221..5ea2ed4 100644\n \t--- a/non-conflict.t\n \t+++ b/non-conflict.t\n \t@@ -1 +1 @@\n \t-non-conflict\n \t+changed\n \tEOF\n-\tgit diff --cached >diff &&\n+\tgit diff --cached | sed /^index/d >diff &&\n \ttest_cmp expected diff\n '\n \n-- \n2.16.1\n\n"},{"id":"339606","messageId":"20180219112910.24471-4-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 3/9] t3701: use test_write_lines and write_script","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:04Z","receivedAt":"2018-02-19T11:29:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSimplify things slightly by using the above helpers.\n\nSigned-off-by: Phillip Wood \n---\n t/t3701-add-interactive.sh | 36 +++++++-----------------------------\n 1 file changed, 7 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 861ea2e08cce750515f59fc424b3f8336fd9b1a9..b73a9598ab3eaed074735e99f3dcbc8f88d86f4c 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n \tEOF\n '\n \n-test_expect_success 'setup fake editor' '\n-\t>fake_editor.sh &&\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n-'\n-\n test_expect_success 'dummy edit works' '\n+\ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n \ttest_cmp expected diff\n@@ -110,13 +105,12 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<-\\EOF &&\n+\tFAKE_EDITOR=\"$(pwd)/fake-editor.sh\" &&\n+\twrite_script \"$FAKE_EDITOR\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n+\ttest_set_editor \"$FAKE_EDITOR\"\n '\n \n test_expect_success 'bad edit rejected' '\n@@ -302,18 +296,12 @@ test_expect_success 'deleting an empty file' '\n \n test_expect_success 'split hunk setup' '\n \tgit reset --hard &&\n-\tfor i in 10 20 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test &&\n+\ttest_write_lines 10 20 30 40 50 60 >test &&\n \tgit add test &&\n \ttest_tick &&\n \tgit commit -m test &&\n \n-\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test\n+\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n '\n \n test_expect_success 'split hunk \"add -p (edit)\"' '\n@@ -334,17 +322,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n '\n \n test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n-\tcat >test <<-\\EOF &&\n-\t5\n-\t10\n-\t20\n-\t21\n-\t30\n-\t31\n-\t40\n-\t50\n-\t60\n-\tEOF\n+\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n \t# q n q q is there to make sure we exit at the end.\n-- \n2.16.1\n\n"},{"id":"339607","messageId":"20180219112910.24471-7-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:07Z","receivedAt":"2018-02-19T11:29:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince commit 8cbd431082 (\"git-add--interactive: replace hunk\nrecounting with apply --recount\", 2008-7-2) if a hunk is skipped then\nwe rely on the context lines to apply subsequent hunks in the right\nplace. While this works most of the time it is possible for hunks to\nend up being applied in the wrong place. To fix this adjust the offset\nof subsequent hunks to correct for any change in the number of\ninsertions or deletions due to the skipped hunk. The change in offset\ndue to edited hunks that have the number of insertions or deletions\nchanged is ignored here, it will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 +++++++++++++--\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8ababa6453ac4f57a925aacbb8b9bb1c973e4a54..7a0a5896bb8fa063ace29b85840849c867b874f5 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {\n \tmy @out = ();\n \n \tmy ($last_o_ctx, $last_was_dirty);\n+\tmy $ofs_delta = 0;\n \n-\tfor (grep { $_->{USE} } @in) {\n+\tfor (@in) {\n \t\tif ($_->{TYPE} ne 'hunk') {\n \t\t\tpush @out, $_;\n \t\t\tnext;\n \t\t}\n \t\tmy $text = $_->{TEXT};\n-\t\tmy ($o_ofs) = parse_hunk_header($text->[0]);\n+\t\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =\n+\t\t\t\t\t\tparse_hunk_header($text->[0]);\n+\t\tunless ($_->{USE}) {\n+\t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($ofs_delta) {\n+\t\t\t$n_ofs += $ofs_delta;\n+\t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t $n_ofs, $n_cnt);\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 06c4747f506a1b05ccad0f9387e1fbd69cfd7784..e153a02605df25ea40e49dd48b7c9fd981713b9d 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -524,7 +524,7 @@ test_expect_success 'set up pathological context' '\n \ttest_write_lines +b \" a\" >patch\n '\n \n-test_expect_failure 'add -p works with pathological context lines' '\n+test_expect_success 'add -p works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" n y |\n \tgit add -p &&\n-- \n2.16.1\n\n"},{"id":"339608","messageId":"20180219112910.24471-6-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-1-phillip.wood@talktalk.net","subject":"[PATCH v2 5/9] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:29:06Z","receivedAt":"2018-02-19T11:29:56Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a hunk is skipped by add -i the offsets of subsequent hunks are\nnot adjusted to account for any missing insertions due to the skipped\nhunk. Most of the time this does not matter as apply uses the context\nlines to apply the subsequent hunks in the correct place, however in\npathological cases the context lines will match at the now incorrect\noffset and the hunk will be applied in the wrong place. The offsets of\nhunks following an edited hunk that has had the number of insertions\nor deletions changed also need to be updated in the same way. Add\nfailing tests to demonstrate this.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v1:\n - changed edit test to use the existing fake editor and to strip\n the hunk header and some context lines as well as deleting an\n insertion\n - style fixes\n\n t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++++++++++\n 1 file changed, 31 insertions(+)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 70748393f28c93f4bc5d43b07bd96bd208aba3e9..06c4747f506a1b05ccad0f9387e1fbd69cfd7784 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -511,4 +511,35 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n \t! grep dirty-otherwise output\n '\n \n+test_expect_success 'set up pathological context' '\n+\tgit reset --hard &&\n+\ttest_write_lines a a a a a a a a a a a >a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\ttest_write_lines c b a a a a a a a b a a a a >a &&\n+\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n+\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n+\t# check editing can cope with missing header and deleted context lines\n+\t# as well as changes to other lines\n+\ttest_write_lines +b \" a\" >patch\n+'\n+\n+test_expect_failure 'add -p works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" n y |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-1 actual\n+'\n+\n+test_expect_failure 'add -p patch editing works with pathological context lines' '\n+\tgit reset &&\n+\ttest_set_editor \"$FAKE_EDITOR\" &&\n+\t# n q q below is in case edit fails\n+\tprintf \"%s\\n\" e y n q q |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.1\n\n"},{"id":"339616","messageId":"21bc3131-9432-537f-9e60-8cb3166572eb@talktalk.net","threadId":"47825","inReplyTo":"20180213235642.GD1022467@genre.crustytoothpaste.net","subject":"Re: [PATCH 0/4] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T13:01:34Z","receivedAt":"2018-02-19T13:01:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 13/02/18 23:56, brian m. carlson wrote:\n> On Tue, Feb 13, 2018 at 10:44:04AM +0000, Phillip Wood wrote:\n>> From: Phillip Wood \n>>\n>> While working on a patch series to stage selected lines from a hunk\n>> without having to edit it I got worried that subsequent patches would\n>> be applied in the wrong place which lead to this series to correct the\n>> offsets of hunks following those that are skipped or edited.\n>>\n>> Phillip Wood (4):\n>> add -i: add function to format hunk header\n>> t3701: add failing test for pathological context lines\n>> add -p: Adjust offsets of subsequent hunks when one is skipped\n>> add -p: calculate offset delta for edited patches\n>>\n>> git-add--interactive.perl | 93 +++++++++++++++++++++++++++++++++++-----------\n>> t/t3701-add-interactive.sh | 30 +++++++++++++++\n>> 2 files changed, 102 insertions(+), 21 deletions(-)\n> \n> This looks reasonably sane to me. I really like that you managed to\n> produce failing tests for this situation. I know pathological cases\n> like this have bit GCC in the past, so it's good that you fixed this.\n> \n\nThanks Brain, it's interesting to hear that GCC has been bitten in the past\n\nBest Wishes\n\nPhillip\n"},{"id":"339618","messageId":"CAPig+cTUdw+eHOfj9=wt6DRR5xfaQxg-qQpF1VFfArY1uwcHnA@mail.gmail.com","threadId":"47825","inReplyTo":"20180219112910.24471-3-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 2/9] t3701: indent here documents","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-02-19T18:36:39Z","receivedAt":"2018-02-19T18:36:47Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood wrote:\n> From: Phillip Wood \n>\n> Indent here documents in line with the current style for tests.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> @@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '\n> test_expect_success 'setup expected' '\n> -cat >expected < -new file mode 100644\n> -index 0000000..d95f3ad\n> ---- /dev/null\n> -+++ b/file\n> -@@ -0,0 +1 @@\n> -+content\n> -EOF\n> + cat >expected <<-EOF\n\nMinor: You could take the opportunity to update these to use -\\EOF\n(rather than -EOF) to document that no variable interpolation is\nexpected inside the 'here' document. Probably itself not worth a\nre-roll.\n\n> + new file mode 100644\n> + index 0000000..d95f3ad\n> + --- /dev/null\n> + +++ b/file\n> + @@ -0,0 +1 @@\n> + +content\n> + EOF\n> '\n"},{"id":"339619","messageId":"CAPig+cRWCd6pYxc8NrOM_ntodgkwwfMpD_tWQXYXOukVWgQfCw@mail.gmail.com","threadId":"47825","inReplyTo":"20180219112910.24471-4-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-02-19T18:47:16Z","receivedAt":"2018-02-19T18:47:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood wrote:\n> From: Phillip Wood \n>\n> Simplify things slightly by using the above helpers.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> @@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n> @@ -110,13 +105,12 @@ test_expect_success 'setup patch' '\n> test_expect_success 'setup fake editor' '\n> - echo \"#!$SHELL_PATH\" >fake_editor.sh &&\n> - cat >>fake_editor.sh <<-\\EOF &&\n> + FAKE_EDITOR=\"$(pwd)/fake-editor.sh\" &&\n> + write_script \"$FAKE_EDITOR\" <<-\\EOF &&\n> mv -f \"$1\" oldpatch &&\n> mv -f patch \"$1\"\n> EOF\n> - chmod a+x fake_editor.sh &&\n> - test_set_editor \"$(pwd)/fake_editor.sh\"\n> + test_set_editor \"$FAKE_EDITOR\"\n> '\n\nThe very first thing that test_set_editor() does is set FAKE_EDITOR to\nthe value of $1, so it is confusing to see it getting setting it here\nfirst; the reader has to spend extra brain cycles wondering if\nsomething non-obvious is going on that requires this manual\nassignment. Perhaps drop the assignment altogether and just write\nliteral \"fake_editor.sh\" in the couple places it's needed (as was done\nin the original code)?\n"},{"id":"339620","messageId":"CAPig+cRUrct06AxgW6BgsDFPq2i_MpD8OOR_Ks_Kko_AYOQMcA@mail.gmail.com","threadId":"47825","inReplyTo":"20180219112910.24471-5-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-02-19T18:52:34Z","receivedAt":"2018-02-19T18:52:40Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood wrote:\n> Purge the index lines from diffs so we're not hard coding sha1 hash\n> values in the expected output.\n\nPerhaps the commit message could provide a bit more explanation about\nwhy this is a good idea. For instance, briefly mention that doing so\nwill ease Git's transition from SHA1 to some newer hash function.\n\n> Signed-off-by: Phillip Wood \n"},{"id":"339716","messageId":"xmqqr2pf4m0l.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"CAPig+cRWCd6pYxc8NrOM_ntodgkwwfMpD_tWQXYXOukVWgQfCw@mail.gmail.com","subject":"Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-20T17:19:06Z","receivedAt":"2018-02-20T17:19:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> test_expect_success 'setup fake editor' '\n>> - echo \"#!$SHELL_PATH\" >fake_editor.sh &&\n>> - cat >>fake_editor.sh <<-\\EOF &&\n>> + FAKE_EDITOR=\"$(pwd)/fake-editor.sh\" &&\n>> + write_script \"$FAKE_EDITOR\" <<-\\EOF &&\n>> mv -f \"$1\" oldpatch &&\n>> mv -f patch \"$1\"\n>> EOF\n>> - chmod a+x fake_editor.sh &&\n>> - test_set_editor \"$(pwd)/fake_editor.sh\"\n>> + test_set_editor \"$FAKE_EDITOR\"\n>> '\n>\n> The very first thing that test_set_editor() does is set FAKE_EDITOR to\n> the value of $1, so it is confusing to see it getting setting it here\n> first; the reader has to spend extra brain cycles wondering if\n> something non-obvious is going on that requires this manual\n> assignment. Perhaps drop the assignment altogether and just write\n> literal \"fake_editor.sh\" in the couple places it's needed (as was done\n> in the original code)?\n\nYeah, I think $(pwd)/ prefix is needed at the final step (i.e. as\nthe first argument to test_set_editor) for this to be a faithful\nrewrite but it is distracting having to write it anywhere else.\n\nOther than that, this looks like a quite straight-forward cleanup.\n\nThanks, both. Here is what I'd be queuing tentatively.\n\n t/t3701-add-interactive.sh | 33 +++++----------------------------\n 1 file changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 39c7423069..4a369fcb51 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n \tEOF\n '\n \n-test_expect_success 'setup fake editor' '\n-\t>fake_editor.sh &&\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n-'\n-\n test_expect_success 'dummy edit works' '\n+\ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n \ttest_cmp expected diff\n@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<-\\EOF &&\n+\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n \n@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '\n \n test_expect_success 'split hunk setup' '\n \tgit reset --hard &&\n-\tfor i in 10 20 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test &&\n+\ttest_write_lines 10 20 30 40 50 60 >test &&\n \tgit add test &&\n \ttest_tick &&\n \tgit commit -m test &&\n \n-\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test\n+\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n '\n \n test_expect_success 'split hunk \"add -p (edit)\"' '\n@@ -334,17 +321,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n '\n \n test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n-\tcat >test <<-\\EOF &&\n-\t5\n-\t10\n-\t20\n-\t21\n-\t30\n-\t31\n-\t40\n-\t50\n-\t60\n-\tEOF\n+\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n \t# q n q q is there to make sure we exit at the end.\n"},{"id":"339717","messageId":"xmqqk1v74l1t.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180219112910.24471-5-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-20T17:39:58Z","receivedAt":"2018-02-20T17:40:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Purge the index lines from diffs so we're not hard coding sha1 hash\n> values in the expected output.\n\nThe motivation of this patch is clear, but all-zero object name for\nmissing side of deletion or creation patch should not change when we\ntransition to any hash function. Neither the permission bits shown\nin the output (and whether the index line has the bits are shown on\nit in the first place, i.e. the index line of a creation patch does\nnot, whilethe one in a modification patch does).\n\nSo I am a bit ambivalent about this change.\n\nPerhaps have a filter that redacts, instead of removes, selected\npieces of information that are likely to change while hash\ntransition, and use that consistently to filter both the expected\noutput and the actual output before comparing?\n\n"},{"id":"339719","messageId":"xmqqbmgj4j5c.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180219112910.24471-2-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 1/9] add -i: add function to format hunk header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-20T18:21:03Z","receivedAt":"2018-02-20T18:21:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> This code is duplicated in a couple of places so make it into a\n> function as we're going to add some more callers shortly.\n>\n> Signed-off-by: Phillip Wood \n> ---\n> git-add--interactive.perl | 21 +++++++++++----------\n> 1 file changed, 11 insertions(+), 10 deletions(-)\n>\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755\n\nThis is quite a tangent, but please refrain from using full object\nname on the index line. Because it does not add much value in the\ncontext of patch exchange for project of this size to use more than\n7-12 hexdigits each, the only effect of doing so is to push the mode\nbit far to the right.\n"},{"id":"339720","messageId":"xmqq7er74ht2.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180219112910.24471-10-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-20T18:50:01Z","receivedAt":"2018-02-20T18:50:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Now that add -p counts patches properly it should be possible to turn\n> off the '--recount' option when invoking 'git apply'\n>\n> Signed-off-by: Phillip Wood \n> ---\n>\n> Notes:\n> I can't think of a reason why this shouldn't be OK but I can't help\n> feeling slightly nervous about it. I've made it a separate patch so it\n> can be easily dropped or reverted if I've missed something.\n\nIt is a lot more preferrable approach to produce a patch with\ninformation as precise as the producer (i.e. add -p) can and feed it\nto the consumer (i.e. apply) than to rely on the --recount and the\n--allow-overlap options that are mainly ways to force the consumer\nmake imprecise guesses and accepting potential garbage with looser\nconsistency checks. So I very much like the direction (and the next\nstep may be to make sure we coalesce correctly so that we do not\nhave to use \"--allow-overlap\").\n\nThanks for working on this. Let's see if the updated code really\n\"counts patches properly\" as the log message claims to ;-)\n\n> git-add--interactive.perl | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 3226c2c4f02d5f8679d77b8eede984fc727b422d..a64c0db57d62ab02ef718b8c8f821105132d9920 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -677,7 +677,7 @@ sub add_untracked_cmd {\n> sub run_git_apply {\n> \tmy $cmd = shift;\n> \tmy $fh;\n> -\topen $fh, '| git ' . $cmd . \" --recount --allow-overlap\";\n> +\topen $fh, '| git ' . $cmd . \" --allow-overlap\";\n> \tprint $fh @_;\n> \treturn close $fh;\n> }\n"},{"id":"339800","messageId":"372bd26b-4e95-3f82-a1b1-dd0191568503@talktalk.net","threadId":"47825","inReplyTo":"xmqqr2pf4m0l.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-21T11:26:30Z","receivedAt":"2018-02-21T11:26:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/02/18 17:19, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> \n>>> test_expect_success 'setup fake editor' '\n>>> - echo \"#!$SHELL_PATH\" >fake_editor.sh &&\n>>> - cat >>fake_editor.sh <<-\\EOF &&\n>>> + FAKE_EDITOR=\"$(pwd)/fake-editor.sh\" &&\n>>> + write_script \"$FAKE_EDITOR\" <<-\\EOF &&\n>>> mv -f \"$1\" oldpatch &&\n>>> mv -f patch \"$1\"\n>>> EOF\n>>> - chmod a+x fake_editor.sh &&\n>>> - test_set_editor \"$(pwd)/fake_editor.sh\"\n>>> + test_set_editor \"$FAKE_EDITOR\"\n>>> '\n>>\n>> The very first thing that test_set_editor() does is set FAKE_EDITOR to\n>> the value of $1, so it is confusing to see it getting setting it here\n>> first; the reader has to spend extra brain cycles wondering if\n>> something non-obvious is going on that requires this manual\n>> assignment. Perhaps drop the assignment altogether and just write\n>> literal \"fake_editor.sh\" in the couple places it's needed (as was done\n>> in the original code)?\n> \n> Yeah, I think $(pwd)/ prefix is needed at the final step (i.e. as\n> the first argument to test_set_editor) for this to be a faithful\n> rewrite but it is distracting having to write it anywhere else.\n> \n> Other than that, this looks like a quite straight-forward cleanup.\n> \n> Thanks, both. Here is what I'd be queuing tentatively.\n\nThat looks good, thanks for fixing it up\n\nPhillip\n> \n> t/t3701-add-interactive.sh | 33 +++++----------------------------\n> 1 file changed, 5 insertions(+), 28 deletions(-)\n> \n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 39c7423069..4a369fcb51 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n> \tEOF\n> '\n> \n> -test_expect_success 'setup fake editor' '\n> -\t>fake_editor.sh &&\n> -\tchmod a+x fake_editor.sh &&\n> -\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n> -'\n> -\n> test_expect_success 'dummy edit works' '\n> +\ttest_set_editor : &&\n> \t(echo e; echo a) | git add -p &&\n> \tgit diff > diff &&\n> \ttest_cmp expected diff\n> @@ -110,12 +105,10 @@ test_expect_success 'setup patch' '\n> '\n> \n> test_expect_success 'setup fake editor' '\n> -\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n> -\tcat >>fake_editor.sh <<-\\EOF &&\n> +\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n> \tmv -f \"$1\" oldpatch &&\n> \tmv -f patch \"$1\"\n> \tEOF\n> -\tchmod a+x fake_editor.sh &&\n> \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n> '\n> \n> @@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '\n> \n> test_expect_success 'split hunk setup' '\n> \tgit reset --hard &&\n> -\tfor i in 10 20 30 40 50 60\n> -\tdo\n> -\t\techo $i\n> -\tdone >test &&\n> +\ttest_write_lines 10 20 30 40 50 60 >test &&\n> \tgit add test &&\n> \ttest_tick &&\n> \tgit commit -m test &&\n> \n> -\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n> -\tdo\n> -\t\techo $i\n> -\tdone >test\n> +\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n> '\n> \n> test_expect_success 'split hunk \"add -p (edit)\"' '\n> @@ -334,17 +321,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n> '\n> \n> test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n> -\tcat >test <<-\\EOF &&\n> -\t5\n> -\t10\n> -\t20\n> -\t21\n> -\t30\n> -\t31\n> -\t40\n> -\t50\n> -\t60\n> -\tEOF\n> +\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n> \tgit reset &&\n> \t# test sequence is s(plit), n(o), y(es), e(dit)\n> \t# q n q q is there to make sure we exit at the end.\n> \n\n"},{"id":"339801","messageId":"69548d34-3982-6eb7-2620-6e223ab80aaf@talktalk.net","threadId":"47825","inReplyTo":"20180219112910.24471-6-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 5/9] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-21T11:28:31Z","receivedAt":"2018-02-21T11:28:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 19/02/18 11:29, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> When a hunk is skipped by add -i the offsets of subsequent hunks are\n> not adjusted to account for any missing insertions due to the skipped\n> hunk. Most of the time this does not matter as apply uses the context\n> lines to apply the subsequent hunks in the correct place, however in\n> pathological cases the context lines will match at the now incorrect\n> offset and the hunk will be applied in the wrong place. The offsets of\n> hunks following an edited hunk that has had the number of insertions\n> or deletions changed also need to be updated in the same way. Add\n> failing tests to demonstrate this.\n> \n> Signed-off-by: Phillip Wood \n> ---\n> \n> Notes:\n> changes since v1:\n> - changed edit test to use the existing fake editor and to strip\n> the hunk header and some context lines as well as deleting an\n> insertion\n> - style fixes\n> \n> t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++++++++++\n> 1 file changed, 31 insertions(+)\n> \n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 70748393f28c93f4bc5d43b07bd96bd208aba3e9..06c4747f506a1b05ccad0f9387e1fbd69cfd7784 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -511,4 +511,35 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n> \t! grep dirty-otherwise output\n> '\n> \n> +test_expect_success 'set up pathological context' '\n> +\tgit reset --hard &&\n> +\ttest_write_lines a a a a a a a a a a a >a &&\n> +\tgit add a &&\n> +\tgit commit -m a &&\n> +\ttest_write_lines c b a a a a a a a b a a a a >a &&\n> +\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n> +\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n> +\t# check editing can cope with missing header and deleted context lines\n> +\t# as well as changes to other lines\n> +\ttest_write_lines +b \" a\" >patch\n> +'\n> +\n> +test_expect_failure 'add -p works with pathological context lines' '\n> +\tgit reset &&\n> +\tprintf \"%s\\n\" n y |\n> +\tgit add -p &&\n> +\tgit cat-file blob :a >actual &&\n> +\ttest_cmp expected-1 actual\n> +'\n> +\n> +test_expect_failure 'add -p patch editing works with pathological context lines' '\n> +\tgit reset &&\n> +\ttest_set_editor \"$FAKE_EDITOR\" &&\nIn light of the discussion of patch 2, I think this line should be deleted\n\n\n> +\t# n q q below is in case edit fails\n> +\tprintf \"%s\\n\" e y n q q |\n> +\tgit add -p &&\n> +\tgit cat-file blob :a >actual &&\n> +\ttest_cmp expected-2 actual\n> +'\n> +\n> test_done\n> \n\n"},{"id":"339802","messageId":"8f39e6b1-0031-5d97-f636-ae67cfdb3f1c@talktalk.net","threadId":"47825","inReplyTo":"xmqqk1v74l1t.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-21T11:42:30Z","receivedAt":"2018-02-21T11:42:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/02/18 17:39, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> Purge the index lines from diffs so we're not hard coding sha1 hash\n>> values in the expected output.\n> \n> The motivation of this patch is clear, but all-zero object name for\n> missing side of deletion or creation patch should not change when we\n> transition to any hash function. Neither the permission bits shown\n> in the output (and whether the index line has the bits are shown on\n> it in the first place, i.e. the index line of a creation patch does\n> not, whilethe one in a modification patch does).\n> \n> So I am a bit ambivalent about this change.\n> \n> Perhaps have a filter that redacts, instead of removes, selected\n> pieces of information that are likely to change while hash\n> transition, and use that consistently to filter both the expected\n> output and the actual output before comparing?\n> \nKeeping the permission bits makes sense (I'd not thought of them when I \ncreated the patch) as we want to check that the file has the correct \npermissions. As for the all-zero object name, is it really worth leaving \nit in - if a file has been created or deleted then we'll still have \n/dev/null as the file name for one side or the other and the diff lines \nwill show it as well. As these tests are just to check the state of the \nindex then I'm not sure the hash values add anything. How do you feel \nabout a filter like\n\nsed \"/^\tindex/s/ [0-9a-f][0-9a-f]*\\.\\.[0-9a-f][0-9a-f]*/x/\"\n\nBest Wishes\n\nPhillip\n"},{"id":"339810","messageId":"xmqq606q1dpz.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"8f39e6b1-0031-5d97-f636-ae67cfdb3f1c@talktalk.net","subject":"Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-21T16:58:48Z","receivedAt":"2018-02-21T16:59:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Keeping the permission bits makes sense (I'd not thought of them when\n> I created the patch) as we want to check that the file has the correct\n> permissions. As for the all-zero object name, is it really worth\n> leaving it in - if a file has been created or deleted then we'll still\n> have /dev/null as the file name for one side or the other and the diff\n> lines will show it as well. As these tests are just to check the state\n> of the index then I'm not sure the hash values add anything. How do\n> you feel about a filter like\n>\n> sed \"/^\tindex/s/ [0-9a-f][0-9a-f]*\\.\\.[0-9a-f][0-9a-f]*/x/\"\n\nSomething like that, with perhaps the single 'x' replaced with\nsomething more normal looking and the all-zero thing special cased,\nwas what I had in mind.\n\nSpecial casing all-zero matters. I know that the current code uses\nall-zero on the missing side. The thing is, tests are about\nprotecting the correctness we currently have, and we want to catch\nthe next idiot that breaks the code to stop showing all-zero when\ntalking about the missing side.\n"},{"id":"340397","messageId":"20180227110404.16816-2-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 1/9] add -i: add function to format hunk header","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:03:56Z","receivedAt":"2018-02-27T11:04:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis code is duplicated in a couple of places so make it into a\nfunction as we're going to add some more callers shortly.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a7542..8ababa6453 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -751,6 +751,15 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" .\n+\t\t(($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\" +$n_ofs\" .\n+\t\t(($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -838,11 +847,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -912,11 +917,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n-- \n2.16.1\n\n"},{"id":"340398","messageId":"20180227110404.16816-5-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:03:59Z","receivedAt":"2018-02-27T11:04:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nUse a filter when comparing diffs to fix the value of non-zero hashes\nin diff index lines so we're not hard coding sha1 hash values in the\nexpected output. This makes it easier to change the expected output if\na test is edited as we don't need to worry about the exact hash value\nand means the tests will work when the hash algorithm is transitioned\naway from sha1.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - fix hash values in index lines rather than removing the line\n - reworded commit message\n\n t/t3701-add-interactive.sh | 30 ++++++++++++++++++++----------\n 1 file changed, 20 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex bdd1f292a9..46d655038f 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -10,6 +10,16 @@ then\n \ttest_done\n fi\n \n+diff_cmp () {\n+\tfor x\n+\tdo\n+\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n+\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n+\t\t \"$x\" >\"$x.filtered\"\n+\tdone\n+\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n+}\n+\n test_expect_success 'setup (initial)' '\n \techo content >file &&\n \tgit add file &&\n@@ -35,7 +45,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (initial)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/new file/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (initial)' '\n \tgit add file &&\n@@ -72,7 +82,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (commit)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/^index/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (commit)' '\n \tgit add file &&\n@@ -91,7 +101,7 @@ test_expect_success 'dummy edit works' '\n \ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup patch' '\n@@ -159,7 +169,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'real edit works' '\n \t(echo e; echo n; echo d) | git add -p &&\n \tgit diff >output &&\n-\ttest_cmp expected output\n+\tdiff_cmp expected output\n '\n \n test_expect_success 'skip files similarly as commit -a' '\n@@ -171,7 +181,7 @@ test_expect_success 'skip files similarly as commit -a' '\n \tgit reset &&\n \tgit commit -am commit &&\n \tgit diff >expected &&\n-\ttest_cmp expected output &&\n+\tdiff_cmp expected output &&\n \tgit reset --hard HEAD^\n '\n rm -f .gitignore\n@@ -248,7 +258,7 @@ test_expect_success 'add first line works' '\n \tgit apply patch &&\n \t(echo s; echo y; echo y) | git add -p file &&\n \tgit diff --cached > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -271,7 +281,7 @@ test_expect_success 'deleting a non-empty file' '\n \trm non-empty &&\n \techo y | git add -p non-empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -290,7 +300,7 @@ test_expect_success 'deleting an empty file' '\n \trm empty &&\n \techo y | git add -p empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'split hunk setup' '\n@@ -355,7 +365,7 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \t+changed\n \tEOF\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success TTY 'diffs can be colorized' '\n@@ -384,7 +394,7 @@ test_expect_success 'patch-mode via -i prompts for files' '\n \n \techo test >expect &&\n \tgit diff --cached --name-only >actual &&\n-\ttest_cmp expect actual\n+\tdiff_cmp expect actual\n '\n \n test_expect_success 'add -p handles globs' '\n-- \n2.16.1\n\n"},{"id":"340399","messageId":"20180227110404.16816-4-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 3/9] t3701: use test_write_lines and write_script","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:03:58Z","receivedAt":"2018-02-27T11:04:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSimplify things slightly by using the above helpers.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2\n - fixed use of test_set_editor to match what was in pu\n\n t/t3701-add-interactive.sh | 33 +++++----------------------------\n 1 file changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 861ea2e08c..bdd1f292a9 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n \tEOF\n '\n \n-test_expect_success 'setup fake editor' '\n-\t>fake_editor.sh &&\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n-'\n-\n test_expect_success 'dummy edit works' '\n+\ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n \ttest_cmp expected diff\n@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<-\\EOF &&\n+\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n \n@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '\n \n test_expect_success 'split hunk setup' '\n \tgit reset --hard &&\n-\tfor i in 10 20 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test &&\n+\ttest_write_lines 10 20 30 40 50 60 >test &&\n \tgit add test &&\n \ttest_tick &&\n \tgit commit -m test &&\n \n-\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test\n+\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n '\n \n test_expect_success 'split hunk \"add -p (edit)\"' '\n@@ -334,17 +321,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n '\n \n test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n-\tcat >test <<-\\EOF &&\n-\t5\n-\t10\n-\t20\n-\t21\n-\t30\n-\t31\n-\t40\n-\t50\n-\t60\n-\tEOF\n+\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n \t# q n q q is there to make sure we exit at the end.\n-- \n2.16.1\n\n"},{"id":"340400","messageId":"20180227110404.16816-6-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 5/9] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:04:00Z","receivedAt":"2018-02-27T11:04:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a hunk is skipped by add -i the offsets of subsequent hunks are\nnot adjusted to account for any missing insertions due to the skipped\nhunk. Most of the time this does not matter as apply uses the context\nlines to apply the subsequent hunks in the correct place, however in\npathological cases the context lines will match at the now incorrect\noffset and the hunk will be applied in the wrong place. The offsets of\nhunks following an edited hunk that has had the number of insertions\nor deletions changed also need to be updated in the same way. Add\nfailing tests to demonstrate this.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - removed test_set_editor as it is already set\n \n changes since v1:\n - changed edit test to use the existing fake editor and to strip\n the hunk header and some context lines as well as deleting an\n insertion\n - style fixes\n\n t/t3701-add-interactive.sh | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 46d655038f..da73fbdf4d 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -528,4 +528,34 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n \t! grep dirty-otherwise output\n '\n \n+test_expect_success 'set up pathological context' '\n+\tgit reset --hard &&\n+\ttest_write_lines a a a a a a a a a a a >a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\ttest_write_lines c b a a a a a a a b a a a a >a &&\n+\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n+\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n+\t# check editing can cope with missing header and deleted context lines\n+\t# as well as changes to other lines\n+\ttest_write_lines +b \" a\" >patch\n+'\n+\n+test_expect_failure 'add -p works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" n y |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-1 actual\n+'\n+\n+test_expect_failure 'add -p patch editing works with pathological context lines' '\n+\tgit reset &&\n+\t# n q q below is in case edit fails\n+\tprintf \"%s\\n\" e y n q q |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.1\n\n"},{"id":"340401","messageId":"20180227110404.16816-10-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:04:04Z","receivedAt":"2018-02-27T11:04:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nNow that add -p counts patches properly it should be possible to turn\noff the '--recount' option when invoking 'git apply'\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n I can't think of a reason why this shouldn't be OK but I can't help\n feeling slightly nervous about it. I've made it a separate patch so it\n can be easily dropped or reverted if I've missed something.\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3226c2c4f0..a64c0db57d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -677,7 +677,7 @@ sub add_untracked_cmd {\n sub run_git_apply {\n \tmy $cmd = shift;\n \tmy $fh;\n-\topen $fh, '| git ' . $cmd . \" --recount --allow-overlap\";\n+\topen $fh, '| git ' . $cmd . \" --allow-overlap\";\n \tprint $fh @_;\n \treturn close $fh;\n }\n-- \n2.16.1\n\n"},{"id":"340402","messageId":"20180227110404.16816-1-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH v3 0/9] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:03:55Z","receivedAt":"2018-02-27T11:04:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI've fixed the use of test_set_editor most of which was already in pu\nand reworked the sha1 comparisons to rewrite the hashes in the index\nlines rather than deleting the index lines.\n\nCover letter to v1:\n\nWhile working on a patch series to stage selected lines from a hunk\nwithout having to edit it I got worried that subsequent patches would\nbe applied in the wrong place which lead to this series to correct the\noffsets of hunks following those that are skipped or edited.\n\nInterdiff:\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 0fb9c0e0f1..05540ee9ef 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -10,6 +10,16 @@ then\n \ttest_done\n fi\n \n+diff_cmp () {\n+\tfor x\n+\tdo\n+\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n+\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n+\t\t \"$x\" >\"$x.filtered\"\n+\tdone\n+\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n+}\n+\n test_expect_success 'setup (initial)' '\n \techo content >file &&\n \tgit add file &&\n@@ -24,6 +34,7 @@ test_expect_success 'status works (initial)' '\n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tnew file mode 100644\n+\tindex 0000000..d95f3ad\n \t--- /dev/null\n \t+++ b/file\n \t@@ -0,0 +1 @@\n@@ -33,8 +44,8 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'diff works (initial)' '\n \t(echo d; echo 1) | git add -i >output &&\n-\tsed -ne /^index/d -e \"/new file/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tsed -ne \"/new file/,/content/p\" diff &&\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (initial)' '\n \tgit add file &&\n@@ -59,6 +70,7 @@ test_expect_success 'status works (commit)' '\n \n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1 +1,2 @@\n@@ -69,8 +81,8 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'diff works (commit)' '\n \t(echo d; echo 1) | git add -i >output &&\n-\tsed -ne \"/^---/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tsed -ne \"/^index/,/content/p\" diff &&\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (commit)' '\n \tgit add file &&\n@@ -88,8 +100,8 @@ test_expect_success 'setup expected' '\n test_expect_success 'dummy edit works' '\n \ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n-\tgit diff | sed /^index/d >diff &&\n-\ttest_cmp expected diff\n+\tgit diff > diff &&\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup patch' '\n@@ -103,12 +115,11 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\tFAKE_EDITOR=\"$(pwd)/fake-editor.sh\" &&\n-\twrite_script \"$FAKE_EDITOR\" <<-\\EOF &&\n+\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\ttest_set_editor \"$FAKE_EDITOR\"\n+\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n \n test_expect_success 'bad edit rejected' '\n@@ -143,6 +154,7 @@ test_expect_success 'setup patch' '\n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/file b/file\n+\tindex b5dd6c9..f910ae9 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1,4 +1,4 @@\n@@ -156,8 +168,8 @@ test_expect_success 'setup expected' '\n \n test_expect_success 'real edit works' '\n \t(echo e; echo n; echo d) | git add -p &&\n-\tgit diff | sed /^index/d >output &&\n-\ttest_cmp expected output\n+\tgit diff >output &&\n+\tdiff_cmp expected output\n '\n \n test_expect_success 'skip files similarly as commit -a' '\n@@ -165,11 +177,11 @@ test_expect_success 'skip files similarly as commit -a' '\n \techo file >.gitignore &&\n \techo changed >file &&\n \techo y | git add -p file &&\n-\tgit diff | sed /^index/d >output &&\n+\tgit diff >output &&\n \tgit reset &&\n \tgit commit -am commit &&\n-\tgit diff | sed /^index/d >expected &&\n-\ttest_cmp expected output &&\n+\tgit diff >expected &&\n+\tdiff_cmp expected output &&\n \tgit reset --hard HEAD^\n '\n rm -f .gitignore\n@@ -214,6 +226,7 @@ test_expect_success 'setup again' '\n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n \tcat >patch <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n@@ -228,7 +241,7 @@ test_expect_success 'setup patch' '\n # Expected output, diff is similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n \techo diff --git a/file b/file >expected &&\n-\tcat patch >>expected &&\n+\tcat patch |sed \"/^index/s/ 100644/ 100755/\" >>expected &&\n \tcat >expected-output <<-EOF\n \t--- a/file\n \t+++ b/file\n@@ -258,8 +271,8 @@ test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n \t\tsed -n -e \"s/^Stage this hunk[^@]*\\(@@ .*\\)/\\1/\" \\\n \t\t -e \"/^[-+@ \\\\\\\\]\"/p >output &&\n \ttest_must_be_empty error &&\n-\tgit diff --cached | sed /^index/d >diff &&\n-\ttest_cmp expected diff &&\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff &&\n \ttest_cmp expected-output output\n '\n \n@@ -267,6 +280,7 @@ test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/non-empty b/non-empty\n \tdeleted file mode 100644\n+\tindex d95f3ad..0000000\n \t--- a/non-empty\n \t+++ /dev/null\n \t@@ -1 +0,0 @@\n@@ -281,14 +295,15 @@ test_expect_success 'deleting a non-empty file' '\n \tgit commit -m non-empty &&\n \trm non-empty &&\n \techo y | git add -p non-empty &&\n-\tgit diff --cached | sed /^index/d >diff &&\n-\ttest_cmp expected diff\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n \tcat >expected <<-EOF\n \tdiff --git a/empty b/empty\n \tdeleted file mode 100644\n+\tindex e69de29..0000000\n \tEOF\n '\n \n@@ -299,8 +314,8 @@ test_expect_success 'deleting an empty file' '\n \tgit commit -m empty &&\n \trm empty &&\n \techo y | git add -p empty &&\n-\tgit diff --cached | sed /^index/d >diff &&\n-\ttest_cmp expected diff\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'split hunk setup' '\n@@ -326,7 +341,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n \t# exits.\n \tprintf \"%s\\n\" s e q n q q |\n \tEDITOR=: git add -p &&\n-\tgit diff | sed /^index/d >actual &&\n+\tgit diff >actual &&\n \t! grep \"^+15\" actual\n '\n \n@@ -338,7 +353,7 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \tprintf \"%s\\n\" s n y e q n q q |\n \tEDITOR=: git add -p 2>error &&\n \ttest_must_be_empty error &&\n-\tgit diff | sed /^index/d >actual &&\n+\tgit diff >actual &&\n \t! grep \"^+31\" actual\n '\n \n@@ -357,14 +372,15 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \tcat >expected <<-\\EOF &&\n \t* Unmerged path conflict.t\n \tdiff --git a/non-conflict.t b/non-conflict.t\n+\tindex f766221..5ea2ed4 100644\n \t--- a/non-conflict.t\n \t+++ b/non-conflict.t\n \t@@ -1 +1 @@\n \t-non-conflict\n \t+changed\n \tEOF\n-\tgit diff --cached | sed /^index/d >diff &&\n-\ttest_cmp expected diff\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff\n '\n \n test_expect_success TTY 'diffs can be colorized' '\n@@ -393,7 +409,7 @@ test_expect_success 'patch-mode via -i prompts for files' '\n \n \techo test >expect &&\n \tgit diff --cached --name-only >actual &&\n-\ttest_cmp expect actual\n+\tdiff_cmp expect actual\n '\n \n test_expect_success 'add -p handles globs' '\n@@ -550,7 +566,6 @@ test_expect_success 'add -p works with pathological context lines' '\n \n test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n-\ttest_set_editor \"$FAKE_EDITOR\" &&\n \t# n q q below is in case edit fails\n \tprintf \"%s\\n\" e y n q q |\n \tgit add -p &&\n\nPhillip Wood (9):\n add -i: add function to format hunk header\n t3701: indent here documents\n t3701: use test_write_lines and write_script\n t3701: don't hard code sha1 hash values\n t3701: add failing test for pathological context lines\n add -p: Adjust offsets of subsequent hunks when one is skipped\n add -p: calculate offset delta for edited patches\n add -p: fix counting when splitting and coalescing\n add -p: don't rely on apply's '--recount' option\n\n git-add--interactive.perl | 106 +++++++++++++----\n t/t3701-add-interactive.sh | 288 +++++++++++++++++++++++++--------------------\n 2 files changed, 240 insertions(+), 154 deletions(-)\n\n-- \n2.16.1\n\n"},{"id":"340403","messageId":"20180227110404.16816-3-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:03:57Z","receivedAt":"2018-02-27T11:04:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIndent here documents in line with the current style for tests.\n\nSigned-off-by: Phillip Wood \n---\n t/t3701-add-interactive.sh | 174 ++++++++++++++++++++++-----------------------\n 1 file changed, 87 insertions(+), 87 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 058698df6a..861ea2e08c 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tnew file mode 100644\n+\tindex 0000000..d95f3ad\n+\t--- /dev/null\n+\t+++ b/file\n+\t@@ -0,0 +1 @@\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (initial)' '\n@@ -59,14 +59,14 @@ test_expect_success 'status works (commit)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,2 @@\n+\t baseline\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (commit)' '\n@@ -83,8 +83,8 @@ test_expect_success 'revert works (commit)' '\n \n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n@@ -100,21 +100,21 @@ test_expect_success 'dummy edit works' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\t@@ -1,1 +1,4 @@\n+\t this\n+\t+patch\n+\t-does not\n+\t apply\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n \techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<\\EOF &&\n-mv -f \"$1\" oldpatch &&\n-mv -f patch \"$1\"\n-EOF\n+\tcat >>fake_editor.sh <<-\\EOF &&\n+\tmv -f \"$1\" oldpatch &&\n+\tmv -f patch \"$1\"\n+\tEOF\n \tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n@@ -126,10 +126,10 @@ test_expect_success 'bad edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\tthis patch\n+\tis garbage\n+\tEOF\n '\n \n test_expect_success 'garbage edit rejected' '\n@@ -139,28 +139,28 @@ test_expect_success 'garbage edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\t@@ -1,0 +1,0 @@\n+\t baseline\n+\t+content\n+\t+newcontent\n+\t+lines\n+\tEOF\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/file b/file\n+\tindex b5dd6c9..f910ae9 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,4 +1,4 @@\n+\t baseline\n+\t content\n+\t-newcontent\n+\t+more\n+\t lines\n+\tEOF\n '\n \n test_expect_success 'real edit works' '\n@@ -222,31 +222,31 @@ test_expect_success 'setup again' '\n \n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n-cat >patch <patch <<-EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Expected output, similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/file b/file\n+\tindex b6f2c08..61b9053 100755\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Test splitting the first patch, then adding both\n@@ -259,15 +259,15 @@ test_expect_success 'add first line works' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/non-empty b/non-empty\n+\tdeleted file mode 100644\n+\tindex d95f3ad..0000000\n+\t--- a/non-empty\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-content\n+\tEOF\n '\n \n test_expect_success 'deleting a non-empty file' '\n@@ -282,11 +282,11 @@ test_expect_success 'deleting a non-empty file' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-EOF\n+\tdiff --git a/empty b/empty\n+\tdeleted file mode 100644\n+\tindex e69de29..0000000\n+\tEOF\n '\n \n test_expect_success 'deleting an empty file' '\n-- \n2.16.1\n\n"},{"id":"340404","messageId":"20180227110404.16816-7-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:04:01Z","receivedAt":"2018-02-27T11:04:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince commit 8cbd431082 (\"git-add--interactive: replace hunk\nrecounting with apply --recount\", 2008-7-2) if a hunk is skipped then\nwe rely on the context lines to apply subsequent hunks in the right\nplace. While this works most of the time it is possible for hunks to\nend up being applied in the wrong place. To fix this adjust the offset\nof subsequent hunks to correct for any change in the number of\ninsertions or deletions due to the skipped hunk. The change in offset\ndue to edited hunks that have the number of insertions or deletions\nchanged is ignored here, it will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 +++++++++++++--\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8ababa6453..7a0a5896bb 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {\n \tmy @out = ();\n \n \tmy ($last_o_ctx, $last_was_dirty);\n+\tmy $ofs_delta = 0;\n \n-\tfor (grep { $_->{USE} } @in) {\n+\tfor (@in) {\n \t\tif ($_->{TYPE} ne 'hunk') {\n \t\t\tpush @out, $_;\n \t\t\tnext;\n \t\t}\n \t\tmy $text = $_->{TEXT};\n-\t\tmy ($o_ofs) = parse_hunk_header($text->[0]);\n+\t\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =\n+\t\t\t\t\t\tparse_hunk_header($text->[0]);\n+\t\tunless ($_->{USE}) {\n+\t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($ofs_delta) {\n+\t\t\t$n_ofs += $ofs_delta;\n+\t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t $n_ofs, $n_cnt);\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex da73fbdf4d..4c30aa6b6e 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -541,7 +541,7 @@ test_expect_success 'set up pathological context' '\n \ttest_write_lines +b \" a\" >patch\n '\n \n-test_expect_failure 'add -p works with pathological context lines' '\n+test_expect_success 'add -p works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" n y |\n \tgit add -p &&\n-- \n2.16.1\n\n"},{"id":"340405","messageId":"20180227110404.16816-8-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 7/9] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:04:02Z","receivedAt":"2018-02-27T11:04:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRecount the number of preimage and postimage lines in a hunk after it\nhas been edited so any change in the number of insertions or deletions\ncan be used to adjust the offsets of subsequent hunks. If an edited\nhunk is subsequently split then the offset correction will be lost. It\nwould be possible to fix this if it is a problem, however the code\nhere is still an improvement on the status quo for the common case\nwhere an edited hunk is applied without being split.\n\nThis is also a necessary step to removing '--recount' and\n'--allow-overlap' from the invocation of 'git apply'. Before\n'--recount' can be removed the splitting and coalescing counting needs\nto be fixed to handle a missing newline at the end of a file. In order\nto remove '--allow-overlap' there needs to be i) some way of verifying\nthe offset data in the edited hunk (probably by correlating the\npreimage (or postimage if the patch is going to be applied in reverse)\nlines of the edited and unedited versions to see if they are offset or\nif any leading/trailing context lines have been removed) and ii) a way of\ndealing with edited hunks that change context lines that are shared\nwith neighbouring hunks.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v1\n - edited hunks are now recounted before seeing if they apply in\n preparation for removing '--recount' when invoking 'git apply'\n - added sentence to commit message about the offset data being lost\n if an edited hunk is split\n\n git-add--interactive.perl | 55 ++++++++++++++++++++++++++++++++++++++--------\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 47 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7a0a5896bb..0df0c2aa06 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {\n \t\t\t\t\t\tparse_hunk_header($text->[0]);\n \t\tunless ($_->{USE}) {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\t# If this hunk has been edited then subtract\n+\t\t\t# the delta that is due to the edit.\n+\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n \t\t\t$n_ofs += $ofs_delta;\n \t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n \t\t\t\t\t\t\t $n_ofs, $n_cnt);\n \t\t}\n+\t\t# If this hunk was edited then adjust the offset delta\n+\t\t# to reflect the edit.\n+\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n@@ -1016,6 +1022,30 @@ marked for discarding.\"),\n marked for applying.\"),\n );\n \n+sub recount_edited_hunk {\n+\tlocal $_;\n+\tmy ($oldtext, $newtext) = @_;\n+\tmy ($o_cnt, $n_cnt) = (0, 0);\n+\tfor (@{$newtext}[1..$#{$newtext}]) {\n+\t\tmy $mode = substr($_, 0, 1);\n+\t\tif ($mode eq '-') {\n+\t\t\t$o_cnt++;\n+\t\t} elsif ($mode eq '+') {\n+\t\t\t$n_cnt++;\n+\t\t} elsif ($mode eq ' ') {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, undef, $n_ofs, undef) =\n+\t\t\t\t\tparse_hunk_header($newtext->[0]);\n+\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n+\t# Return the change in the number of lines inserted by this hunk\n+\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n+}\n+\n sub edit_hunk_manually {\n \tmy ($oldtext) = @_;\n \n@@ -1114,25 +1144,32 @@ sub prompt_yesno {\n }\n \n sub edit_hunk_loop {\n-\tmy ($head, $hunk, $ix) = @_;\n-\tmy $text = $hunk->[$ix]->{TEXT};\n+\tmy ($head, $hunks, $ix) = @_;\n+\tmy $hunk = $hunks->[$ix];\n+\tmy $text = $hunk->{TEXT};\n \n \twhile (1) {\n-\t\t$text = edit_hunk_manually($text);\n-\t\tif (!defined $text) {\n+\t\tmy $newtext = edit_hunk_manually($text);\n+\t\tif (!defined $newtext) {\n \t\t\treturn undef;\n \t\t}\n \t\tmy $newhunk = {\n-\t\t\tTEXT => $text,\n-\t\t\tTYPE => $hunk->[$ix]->{TYPE},\n+\t\t\tTEXT => $newtext,\n+\t\t\tTYPE => $hunk->{TYPE},\n \t\t\tUSE => 1,\n \t\t\tDIRTY => 1,\n \t\t};\n+\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n+\t\t# If this hunk has already been edited then add the\n+\t\t# offset delta of the previous edit to get the real\n+\t\t# delta from the original unedited hunk.\n+\t\t$hunk->{OFS_DELTA} and\n+\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n \t\tif (diff_applies($head,\n-\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t @{$hunks}[0..$ix-1],\n \t\t\t\t $newhunk,\n-\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n-\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\t\t @{$hunks}[$ix+1..$#{$hunks}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$newtext})];\n \t\t\treturn $newhunk;\n \t\t}\n \t\telse {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 4c30aa6b6e..83f4c8d6ea 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -549,7 +549,7 @@ test_expect_success 'add -p works with pathological context lines' '\n \ttest_cmp expected-1 actual\n '\n \n-test_expect_failure 'add -p patch editing works with pathological context lines' '\n+test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n \t# n q q below is in case edit fails\n \tprintf \"%s\\n\" e y n q q |\n-- \n2.16.1\n\n"},{"id":"340406","messageId":"20180227110404.16816-9-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180227110404.16816-1-phillip.wood@talktalk.net","subject":"[PATCH v3 8/9] add -p: fix counting when splitting and coalescing","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-27T11:04:03Z","receivedAt":"2018-02-27T11:04:56Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a file has no trailing new line at the end diff records this by\nappending \"\\ No newline at end of file\" below the last line of the\nfile. This line should not be counted in the hunk header. Fix the\nsplitting and coalescing code to count files without a trailing new line\nproperly and change one of the tests to test splitting without a\ntrailing new line.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 13 +++++++++----\n t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++--------\n 2 files changed, 32 insertions(+), 12 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 0df0c2aa06..3226c2c4f0 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -793,6 +793,11 @@ sub split_hunk {\n \t\twhile (++$i < @$text) {\n \t\t\tmy $line = $text->[$i];\n \t\t\tmy $display = $display->[$i];\n+\t\t\tif ($line =~ /^\\\\/) {\n+\t\t\t\tpush @{$this->{TEXT}}, $line;\n+\t\t\t\tpush @{$this->{DISPLAY}}, $display;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\tif ($line =~ /^ /) {\n \t\t\t\tif ($this->{ADDDEL} &&\n \t\t\t\t !defined $next_hunk_start) {\n@@ -887,8 +892,8 @@ sub merge_hunk {\n \t$o_cnt = $n_cnt = 0;\n \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n \t\tmy $line = $prev->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\n@@ -905,8 +910,8 @@ sub merge_hunk {\n \n \tfor ($i = 1; $i < @{$this->{TEXT}}; $i++) {\n \t\tmy $line = $this->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 83f4c8d6ea..05540ee9ef 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -234,31 +234,46 @@ test_expect_success 'setup patch' '\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n-# Expected output, similar to the patch but w/ diff at the top\n+# Expected output, diff is similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n-\tdiff --git a/file b/file\n-\tindex b6f2c08..61b9053 100755\n+\techo diff --git a/file b/file >expected &&\n+\tcat patch |sed \"/^index/s/ 100644/ 100755/\" >>expected &&\n+\tcat >expected-output <<-EOF\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n \t+firstline\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n+\t@@ -1,2 +1,3 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t@@ -1,2 +2,3 @@\n+\t baseline\n+\t content\n+\t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n # Test splitting the first patch, then adding both\n-test_expect_success 'add first line works' '\n+test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n \tgit commit -am \"clear local changes\" &&\n \tgit apply patch &&\n-\t(echo s; echo y; echo y) | git add -p file &&\n-\tgit diff --cached > diff &&\n-\tdiff_cmp expected diff\n+\tprintf \"%s\\n\" s y y | git add -p file 2>error |\n+\t\tsed -n -e \"s/^Stage this hunk[^@]*\\(@@ .*\\)/\\1/\" \\\n+\t\t -e \"/^[-+@ \\\\\\\\]\"/p >output &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff &&\n+\ttest_cmp expected-output output\n '\n \n test_expect_success 'setup expected' '\n-- \n2.16.1\n\n"},{"id":"340477","messageId":"xmqqo9kanjrk.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180227110404.16816-3-phillip.wood@talktalk.net","subject":"Re: [PATCH v3 2/9] t3701: indent here documents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-27T22:35:27Z","receivedAt":"2018-02-27T22:35:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Indent here documents in line with the current style for tests.\n>\n> Signed-off-by: Phillip Wood \n> ---\n\nThis loses the hand-edit-while-queuing done based on Eric's comment\nfor the previous round (see what has been queued on 'pu' for quite a\nwhile), which is not very much appreciated.\n\nAlso you lost the title fix done while queuing on 6/9 (see 'pu').\n"},{"id":"340480","messageId":"xmqqinainjfy.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180227110404.16816-5-phillip.wood@talktalk.net","subject":"Re: [PATCH v3 4/9] t3701: don't hard code sha1 hash values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-27T22:42:25Z","receivedAt":"2018-02-27T22:42:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> t/t3701-add-interactive.sh | 30 ++++++++++++++++++++----------\n> 1 file changed, 20 insertions(+), 10 deletions(-)\n>\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index bdd1f292a9..46d655038f 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -10,6 +10,16 @@ then\n> \ttest_done\n> fi\n> \n> +diff_cmp () {\n> +\tfor x\n> +\tdo\n> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n> +\t\t \"$x\" >\"$x.filtered\"\n\nInteresting ;-) You require .. and on the left hand side you want\nto see a run of hexdec with at least one non-zero hexdigit, which is\nfiltered to fixed-length 1234567; right hand side is the same deal.\n\nWhich sounds like a reasonable way to future-proof the comparison.\n\nIf 7 zeros are expected in the result, and the actual output had 8\nzeros, the filter does not touch either so they compare differently,\nwhich is somewhat unfortunate. Perhaps something like\n\n\t/^index/s/^00*\\.\\./0000000../\n\t/^index/s/\\([^0-9a-f]\\)00*\\.\\./\\10000000../\n\t/^index/s/\\.\\.00*$/..0000000/\n\t/^index/s/\\.\\.00*\\([^0-9a-f]\\)/..0000000\\1/\n\nafter the above two patterns help?\n"},{"id":"340551","messageId":"12a03e03-640e-90c8-0ac8-14807a1834c9@talktalk.net","threadId":"47825","inReplyTo":"xmqqo9kanjrk.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-28T11:00:21Z","receivedAt":"2018-02-28T11:00:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 27/02/18 22:35, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> Indent here documents in line with the current style for tests.\n>>\n>> Signed-off-by: Phillip Wood \n>> ---\n> \n> This loses the hand-edit-while-queuing done based on Eric's comment\n> for the previous round (see what has been queued on 'pu' for quite a\n> while), which is not very much appreciated.\n\nSorry as you had not commented on that patch I didn't realize you had\nedited it before queueing it. (There are probably other here documents\nin that file that were already indented that would benefit from Eric's\nsuggestion.)\n\n> Also you lost the title fix done while queuing on 6/9 (see 'pu').\n> \nIs there an easy way for contributors to compare the branch they post to\nwhat ends up it pu?\n\nBest Wishes\n\nPhillip\n"},{"id":"340553","messageId":"454d7edf-6331-2ca9-8113-de383aa5df4a@talktalk.net","threadId":"47825","inReplyTo":"xmqqinainjfy.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-28T11:03:06Z","receivedAt":"2018-02-28T11:03:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 27/02/18 22:42, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> t/t3701-add-interactive.sh | 30 ++++++++++++++++++++----------\n>> 1 file changed, 20 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n>> index bdd1f292a9..46d655038f 100755\n>> --- a/t/t3701-add-interactive.sh\n>> +++ b/t/t3701-add-interactive.sh\n>> @@ -10,6 +10,16 @@ then\n>> \ttest_done\n>> fi\n>> \n>> +diff_cmp () {\n>> +\tfor x\n>> +\tdo\n>> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n>> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n>> +\t\t \"$x\" >\"$x.filtered\"\n> \n> Interesting ;-) You require .. and on the left hand side you want\n> to see a run of hexdec with at least one non-zero hexdigit, which is\n> filtered to fixed-length 1234567; right hand side is the same deal.\n> \n> Which sounds like a reasonable way to future-proof the comparison.\n> \n> If 7 zeros are expected in the result, and the actual output had 8\n> zeros, the filter does not touch either so they compare differently,\n> which is somewhat unfortunate. Perhaps something like\n\nAh, good point\n\n> \t/^index/s/^00*\\.\\./0000000../\n> \t/^index/s/\\([^0-9a-f]\\)00*\\.\\./\\10000000../\n> \t/^index/s/\\.\\.00*$/..0000000/\n> \t/^index/s/\\.\\.00*\\([^0-9a-f]\\)/..0000000\\1/\n> \n> after the above two patterns help?\n\nYeah, something like that though matching the beginning and end of the\nline for the beginning and end of the hashes wont work. I'll reroll with\nsomething similar\n\nThanks\n\nPhillip\n\n"},{"id":"340554","messageId":"a44c4432-ea10-bf0b-7014-bebee13eef3c@talktalk.net","threadId":"47825","inReplyTo":"454d7edf-6331-2ca9-8113-de383aa5df4a@talktalk.net","subject":"Re: [PATCH v3 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-28T11:10:27Z","receivedAt":"2018-02-28T11:10:34Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 28/02/18 11:03, Phillip Wood wrote:\n> On 27/02/18 22:42, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>>\n>>> t/t3701-add-interactive.sh | 30 ++++++++++++++++++++----------\n>>> 1 file changed, 20 insertions(+), 10 deletions(-)\n>>>\n>>> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n>>> index bdd1f292a9..46d655038f 100755\n>>> --- a/t/t3701-add-interactive.sh\n>>> +++ b/t/t3701-add-interactive.sh\n>>> @@ -10,6 +10,16 @@ then\n>>> \ttest_done\n>>> fi\n>>> \n>>> +diff_cmp () {\n>>> +\tfor x\n>>> +\tdo\n>>> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n>>> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n>>> +\t\t \"$x\" >\"$x.filtered\"\n>>\n>> Interesting ;-) You require .. and on the left hand side you want\n>> to see a run of hexdec with at least one non-zero hexdigit, which is\n>> filtered to fixed-length 1234567; right hand side is the same deal.\n>>\n>> Which sounds like a reasonable way to future-proof the comparison.\n>>\n>> If 7 zeros are expected in the result, and the actual output had 8\n>> zeros, the filter does not touch either so they compare differently,\n>> which is somewhat unfortunate. Perhaps something like\n> \n> Ah, good point\n> \n>> \t/^index/s/^00*\\.\\./0000000../\n>> \t/^index/s/\\([^0-9a-f]\\)00*\\.\\./\\10000000../\n>> \t/^index/s/\\.\\.00*$/..0000000/\n>> \t/^index/s/\\.\\.00*\\([^0-9a-f]\\)/..0000000\\1/\n>>\n>> after the above two patterns help?\n> \n> Yeah, something like that though matching the beginning and end of the\n> line for the beginning and end of the hashes wont work. I'll reroll with\n> something similar\n\nThinking about it some more, just using your last three patterns should\ndo the job. - I'll test and reroll.\n\n> Thanks\n> \n> Phillip\n> \n\n"},{"id":"340569","messageId":"xmqqsh9lm8fl.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"12a03e03-640e-90c8-0ac8-14807a1834c9@talktalk.net","subject":"Re: [PATCH v3 2/9] t3701: indent here documents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-28T15:37:50Z","receivedAt":"2018-02-28T15:37:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Is there an easy way for contributors to compare the branch they post to\n> what ends up it pu?\n\nDistributed work is pretty much symmetric, so it can be done the\nsame way as one would review a rerolled series by another co-worker.\n\n $ git log --oneline --first-parent origin/master..origin/pu\n\nwould show merges of topic branches, so you can find the tip of the\ntopic of your earlier submission (it would be one $commit^2; call\nthat $topic). origin/master..$topic would be the one branch\n(i.e. what is in 'pu') to be compared.\n\nThe other branch to be compared is what you sent the previous one\nout of, or the new version of the patches.\n\nTo compare two branches, git://github.com/trast/tbdiff is one of the\neasier way. \n\nBefore I learned about the tool, I used to \"format-patch --stdout\"\non both branches, and ran \"diff -u\" between them, as a crude measure;\nit was more useful for spotting typofixes in the log messages than\ncode changes, before I got good at reading diff of diffs ;-).\n\nAlso, tentatively rebasing the two branches on a common base, and\nthen doing \"git diff $oldtopic~$N $newtopic~$N\" or something like\nthat for varying value of $N (and N==0 is a good way for final\nsanity checks).\n\n\n\n"},{"id":"340709","messageId":"20180301105103.24268-2-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 1/9] add -i: add function to format hunk header","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:55Z","receivedAt":"2018-03-01T10:51:37Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis code is duplicated in a couple of places so make it into a\nfunction as we're going to add some more callers shortly.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a7542..8ababa6453 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -751,6 +751,15 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" .\n+\t\t(($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\" +$n_ofs\" .\n+\t\t(($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -838,11 +847,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -912,11 +917,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n-- \n2.16.1\n\n"},{"id":"340710","messageId":"20180301105103.24268-1-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH v4 0/9] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:54Z","receivedAt":"2018-03-01T10:51:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI've fixed the second patch to match what was in pu and added some\nextra code to patch 4 to handle zero sha1 hashes where the length\nvaries.\n\nCover letter to v1:\n\nWhile working on a patch series to stage selected lines from a hunk\nwithout having to edit it I got worried that subsequent patches would\nbe applied in the wrong place which lead to this series to correct the\noffsets of hunks following those that are skipped or edited.\n\nInterdiff to v3:\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 05540ee9ef..a9a9478a29 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -15,6 +15,9 @@ diff_cmp () {\n \tdo\n \t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n \t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n+\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n+\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n+\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n \t\t \"$x\" >\"$x.filtered\"\n \tdone\n \ttest_cmp \"$1.filtered\" \"$2.filtered\"\n@@ -32,7 +35,7 @@ test_expect_success 'status works (initial)' '\n '\n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tnew file mode 100644\n \tindex 0000000..d95f3ad\n \t--- /dev/null\n@@ -69,7 +72,7 @@ test_expect_success 'status works (commit)' '\n '\n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n@@ -93,7 +96,7 @@ test_expect_success 'revert works (commit)' '\n \n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tEOF\n '\n \n@@ -105,7 +108,7 @@ test_expect_success 'dummy edit works' '\n '\n \n test_expect_success 'setup patch' '\n-\tcat >patch <<-EOF\n+\tcat >patch <<-\\EOF\n \t@@ -1,1 +1,4 @@\n \t this\n \t+patch\n@@ -129,7 +132,7 @@ test_expect_success 'bad edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-\tcat >patch <<-EOF\n+\tcat >patch <<-\\EOF\n \tthis patch\n \tis garbage\n \tEOF\n@@ -142,7 +145,7 @@ test_expect_success 'garbage edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-\tcat >patch <<-EOF\n+\tcat >patch <<-\\EOF\n \t@@ -1,0 +1,0 @@\n \t baseline\n \t+content\n@@ -152,7 +155,7 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tdiff --git a/file b/file\n \tindex b5dd6c9..f910ae9 100644\n \t--- a/file\n@@ -225,7 +228,7 @@ test_expect_success 'setup again' '\n \n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n-\tcat >patch <<-EOF\n+\tcat >patch <<-\\EOF\n \tindex 180b47c..b6f2c08 100644\n \t--- a/file\n \t+++ b/file\n@@ -242,7 +245,7 @@ test_expect_success 'setup patch' '\n test_expect_success 'setup expected' '\n \techo diff --git a/file b/file >expected &&\n \tcat patch |sed \"/^index/s/ 100644/ 100755/\" >>expected &&\n-\tcat >expected-output <<-EOF\n+\tcat >expected-output <<-\\EOF\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n@@ -277,7 +280,7 @@ test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n '\n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tdiff --git a/non-empty b/non-empty\n \tdeleted file mode 100644\n \tindex d95f3ad..0000000\n@@ -300,7 +303,7 @@ test_expect_success 'deleting a non-empty file' '\n '\n \n test_expect_success 'setup expected' '\n-\tcat >expected <<-EOF\n+\tcat >expected <<-\\EOF\n \tdiff --git a/empty b/empty\n \tdeleted file mode 100644\n \tindex e69de29..0000000\n\n\nPhillip Wood (9):\n add -i: add function to format hunk header\n t3701: indent here documents\n t3701: use test_write_lines and write_script\n t3701: don't hard code sha1 hash values\n t3701: add failing test for pathological context lines\n add -p: adjust offsets of subsequent hunks when one is skipped\n add -p: calculate offset delta for edited patches\n add -p: fix counting when splitting and coalescing\n add -p: don't rely on apply's '--recount' option\n\n git-add--interactive.perl | 106 +++++++++++++----\n t/t3701-add-interactive.sh | 291 +++++++++++++++++++++++++--------------------\n 2 files changed, 243 insertions(+), 154 deletions(-)\n\n-- \n2.16.1\n\n"},{"id":"340711","messageId":"20180301105103.24268-10-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:51:03Z","receivedAt":"2018-03-01T10:51:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nNow that add -p counts patches properly it should be possible to turn\noff the '--recount' option when invoking 'git apply'\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n I can't think of a reason why this shouldn't be OK but I can't help\n feeling slightly nervous about it. I've made it a separate patch so it\n can be easily dropped or reverted if I've missed something.\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3226c2c4f0..a64c0db57d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -677,7 +677,7 @@ sub add_untracked_cmd {\n sub run_git_apply {\n \tmy $cmd = shift;\n \tmy $fh;\n-\topen $fh, '| git ' . $cmd . \" --recount --allow-overlap\";\n+\topen $fh, '| git ' . $cmd . \" --allow-overlap\";\n \tprint $fh @_;\n \treturn close $fh;\n }\n-- \n2.16.1\n\n"},{"id":"340712","messageId":"20180301105103.24268-8-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 7/9] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:51:01Z","receivedAt":"2018-03-01T10:51:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRecount the number of preimage and postimage lines in a hunk after it\nhas been edited so any change in the number of insertions or deletions\ncan be used to adjust the offsets of subsequent hunks. If an edited\nhunk is subsequently split then the offset correction will be lost. It\nwould be possible to fix this if it is a problem, however the code\nhere is still an improvement on the status quo for the common case\nwhere an edited hunk is applied without being split.\n\nThis is also a necessary step to removing '--recount' and\n'--allow-overlap' from the invocation of 'git apply'. Before\n'--recount' can be removed the splitting and coalescing counting needs\nto be fixed to handle a missing newline at the end of a file. In order\nto remove '--allow-overlap' there needs to be i) some way of verifying\nthe offset data in the edited hunk (probably by correlating the\npreimage (or postimage if the patch is going to be applied in reverse)\nlines of the edited and unedited versions to see if they are offset or\nif any leading/trailing context lines have been removed) and ii) a way of\ndealing with edited hunks that change context lines that are shared\nwith neighbouring hunks.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v1\n - edited hunks are now recounted before seeing if they apply in\n preparation for removing '--recount' when invoking 'git apply'\n - added sentence to commit message about the offset data being lost\n if an edited hunk is split\n\n git-add--interactive.perl | 55 ++++++++++++++++++++++++++++++++++++++--------\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 47 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7a0a5896bb..0df0c2aa06 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {\n \t\t\t\t\t\tparse_hunk_header($text->[0]);\n \t\tunless ($_->{USE}) {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\t# If this hunk has been edited then subtract\n+\t\t\t# the delta that is due to the edit.\n+\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n \t\t\t$n_ofs += $ofs_delta;\n \t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n \t\t\t\t\t\t\t $n_ofs, $n_cnt);\n \t\t}\n+\t\t# If this hunk was edited then adjust the offset delta\n+\t\t# to reflect the edit.\n+\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n@@ -1016,6 +1022,30 @@ marked for discarding.\"),\n marked for applying.\"),\n );\n \n+sub recount_edited_hunk {\n+\tlocal $_;\n+\tmy ($oldtext, $newtext) = @_;\n+\tmy ($o_cnt, $n_cnt) = (0, 0);\n+\tfor (@{$newtext}[1..$#{$newtext}]) {\n+\t\tmy $mode = substr($_, 0, 1);\n+\t\tif ($mode eq '-') {\n+\t\t\t$o_cnt++;\n+\t\t} elsif ($mode eq '+') {\n+\t\t\t$n_cnt++;\n+\t\t} elsif ($mode eq ' ') {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, undef, $n_ofs, undef) =\n+\t\t\t\t\tparse_hunk_header($newtext->[0]);\n+\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n+\t# Return the change in the number of lines inserted by this hunk\n+\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n+}\n+\n sub edit_hunk_manually {\n \tmy ($oldtext) = @_;\n \n@@ -1114,25 +1144,32 @@ sub prompt_yesno {\n }\n \n sub edit_hunk_loop {\n-\tmy ($head, $hunk, $ix) = @_;\n-\tmy $text = $hunk->[$ix]->{TEXT};\n+\tmy ($head, $hunks, $ix) = @_;\n+\tmy $hunk = $hunks->[$ix];\n+\tmy $text = $hunk->{TEXT};\n \n \twhile (1) {\n-\t\t$text = edit_hunk_manually($text);\n-\t\tif (!defined $text) {\n+\t\tmy $newtext = edit_hunk_manually($text);\n+\t\tif (!defined $newtext) {\n \t\t\treturn undef;\n \t\t}\n \t\tmy $newhunk = {\n-\t\t\tTEXT => $text,\n-\t\t\tTYPE => $hunk->[$ix]->{TYPE},\n+\t\t\tTEXT => $newtext,\n+\t\t\tTYPE => $hunk->{TYPE},\n \t\t\tUSE => 1,\n \t\t\tDIRTY => 1,\n \t\t};\n+\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n+\t\t# If this hunk has already been edited then add the\n+\t\t# offset delta of the previous edit to get the real\n+\t\t# delta from the original unedited hunk.\n+\t\t$hunk->{OFS_DELTA} and\n+\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n \t\tif (diff_applies($head,\n-\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t @{$hunks}[0..$ix-1],\n \t\t\t\t $newhunk,\n-\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n-\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\t\t @{$hunks}[$ix+1..$#{$hunks}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$newtext})];\n \t\t\treturn $newhunk;\n \t\t}\n \t\telse {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex e3150a4e07..4cc9517eda 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -552,7 +552,7 @@ test_expect_success 'add -p works with pathological context lines' '\n \ttest_cmp expected-1 actual\n '\n \n-test_expect_failure 'add -p patch editing works with pathological context lines' '\n+test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n \t# n q q below is in case edit fails\n \tprintf \"%s\\n\" e y n q q |\n-- \n2.16.1\n\n"},{"id":"340713","messageId":"20180301105103.24268-4-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 3/9] t3701: use test_write_lines and write_script","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:57Z","receivedAt":"2018-03-01T10:51:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSimplify things slightly by using the above helpers.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2\n - fixed use of test_set_editor to match what was in pu\n\n t/t3701-add-interactive.sh | 33 +++++----------------------------\n 1 file changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 3130dafcf0..836ce346ed 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n \tEOF\n '\n \n-test_expect_success 'setup fake editor' '\n-\t>fake_editor.sh &&\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n-'\n-\n test_expect_success 'dummy edit works' '\n+\ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n \ttest_cmp expected diff\n@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<-\\EOF &&\n+\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n \n@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '\n \n test_expect_success 'split hunk setup' '\n \tgit reset --hard &&\n-\tfor i in 10 20 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test &&\n+\ttest_write_lines 10 20 30 40 50 60 >test &&\n \tgit add test &&\n \ttest_tick &&\n \tgit commit -m test &&\n \n-\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test\n+\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n '\n \n test_expect_success 'split hunk \"add -p (edit)\"' '\n@@ -334,17 +321,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n '\n \n test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n-\tcat >test <<-\\EOF &&\n-\t5\n-\t10\n-\t20\n-\t21\n-\t30\n-\t31\n-\t40\n-\t50\n-\t60\n-\tEOF\n+\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n \t# q n q q is there to make sure we exit at the end.\n-- \n2.16.1\n\n"},{"id":"340714","messageId":"20180301105103.24268-5-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:58Z","receivedAt":"2018-03-01T10:52:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nUse a filter when comparing diffs to fix the value of non-zero hashes\nin diff index lines so we're not hard coding sha1 hash values in the\nexpected output. This makes it easier to change the expected output if\na test is edited as we don't need to worry about the exact hash value\nand means the tests will work when the hash algorithm is transitioned\naway from sha1.\n\nThanks-to: Junio C Hamano \nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v3:\n - fix zero hash values to seven digits\n changes since v2:\n - fix hash values in index lines rather than removing the line\n - reworded commit message\n\n t/t3701-add-interactive.sh | 33 +++++++++++++++++++++++----------\n 1 file changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 836ce346ed..f95714230b 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -10,6 +10,19 @@ then\n \ttest_done\n fi\n \n+diff_cmp () {\n+\tfor x\n+\tdo\n+\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n+\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n+\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n+\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n+\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n+\t\t \"$x\" >\"$x.filtered\"\n+\tdone\n+\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n+}\n+\n test_expect_success 'setup (initial)' '\n \techo content >file &&\n \tgit add file &&\n@@ -35,7 +48,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (initial)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/new file/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (initial)' '\n \tgit add file &&\n@@ -72,7 +85,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (commit)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/^index/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (commit)' '\n \tgit add file &&\n@@ -91,7 +104,7 @@ test_expect_success 'dummy edit works' '\n \ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup patch' '\n@@ -159,7 +172,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'real edit works' '\n \t(echo e; echo n; echo d) | git add -p &&\n \tgit diff >output &&\n-\ttest_cmp expected output\n+\tdiff_cmp expected output\n '\n \n test_expect_success 'skip files similarly as commit -a' '\n@@ -171,7 +184,7 @@ test_expect_success 'skip files similarly as commit -a' '\n \tgit reset &&\n \tgit commit -am commit &&\n \tgit diff >expected &&\n-\ttest_cmp expected output &&\n+\tdiff_cmp expected output &&\n \tgit reset --hard HEAD^\n '\n rm -f .gitignore\n@@ -248,7 +261,7 @@ test_expect_success 'add first line works' '\n \tgit apply patch &&\n \t(echo s; echo y; echo y) | git add -p file &&\n \tgit diff --cached > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -271,7 +284,7 @@ test_expect_success 'deleting a non-empty file' '\n \trm non-empty &&\n \techo y | git add -p non-empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -290,7 +303,7 @@ test_expect_success 'deleting an empty file' '\n \trm empty &&\n \techo y | git add -p empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'split hunk setup' '\n@@ -355,7 +368,7 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \t+changed\n \tEOF\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success TTY 'diffs can be colorized' '\n@@ -384,7 +397,7 @@ test_expect_success 'patch-mode via -i prompts for files' '\n \n \techo test >expect &&\n \tgit diff --cached --name-only >actual &&\n-\ttest_cmp expect actual\n+\tdiff_cmp expect actual\n '\n \n test_expect_success 'add -p handles globs' '\n-- \n2.16.1\n\n"},{"id":"340715","messageId":"20180301105103.24268-3-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:56Z","receivedAt":"2018-03-01T10:52:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIndent here documents in line with the current style for tests.\nWhile at it, quote the end marker of here-docs that do not use\nvariable interpolation.\n\nSigned-off-by: Phillip Wood \nSigned-off-by: Junio C Hamano \n---\n\nNotes:\n changes since v3:\n - updated to match what was in pu\n\n t/t3701-add-interactive.sh | 174 ++++++++++++++++++++++-----------------------\n 1 file changed, 87 insertions(+), 87 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 058698df6a..3130dafcf0 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tnew file mode 100644\n+\tindex 0000000..d95f3ad\n+\t--- /dev/null\n+\t+++ b/file\n+\t@@ -0,0 +1 @@\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (initial)' '\n@@ -59,14 +59,14 @@ test_expect_success 'status works (commit)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,2 @@\n+\t baseline\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (commit)' '\n@@ -83,8 +83,8 @@ test_expect_success 'revert works (commit)' '\n \n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n@@ -100,21 +100,21 @@ test_expect_success 'dummy edit works' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\t@@ -1,1 +1,4 @@\n+\t this\n+\t+patch\n+\t-does not\n+\t apply\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n \techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<\\EOF &&\n-mv -f \"$1\" oldpatch &&\n-mv -f patch \"$1\"\n-EOF\n+\tcat >>fake_editor.sh <<-\\EOF &&\n+\tmv -f \"$1\" oldpatch &&\n+\tmv -f patch \"$1\"\n+\tEOF\n \tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n@@ -126,10 +126,10 @@ test_expect_success 'bad edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\tthis patch\n+\tis garbage\n+\tEOF\n '\n \n test_expect_success 'garbage edit rejected' '\n@@ -139,28 +139,28 @@ test_expect_success 'garbage edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\t@@ -1,0 +1,0 @@\n+\t baseline\n+\t+content\n+\t+newcontent\n+\t+lines\n+\tEOF\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/file b/file\n+\tindex b5dd6c9..f910ae9 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,4 +1,4 @@\n+\t baseline\n+\t content\n+\t-newcontent\n+\t+more\n+\t lines\n+\tEOF\n '\n \n test_expect_success 'real edit works' '\n@@ -222,31 +222,31 @@ test_expect_success 'setup again' '\n \n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Expected output, similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/file b/file\n+\tindex b6f2c08..61b9053 100755\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Test splitting the first patch, then adding both\n@@ -259,15 +259,15 @@ test_expect_success 'add first line works' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/non-empty b/non-empty\n+\tdeleted file mode 100644\n+\tindex d95f3ad..0000000\n+\t--- a/non-empty\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-content\n+\tEOF\n '\n \n test_expect_success 'deleting a non-empty file' '\n@@ -282,11 +282,11 @@ test_expect_success 'deleting a non-empty file' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/empty b/empty\n+\tdeleted file mode 100644\n+\tindex e69de29..0000000\n+\tEOF\n '\n \n test_expect_success 'deleting an empty file' '\n-- \n2.16.1\n\n"},{"id":"340716","messageId":"20180301105103.24268-6-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 5/9] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:50:59Z","receivedAt":"2018-03-01T10:52:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a hunk is skipped by add -i the offsets of subsequent hunks are\nnot adjusted to account for any missing insertions due to the skipped\nhunk. Most of the time this does not matter as apply uses the context\nlines to apply the subsequent hunks in the correct place, however in\npathological cases the context lines will match at the now incorrect\noffset and the hunk will be applied in the wrong place. The offsets of\nhunks following an edited hunk that has had the number of insertions\nor deletions changed also need to be updated in the same way. Add\nfailing tests to demonstrate this.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - removed test_set_editor as it is already set\n \n changes since v1:\n - changed edit test to use the existing fake editor and to strip\n the hunk header and some context lines as well as deleting an\n insertion\n - style fixes\n\n t/t3701-add-interactive.sh | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex f95714230b..094eeca405 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -531,4 +531,34 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n \t! grep dirty-otherwise output\n '\n \n+test_expect_success 'set up pathological context' '\n+\tgit reset --hard &&\n+\ttest_write_lines a a a a a a a a a a a >a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\ttest_write_lines c b a a a a a a a b a a a a >a &&\n+\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n+\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n+\t# check editing can cope with missing header and deleted context lines\n+\t# as well as changes to other lines\n+\ttest_write_lines +b \" a\" >patch\n+'\n+\n+test_expect_failure 'add -p works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" n y |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-1 actual\n+'\n+\n+test_expect_failure 'add -p patch editing works with pathological context lines' '\n+\tgit reset &&\n+\t# n q q below is in case edit fails\n+\tprintf \"%s\\n\" e y n q q |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.1\n\n"},{"id":"340717","messageId":"20180301105103.24268-7-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 6/9] add -p: adjust offsets of subsequent hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:51:00Z","receivedAt":"2018-03-01T10:52:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince commit 8cbd431082 (\"git-add--interactive: replace hunk\nrecounting with apply --recount\", 2008-7-2) if a hunk is skipped then\nwe rely on the context lines to apply subsequent hunks in the right\nplace. While this works most of the time it is possible for hunks to\nend up being applied in the wrong place. To fix this adjust the offset\nof subsequent hunks to correct for any change in the number of\ninsertions or deletions due to the skipped hunk. The change in offset\ndue to edited hunks that have the number of insertions or deletions\nchanged is ignored here, it will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 +++++++++++++--\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8ababa6453..7a0a5896bb 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {\n \tmy @out = ();\n \n \tmy ($last_o_ctx, $last_was_dirty);\n+\tmy $ofs_delta = 0;\n \n-\tfor (grep { $_->{USE} } @in) {\n+\tfor (@in) {\n \t\tif ($_->{TYPE} ne 'hunk') {\n \t\t\tpush @out, $_;\n \t\t\tnext;\n \t\t}\n \t\tmy $text = $_->{TEXT};\n-\t\tmy ($o_ofs) = parse_hunk_header($text->[0]);\n+\t\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =\n+\t\t\t\t\t\tparse_hunk_header($text->[0]);\n+\t\tunless ($_->{USE}) {\n+\t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($ofs_delta) {\n+\t\t\t$n_ofs += $ofs_delta;\n+\t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t $n_ofs, $n_cnt);\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 094eeca405..e3150a4e07 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -544,7 +544,7 @@ test_expect_success 'set up pathological context' '\n \ttest_write_lines +b \" a\" >patch\n '\n \n-test_expect_failure 'add -p works with pathological context lines' '\n+test_expect_success 'add -p works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" n y |\n \tgit add -p &&\n-- \n2.16.1\n\n"},{"id":"340718","messageId":"20180301105103.24268-9-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180301105103.24268-1-phillip.wood@talktalk.net","subject":"[PATCH v4 8/9] add -p: fix counting when splitting and coalescing","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:51:02Z","receivedAt":"2018-03-01T10:52:16Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a file has no trailing new line at the end diff records this by\nappending \"\\ No newline at end of file\" below the last line of the\nfile. This line should not be counted in the hunk header. Fix the\nsplitting and coalescing code to count files without a trailing new line\nproperly and change one of the tests to test splitting without a\ntrailing new line.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 13 +++++++++----\n t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++--------\n 2 files changed, 32 insertions(+), 12 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 0df0c2aa06..3226c2c4f0 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -793,6 +793,11 @@ sub split_hunk {\n \t\twhile (++$i < @$text) {\n \t\t\tmy $line = $text->[$i];\n \t\t\tmy $display = $display->[$i];\n+\t\t\tif ($line =~ /^\\\\/) {\n+\t\t\t\tpush @{$this->{TEXT}}, $line;\n+\t\t\t\tpush @{$this->{DISPLAY}}, $display;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\tif ($line =~ /^ /) {\n \t\t\t\tif ($this->{ADDDEL} &&\n \t\t\t\t !defined $next_hunk_start) {\n@@ -887,8 +892,8 @@ sub merge_hunk {\n \t$o_cnt = $n_cnt = 0;\n \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n \t\tmy $line = $prev->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\n@@ -905,8 +910,8 @@ sub merge_hunk {\n \n \tfor ($i = 1; $i < @{$this->{TEXT}}; $i++) {\n \t\tmy $line = $this->{TEXT}[$i];\n-\t\tif ($line =~ /^\\+/) {\n-\t\t\t$n_cnt++;\n+\t\tif ($line =~ /^[+\\\\]/) {\n+\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 4cc9517eda..a9a9478a29 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -237,31 +237,46 @@ test_expect_success 'setup patch' '\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n-# Expected output, similar to the patch but w/ diff at the top\n+# Expected output, diff is similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-\tcat >expected <<-\\EOF\n-\tdiff --git a/file b/file\n-\tindex b6f2c08..61b9053 100755\n+\techo diff --git a/file b/file >expected &&\n+\tcat patch |sed \"/^index/s/ 100644/ 100755/\" >>expected &&\n+\tcat >expected-output <<-\\EOF\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n \t+firstline\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n+\t@@ -1,2 +1,3 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t@@ -1,2 +2,3 @@\n+\t baseline\n+\t content\n+\t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n # Test splitting the first patch, then adding both\n-test_expect_success 'add first line works' '\n+test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n \tgit commit -am \"clear local changes\" &&\n \tgit apply patch &&\n-\t(echo s; echo y; echo y) | git add -p file &&\n-\tgit diff --cached > diff &&\n-\tdiff_cmp expected diff\n+\tprintf \"%s\\n\" s y y | git add -p file 2>error |\n+\t\tsed -n -e \"s/^Stage this hunk[^@]*\\(@@ .*\\)/\\1/\" \\\n+\t\t -e \"/^[-+@ \\\\\\\\]\"/p >output &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff &&\n+\ttest_cmp expected-output output\n '\n \n test_expect_success 'setup expected' '\n-- \n2.16.1\n\n"},{"id":"340719","messageId":"30ff8d98-e86f-2eca-c7fb-25f537f9e3e5@talktalk.net","threadId":"47825","inReplyTo":"xmqqsh9lm8fl.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-01T10:57:39Z","receivedAt":"2018-03-01T10:58:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 28/02/18 15:37, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> Is there an easy way for contributors to compare the branch they post to\n>> what ends up it pu?\n> \n> Distributed work is pretty much symmetric, so it can be done the\n> same way as one would review a rerolled series by another co-worker.\n> \n> $ git log --oneline --first-parent origin/master..origin/pu\n> \n> would show merges of topic branches, so you can find the tip of the\n> topic of your earlier submission (it would be one $commit^2; call\n> that $topic). origin/master..$topic would be the one branch\n> (i.e. what is in 'pu') to be compared.\n> \n> The other branch to be compared is what you sent the previous one\n> out of, or the new version of the patches.\n> \n> To compare two branches, git://github.com/trast/tbdiff is one of the\n> easier way. \n> \n> Before I learned about the tool, I used to \"format-patch --stdout\"\n> on both branches, and ran \"diff -u\" between them, as a crude measure;\n> it was more useful for spotting typofixes in the log messages than\n> code changes, before I got good at reading diff of diffs ;-).\n> \n> Also, tentatively rebasing the two branches on a common base, and\n> then doing \"git diff $oldtopic~$N $newtopic~$N\" or something like\n> that for varying value of $N (and N==0 is a good way for final\n> sanity checks).\n\nThanks for the tips, tbdiff looks useful (I just need to learn to read\ndiffs of diffs!). I also find rebasing them on a common ancestor useful\nbut its a bit tedious.\n\nThanks again\n\nPhillip\n\n"},{"id":"340732","messageId":"xmqqinafiy84.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"30ff8d98-e86f-2eca-c7fb-25f537f9e3e5@talktalk.net","subject":"Re: [PATCH v3 2/9] t3701: indent here documents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-01T15:58:51Z","receivedAt":"2018-03-01T15:59:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Thanks for the tips, tbdiff looks useful (I just need to learn to read\n> diffs of diffs!). I also find rebasing them on a common ancestor useful\n> but its a bit tedious.\n\nYes, comparing two versions of a series is somewhat unusual and\nneeds getting used to before one can do so efficiently. You do\nnot have to always rebase (tbdiff is fairly good at what it does\neven when two ranges are far apart), but it helps not to begin\ndeveloping on a codebase that is newer than needed (e.g. a bugfix\non 'next' is unneeded unless you are fixing a bug introduced by a\ntopic that is still on 'next'---in which case the best fix is one\nthat is on that topic, without depending on the rest of 'next').\n\nIn any case, thank _you_ for contributing.\n\n"},{"id":"340754","messageId":"xmqqwoyvft92.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180301105103.24268-8-phillip.wood@talktalk.net","subject":"Re: [PATCH v4 7/9] add -p: calculate offset delta for edited patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-01T20:14:33Z","receivedAt":"2018-03-01T20:14:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Recount the number of preimage and postimage lines in a hunk after it\n> has been edited so any change in the number of insertions or deletions\n> can be used to adjust the offsets of subsequent hunks. If an edited\n> hunk is subsequently split then the offset correction will be lost. It\n> would be possible to fix this if it is a problem, however the code\n> here is still an improvement on the status quo for the common case\n> where an edited hunk is applied without being split.\n>\n> This is also a necessary step to removing '--recount' and\n> '--allow-overlap' from the invocation of 'git apply'. Before\n> '--recount' can be removed the splitting and coalescing counting needs\n> to be fixed to handle a missing newline at the end of a file. In order\n> to remove '--allow-overlap' there needs to be i) some way of verifying\n> the offset data in the edited hunk (probably by correlating the\n> preimage (or postimage if the patch is going to be applied in reverse)\n> lines of the edited and unedited versions to see if they are offset or\n> if any leading/trailing context lines have been removed) and ii) a way of\n> dealing with edited hunks that change context lines that are shared\n> with neighbouring hunks.\n>\n> Signed-off-by: Phillip Wood \n> ---\n\nThanks for clear description of what is going on in the series.\n\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 7a0a5896bb..0df0c2aa06 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {\n> \t\t\t\t\t\tparse_hunk_header($text->[0]);\n> \t\tunless ($_->{USE}) {\n> \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n> +\t\t\t# If this hunk has been edited then subtract\n> +\t\t\t# the delta that is due to the edit.\n> +\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n\nThe pattern\n\n\t<> and <>;\n\nis something you are newly introducing to this script. I am not\nsure if we want to see them. I somehow find them harder to read\nthan the more straight-forward and naïve\n\n\tif (<>) {\n\t\t<>;\n\t}\n\n\n> +\t\t# If this hunk was edited then adjust the offset delta\n> +\t\t# to reflect the edit.\n> +\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n\nLikewise.\n\n> +sub recount_edited_hunk {\n> +\tlocal $_;\n> +\tmy ($oldtext, $newtext) = @_;\n> +\tmy ($o_cnt, $n_cnt) = (0, 0);\n> +\tfor (@{$newtext}[1..$#{$newtext}]) {\n> +\t\tmy $mode = substr($_, 0, 1);\n> +\t\tif ($mode eq '-') {\n> +\t\t\t$o_cnt++;\n> +\t\t} elsif ($mode eq '+') {\n> +\t\t\t$n_cnt++;\n> +\t\t} elsif ($mode eq ' ') {\n> +\t\t\t$o_cnt++;\n> +\t\t\t$n_cnt++;\n> +\t\t}\n> +\t}\n> +\tmy ($o_ofs, undef, $n_ofs, undef) =\n> +\t\t\t\t\tparse_hunk_header($newtext->[0]);\n> +\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n> +\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n> +\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n> +\t# Return the change in the number of lines inserted by this hunk\n> +\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n> +}\n\nOK.\n\n> @@ -1114,25 +1144,32 @@ sub prompt_yesno {\n> }\n> \n> sub edit_hunk_loop {\n> -\tmy ($head, $hunk, $ix) = @_;\n> -\tmy $text = $hunk->[$ix]->{TEXT};\n> +\tmy ($head, $hunks, $ix) = @_;\n> +\tmy $hunk = $hunks->[$ix];\n> +\tmy $text = $hunk->{TEXT};\n> ...\n> +\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n> +\t\t# If this hunk has already been edited then add the\n> +\t\t# offset delta of the previous edit to get the real\n> +\t\t# delta from the original unedited hunk.\n> +\t\t$hunk->{OFS_DELTA} and\n> +\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n\nAhh, good point.\n\n"},{"id":"340756","messageId":"xmqqsh9jfsjd.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180301105103.24268-9-phillip.wood@talktalk.net","subject":"Re: [PATCH v4 8/9] add -p: fix counting when splitting and coalescing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-01T20:29:58Z","receivedAt":"2018-03-01T20:30:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> @@ -887,8 +892,8 @@ sub merge_hunk {\n> \t$o_cnt = $n_cnt = 0;\n> \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n> \t\tmy $line = $prev->{TEXT}[$i];\n> -\t\tif ($line =~ /^\\+/) {\n> -\t\t\t$n_cnt++;\n> +\t\tif ($line =~ /^[+\\\\]/) {\n> +\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n> \t\t\tpush @line, $line;\n> \t\t\tnext;\n> \t\t}\n\nHmmmm, the logic may be correct, but this looks like a result of\nattempting to minimize the number of changed lines and ending up\nwith a less-than-readble code. \"If the line begins with a plus or\nbackslash, do these things, the first of which is done only when\nthe line begins with a plus.\" The same comment for the other hunk\nthat counts the $this side.\n"},{"id":"340757","messageId":"xmqqo9k7fsht.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180301105103.24268-10-phillip.wood@talktalk.net","subject":"Re: [PATCH v4 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-01T20:30:54Z","receivedAt":"2018-03-01T20:31:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Now that add -p counts patches properly it should be possible to turn\n> off the '--recount' option when invoking 'git apply'\n\nSounds good ;-)\n"},{"id":"340790","messageId":"6fcd21f7-b7f7-4196-6404-463e867443cd@talktalk.net","threadId":"47825","inReplyTo":"xmqqwoyvft92.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 7/9] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-02T10:36:42Z","receivedAt":"2018-03-02T10:37:08Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 01/03/18 20:14, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> Recount the number of preimage and postimage lines in a hunk after it\n>> has been edited so any change in the number of insertions or deletions\n>> can be used to adjust the offsets of subsequent hunks. If an edited\n>> hunk is subsequently split then the offset correction will be lost. It\n>> would be possible to fix this if it is a problem, however the code\n>> here is still an improvement on the status quo for the common case\n>> where an edited hunk is applied without being split.\n>>\n>> This is also a necessary step to removing '--recount' and\n>> '--allow-overlap' from the invocation of 'git apply'. Before\n>> '--recount' can be removed the splitting and coalescing counting needs\n>> to be fixed to handle a missing newline at the end of a file. In order\n>> to remove '--allow-overlap' there needs to be i) some way of verifying\n>> the offset data in the edited hunk (probably by correlating the\n>> preimage (or postimage if the patch is going to be applied in reverse)\n>> lines of the edited and unedited versions to see if they are offset or\n>> if any leading/trailing context lines have been removed) and ii) a way of\n>> dealing with edited hunks that change context lines that are shared\n>> with neighbouring hunks.\n>>\n>> Signed-off-by: Phillip Wood \n>> ---\n> \n> Thanks for clear description of what is going on in the series.\n> \n>> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n>> index 7a0a5896bb..0df0c2aa06 100755\n>> --- a/git-add--interactive.perl\n>> +++ b/git-add--interactive.perl\n>> @@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {\n>> \t\t\t\t\t\tparse_hunk_header($text->[0]);\n>> \t\tunless ($_->{USE}) {\n>> \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n>> +\t\t\t# If this hunk has been edited then subtract\n>> +\t\t\t# the delta that is due to the edit.\n>> +\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n> \n> The pattern\n> \n> \t<> and <>;\n> \n> is something you are newly introducing to this script. I am not\n> sure if we want to see them. I somehow find them harder to read\n> than the more straight-forward and naïve\n> \n> \tif (<>) {\n> \t\t<>;\n> \t}\n> \n\nFair enough, I think I was suffering from brace fatigue when I wrote it,\nif you can hold off merging this into next I'll re-roll with \"if's\"\ninstead of \"and's\".\n\n>> +\t\t# If this hunk was edited then adjust the offset delta\n>> +\t\t# to reflect the edit.\n>> +\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n> \n> Likewise.\n> \n>> +sub recount_edited_hunk {\n>> +\tlocal $_;\n>> +\tmy ($oldtext, $newtext) = @_;\n>> +\tmy ($o_cnt, $n_cnt) = (0, 0);\n>> +\tfor (@{$newtext}[1..$#{$newtext}]) {\n>> +\t\tmy $mode = substr($_, 0, 1);\n>> +\t\tif ($mode eq '-') {\n>> +\t\t\t$o_cnt++;\n>> +\t\t} elsif ($mode eq '+') {\n>> +\t\t\t$n_cnt++;\n>> +\t\t} elsif ($mode eq ' ') {\n>> +\t\t\t$o_cnt++;\n>> +\t\t\t$n_cnt++;\n>> +\t\t}\n>> +\t}\n>> +\tmy ($o_ofs, undef, $n_ofs, undef) =\n>> +\t\t\t\t\tparse_hunk_header($newtext->[0]);\n>> +\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n>> +\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n>> +\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n>> +\t# Return the change in the number of lines inserted by this hunk\n>> +\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n>> +}\n> \n> OK.\n> \n>> @@ -1114,25 +1144,32 @@ sub prompt_yesno {\n>> }\n>> \n>> sub edit_hunk_loop {\n>> -\tmy ($head, $hunk, $ix) = @_;\n>> -\tmy $text = $hunk->[$ix]->{TEXT};\n>> +\tmy ($head, $hunks, $ix) = @_;\n>> +\tmy $hunk = $hunks->[$ix];\n>> +\tmy $text = $hunk->{TEXT};\n>> ...\n>> +\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n>> +\t\t# If this hunk has already been edited then add the\n>> +\t\t# offset delta of the previous edit to get the real\n>> +\t\t# delta from the original unedited hunk.\n>> +\t\t$hunk->{OFS_DELTA} and\n>> +\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n> \n> Ahh, good point.\n> \n\n"},{"id":"340795","messageId":"97910405-bdb5-d85b-1de1-d3548d0e4e18@talktalk.net","threadId":"47825","inReplyTo":"xmqqo9k7fsht.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-02T10:49:03Z","receivedAt":"2018-03-02T10:49:17Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 01/03/18 20:30, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> Now that add -p counts patches properly it should be possible to turn\n>> off the '--recount' option when invoking 'git apply'\n> \n> Sounds good ;-)\n> \nLets hope it works! Thanks for your comments, I'll send a re-roll of\nthis series at the beginning of next week\n\nBest Wishes\n\nPhillip\n"},{"id":"340796","messageId":"b094e133-75bb-04ee-64b1-af7019a436f0@talktalk.net","threadId":"47825","inReplyTo":"xmqqsh9jfsjd.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 8/9] add -p: fix counting when splitting and coalescing","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-02T10:48:45Z","receivedAt":"2018-03-02T10:49:50Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 01/03/18 20:29, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> @@ -887,8 +892,8 @@ sub merge_hunk {\n>> \t$o_cnt = $n_cnt = 0;\n>> \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n>> \t\tmy $line = $prev->{TEXT}[$i];\n>> -\t\tif ($line =~ /^\\+/) {\n>> -\t\t\t$n_cnt++;\n>> +\t\tif ($line =~ /^[+\\\\]/) {\n>> +\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n>> \t\t\tpush @line, $line;\n>> \t\t\tnext;\n>> \t\t}\n> \n> Hmmmm, the logic may be correct, but this looks like a result of\n> attempting to minimize the number of changed lines and ending up\n> with a less-than-readble code. \"If the line begins with a plus or\n> backslash, do these things, the first of which is done only when\n> the line begins with a plus.\" The same comment for the other hunk\n> that counts the $this side.\n> \nRight, I'll re-roll with a separate clause for the \"\\ No new line ..\" lines.\n"},{"id":"340810","messageId":"20180302155528.12106-1-szeder.dev@gmail.com","threadId":"47825","inReplyTo":"20180301105103.24268-5-phillip.wood@talktalk.net","subject":"Re: [PATCH v4 4/9] t3701: don't hard code sha1 hash values","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-02T15:55:28Z","receivedAt":"2018-03-02T15:56:55Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\n> Use a filter when comparing diffs to fix the value of non-zero hashes\n> in diff index lines so we're not hard coding sha1 hash values in the\n> expected output. This makes it easier to change the expected output if\n> a test is edited as we don't need to worry about the exact hash value\n> and means the tests will work when the hash algorithm is transitioned\n> away from sha1.\n\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 836ce346ed..f95714230b 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -10,6 +10,19 @@ then\n> \ttest_done\n> fi\n> \n> +diff_cmp () {\n> +\tfor x\n> +\tdo\n> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n> +\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n> +\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n> +\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n> +\t\t \"$x\" >\"$x.filtered\"\n> +\tdone\n> +\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n> +}\n\nt3701 is not the only test script comparing diffs, many other\ntests do so:\n\n $ git grep 'diff --git' t/ |wc -l\n 835\n\nIt's totally inaccurate, but a good ballpark figure.\n\nI think this function should be a widely available test helper\nfunction in 'test-lib-functions.sh', perhaps renamed to\n'test_cmp_diff', so those other tests could use it as well.\n\n"},{"id":"340813","messageId":"xmqqa7vqe9xn.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180302155528.12106-1-szeder.dev@gmail.com","subject":"Re: [PATCH v4 4/9] t3701: don't hard code sha1 hash values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-02T16:09:24Z","receivedAt":"2018-03-02T16:09:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> +diff_cmp () {\n>> +\tfor x\n>> +\tdo\n>> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n>> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n>> +\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n>> +\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n>> +\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n>> +\t\t \"$x\" >\"$x.filtered\"\n>> +\tdone\n>> +\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n>> +}\n>\n> t3701 is not the only test script comparing diffs, many other\n> tests do so:\n>\n> $ git grep 'diff --git' t/ |wc -l\n> 835\n>\n> It's totally inaccurate, but a good ballpark figure.\n>\n> I think this function should be a widely available test helper\n> function in 'test-lib-functions.sh', perhaps renamed to\n> 'test_cmp_diff', so those other tests could use it as well.\n\nI am on the fence. \n\nThe above does not redact enough (e.g. rename/copy score should be\nredacted while comparing) to serve as a generic filter.\n\nWe could certainly extend it when we make code in other tests use\nthe helper, but then we can do the moving to test-lib-functions when\nthat happens, too.\n\nThose who will be writing new tests that need to compare two diff\noutputs are either (1) people who are careful and try to find\nexisting tests that do the same, to locate the helper we already\nhave to be reused in theirs, or (2) people who don't and roll their\nown. The latter camp cannot be helped either way, but the former\nwill run \"git grep 'diff --git' t/\" and find the helper whether it\nis in this script or in test-lib-functions, I would think.\n\nSo, I certainly do not mind a reroll to move it to a more generic\nplace, but I do not think I would terribly mind if we leave it in\nits current place, later to be moved by the first new caller that\nwants to use it from outside this script.\n"},{"id":"341004","messageId":"20180305105630.14407-2-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 1/9] add -i: add function to format hunk header","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:22Z","receivedAt":"2018-03-05T10:56:57Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nThis code is duplicated in a couple of places so make it into a\nfunction as we're going to add some more callers shortly.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a7542..8ababa6453 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -751,6 +751,15 @@ sub parse_hunk_header {\n \treturn ($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n }\n \n+sub format_hunk_header {\n+\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;\n+\treturn (\"@@ -$o_ofs\" .\n+\t\t(($o_cnt != 1) ? \",$o_cnt\" : '') .\n+\t\t\" +$n_ofs\" .\n+\t\t(($n_cnt != 1) ? \",$n_cnt\" : '') .\n+\t\t\" @@\\n\");\n+}\n+\n sub split_hunk {\n \tmy ($text, $display) = @_;\n \tmy @split = ();\n@@ -838,11 +847,7 @@ sub split_hunk {\n \t\tmy $o_cnt = $hunk->{OCNT};\n \t\tmy $n_cnt = $hunk->{NCNT};\n \n-\t\tmy $head = (\"@@ -$o_ofs\" .\n-\t\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t\t \" +$n_ofs\" .\n-\t\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t\t \" @@\\n\");\n+\t\tmy $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n \t\tmy $display_head = $head;\n \t\tunshift @{$hunk->{TEXT}}, $head;\n \t\tif ($diff_use_color) {\n@@ -912,11 +917,7 @@ sub merge_hunk {\n \t\t}\n \t\tpush @line, $line;\n \t}\n-\tmy $head = (\"@@ -$o0_ofs\" .\n-\t\t (($o_cnt != 1) ? \",$o_cnt\" : '') .\n-\t\t \" +$n0_ofs\" .\n-\t\t (($n_cnt != 1) ? \",$n_cnt\" : '') .\n-\t\t \" @@\\n\");\n+\tmy $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);\n \t@{$prev->{TEXT}} = ($head, @line);\n }\n \n-- \n2.16.2\n\n"},{"id":"341005","messageId":"20180305105630.14407-3-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 2/9] t3701: indent here documents","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:23Z","receivedAt":"2018-03-05T10:57:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIndent here documents in line with the current style for tests.\nWhile at it, quote the end marker of here-docs that do not use\nvariable interpolation.\n\nSigned-off-by: Phillip Wood \nSigned-off-by: Junio C Hamano \n---\n\nNotes:\n changes since v3:\n - updated to match what was in pu\n\n t/t3701-add-interactive.sh | 174 ++++++++++++++++++++++-----------------------\n 1 file changed, 87 insertions(+), 87 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 058698df6a..3130dafcf0 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tnew file mode 100644\n+\tindex 0000000..d95f3ad\n+\t--- /dev/null\n+\t+++ b/file\n+\t@@ -0,0 +1 @@\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (initial)' '\n@@ -59,14 +59,14 @@ test_expect_success 'status works (commit)' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1 +1,2 @@\n+\t baseline\n+\t+content\n+\tEOF\n '\n \n test_expect_success 'diff works (commit)' '\n@@ -83,8 +83,8 @@ test_expect_success 'revert works (commit)' '\n \n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n@@ -100,21 +100,21 @@ test_expect_success 'dummy edit works' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\t@@ -1,1 +1,4 @@\n+\t this\n+\t+patch\n+\t-does not\n+\t apply\n+\tEOF\n '\n \n test_expect_success 'setup fake editor' '\n \techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<\\EOF &&\n-mv -f \"$1\" oldpatch &&\n-mv -f patch \"$1\"\n-EOF\n+\tcat >>fake_editor.sh <<-\\EOF &&\n+\tmv -f \"$1\" oldpatch &&\n+\tmv -f patch \"$1\"\n+\tEOF\n \tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n@@ -126,10 +126,10 @@ test_expect_success 'bad edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\tthis patch\n+\tis garbage\n+\tEOF\n '\n \n test_expect_success 'garbage edit rejected' '\n@@ -139,28 +139,28 @@ test_expect_success 'garbage edit rejected' '\n '\n \n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\t@@ -1,0 +1,0 @@\n+\t baseline\n+\t+content\n+\t+newcontent\n+\t+lines\n+\tEOF\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/file b/file\n+\tindex b5dd6c9..f910ae9 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,4 +1,4 @@\n+\t baseline\n+\t content\n+\t-newcontent\n+\t+more\n+\t lines\n+\tEOF\n '\n \n test_expect_success 'real edit works' '\n@@ -222,31 +222,31 @@ test_expect_success 'setup again' '\n \n # Write the patch file with a new line at the top and bottom\n test_expect_success 'setup patch' '\n-cat >patch <patch <<-\\EOF\n+\tindex 180b47c..b6f2c08 100644\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Expected output, similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/file b/file\n+\tindex b6f2c08..61b9053 100755\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,4 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t+lastline\n+\tEOF\n '\n \n # Test splitting the first patch, then adding both\n@@ -259,15 +259,15 @@ test_expect_success 'add first line works' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/non-empty b/non-empty\n+\tdeleted file mode 100644\n+\tindex d95f3ad..0000000\n+\t--- a/non-empty\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-content\n+\tEOF\n '\n \n test_expect_success 'deleting a non-empty file' '\n@@ -282,11 +282,11 @@ test_expect_success 'deleting a non-empty file' '\n '\n \n test_expect_success 'setup expected' '\n-cat >expected <expected <<-\\EOF\n+\tdiff --git a/empty b/empty\n+\tdeleted file mode 100644\n+\tindex e69de29..0000000\n+\tEOF\n '\n \n test_expect_success 'deleting an empty file' '\n-- \n2.16.2\n\n"},{"id":"341006","messageId":"20180305105630.14407-1-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180213104408.9887-1-phillip.wood@talktalk.net","subject":"[PATCH v5 0/9] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:21Z","receivedAt":"2018-03-05T10:57:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI've updated these to clean up the perl style in response to Junio's\ncomments on v4.\n\nCover letter to v1:\n\nWhile working on a patch series to stage selected lines from a hunk\nwithout having to edit it I got worried that subsequent patches would\nbe applied in the wrong place which lead to this series to correct the\noffsets of hunks following those that are skipped or edited.\n\nInterdiff to v4:\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex a64c0db57d..f83e7450ad 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -892,8 +892,11 @@ sub merge_hunk {\n \t$o_cnt = $n_cnt = 0;\n \tfor ($i = 1; $i < @{$prev->{TEXT}}; $i++) {\n \t\tmy $line = $prev->{TEXT}[$i];\n-\t\tif ($line =~ /^[+\\\\]/) {\n-\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n+\t\tif ($line =~ /^\\+/) {\n+\t\t\t$n_cnt++;\n+\t\t\tpush @line, $line;\n+\t\t\tnext;\n+\t\t} elsif ($line =~ /^\\\\/) {\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\n@@ -910,8 +913,11 @@ sub merge_hunk {\n \n \tfor ($i = 1; $i < @{$this->{TEXT}}; $i++) {\n \t\tmy $line = $this->{TEXT}[$i];\n-\t\tif ($line =~ /^[+\\\\]/) {\n-\t\t\t$n_cnt++ if ($line =~ /^\\+/);\n+\t\tif ($line =~ /^\\+/) {\n+\t\t\t$n_cnt++;\n+\t\t\tpush @line, $line;\n+\t\t\tnext;\n+\t\t} elsif ($line =~ /^\\\\/) {\n \t\t\tpush @line, $line;\n \t\t\tnext;\n \t\t}\n@@ -945,7 +951,9 @@ sub coalesce_overlapping_hunks {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n \t\t\t# If this hunk has been edited then subtract\n \t\t\t# the delta that is due to the edit.\n-\t\t\t$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};\n+\t\t\tif ($_->{OFS_DELTA}) {\n+\t\t\t\t$ofs_delta -= $_->{OFS_DELTA};\n+\t\t\t}\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n@@ -955,7 +963,9 @@ sub coalesce_overlapping_hunks {\n \t\t}\n \t\t# If this hunk was edited then adjust the offset delta\n \t\t# to reflect the edit.\n-\t\t$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};\n+\t\tif ($_->{OFS_DELTA}) {\n+\t\t\t$ofs_delta += $_->{OFS_DELTA};\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n\n\nPhillip Wood (9):\n add -i: add function to format hunk header\n t3701: indent here documents\n t3701: use test_write_lines and write_script\n t3701: don't hard code sha1 hash values\n t3701: add failing test for pathological context lines\n add -p: adjust offsets of subsequent hunks when one is skipped\n add -p: calculate offset delta for edited patches\n add -p: fix counting when splitting and coalescing\n add -p: don't rely on apply's '--recount' option\n\n git-add--interactive.perl | 108 +++++++++++++----\n t/t3701-add-interactive.sh | 291 +++++++++++++++++++++++++--------------------\n 2 files changed, 249 insertions(+), 150 deletions(-)\n\n-- \n2.16.2\n\n"},{"id":"341007","messageId":"20180305105630.14407-4-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 3/9] t3701: use test_write_lines and write_script","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:24Z","receivedAt":"2018-03-05T10:57:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSimplify things slightly by using the above helpers.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2\n - fixed use of test_set_editor to match what was in pu\n\n t/t3701-add-interactive.sh | 33 +++++----------------------------\n 1 file changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 3130dafcf0..836ce346ed 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '\n \tEOF\n '\n \n-test_expect_success 'setup fake editor' '\n-\t>fake_editor.sh &&\n-\tchmod a+x fake_editor.sh &&\n-\ttest_set_editor \"$(pwd)/fake_editor.sh\"\n-'\n-\n test_expect_success 'dummy edit works' '\n+\ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n \ttest_cmp expected diff\n@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '\n '\n \n test_expect_success 'setup fake editor' '\n-\techo \"#!$SHELL_PATH\" >fake_editor.sh &&\n-\tcat >>fake_editor.sh <<-\\EOF &&\n+\twrite_script \"fake_editor.sh\" <<-\\EOF &&\n \tmv -f \"$1\" oldpatch &&\n \tmv -f patch \"$1\"\n \tEOF\n-\tchmod a+x fake_editor.sh &&\n \ttest_set_editor \"$(pwd)/fake_editor.sh\"\n '\n \n@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '\n \n test_expect_success 'split hunk setup' '\n \tgit reset --hard &&\n-\tfor i in 10 20 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test &&\n+\ttest_write_lines 10 20 30 40 50 60 >test &&\n \tgit add test &&\n \ttest_tick &&\n \tgit commit -m test &&\n \n-\tfor i in 10 15 20 21 22 23 24 30 40 50 60\n-\tdo\n-\t\techo $i\n-\tdone >test\n+\ttest_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test\n '\n \n test_expect_success 'split hunk \"add -p (edit)\"' '\n@@ -334,17 +321,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n '\n \n test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n-\tcat >test <<-\\EOF &&\n-\t5\n-\t10\n-\t20\n-\t21\n-\t30\n-\t31\n-\t40\n-\t50\n-\t60\n-\tEOF\n+\ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n \t# q n q q is there to make sure we exit at the end.\n-- \n2.16.2\n\n"},{"id":"341008","messageId":"20180305105630.14407-9-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 8/9] add -p: fix counting when splitting and coalescing","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:29Z","receivedAt":"2018-03-05T10:57:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a file has no trailing new line at the end diff records this by\nappending \"\\ No newline at end of file\" below the last line of the\nfile. This line should not be counted in the hunk header. Fix the\nsplitting and coalescing code to count files without a trailing new line\nproperly and change one of the tests to test splitting without a\ntrailing new line.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v4:\n - use a separate if clause to handle with \"\\ No newline at end of\n file\" lines rather than crowbarring them into the \"+\" line\n handling.\n\n git-add--interactive.perl | 11 +++++++++++\n t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++--------\n 2 files changed, 34 insertions(+), 8 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex f80372d3ad..d1b550d4d8 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -793,6 +793,11 @@ sub split_hunk {\n \t\twhile (++$i < @$text) {\n \t\t\tmy $line = $text->[$i];\n \t\t\tmy $display = $display->[$i];\n+\t\t\tif ($line =~ /^\\\\/) {\n+\t\t\t\tpush @{$this->{TEXT}}, $line;\n+\t\t\t\tpush @{$this->{DISPLAY}}, $display;\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\tif ($line =~ /^ /) {\n \t\t\t\tif ($this->{ADDDEL} &&\n \t\t\t\t !defined $next_hunk_start) {\n@@ -891,6 +896,9 @@ sub merge_hunk {\n \t\t\t$n_cnt++;\n \t\t\tpush @line, $line;\n \t\t\tnext;\n+\t\t} elsif ($line =~ /^\\\\/) {\n+\t\t\tpush @line, $line;\n+\t\t\tnext;\n \t\t}\n \n \t\tlast if ($o1_ofs <= $ofs);\n@@ -909,6 +917,9 @@ sub merge_hunk {\n \t\t\t$n_cnt++;\n \t\t\tpush @line, $line;\n \t\t\tnext;\n+\t\t} elsif ($line =~ /^\\\\/) {\n+\t\t\tpush @line, $line;\n+\t\t\tnext;\n \t\t}\n \t\t$ofs++;\n \t\t$o_cnt++;\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 4cc9517eda..a9a9478a29 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -237,31 +237,46 @@ test_expect_success 'setup patch' '\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n-# Expected output, similar to the patch but w/ diff at the top\n+# Expected output, diff is similar to the patch but w/ diff at the top\n test_expect_success 'setup expected' '\n-\tcat >expected <<-\\EOF\n-\tdiff --git a/file b/file\n-\tindex b6f2c08..61b9053 100755\n+\techo diff --git a/file b/file >expected &&\n+\tcat patch |sed \"/^index/s/ 100644/ 100755/\" >>expected &&\n+\tcat >expected-output <<-\\EOF\n \t--- a/file\n \t+++ b/file\n \t@@ -1,2 +1,4 @@\n \t+firstline\n \t baseline\n \t content\n \t+lastline\n+\t\\ No newline at end of file\n+\t@@ -1,2 +1,3 @@\n+\t+firstline\n+\t baseline\n+\t content\n+\t@@ -1,2 +2,3 @@\n+\t baseline\n+\t content\n+\t+lastline\n+\t\\ No newline at end of file\n \tEOF\n '\n \n # Test splitting the first patch, then adding both\n-test_expect_success 'add first line works' '\n+test_expect_success C_LOCALE_OUTPUT 'add first line works' '\n \tgit commit -am \"clear local changes\" &&\n \tgit apply patch &&\n-\t(echo s; echo y; echo y) | git add -p file &&\n-\tgit diff --cached > diff &&\n-\tdiff_cmp expected diff\n+\tprintf \"%s\\n\" s y y | git add -p file 2>error |\n+\t\tsed -n -e \"s/^Stage this hunk[^@]*\\(@@ .*\\)/\\1/\" \\\n+\t\t -e \"/^[-+@ \\\\\\\\]\"/p >output &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached >diff &&\n+\tdiff_cmp expected diff &&\n+\ttest_cmp expected-output output\n '\n \n test_expect_success 'setup expected' '\n-- \n2.16.2\n\n"},{"id":"341009","messageId":"20180305105630.14407-7-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 6/9] add -p: adjust offsets of subsequent hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:27Z","receivedAt":"2018-03-05T10:57:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince commit 8cbd431082 (\"git-add--interactive: replace hunk\nrecounting with apply --recount\", 2008-7-2) if a hunk is skipped then\nwe rely on the context lines to apply subsequent hunks in the right\nplace. While this works most of the time it is possible for hunks to\nend up being applied in the wrong place. To fix this adjust the offset\nof subsequent hunks to correct for any change in the number of\ninsertions or deletions due to the skipped hunk. The change in offset\ndue to edited hunks that have the number of insertions or deletions\nchanged is ignored here, it will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 +++++++++++++--\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8ababa6453..7a0a5896bb 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {\n \tmy @out = ();\n \n \tmy ($last_o_ctx, $last_was_dirty);\n+\tmy $ofs_delta = 0;\n \n-\tfor (grep { $_->{USE} } @in) {\n+\tfor (@in) {\n \t\tif ($_->{TYPE} ne 'hunk') {\n \t\t\tpush @out, $_;\n \t\t\tnext;\n \t\t}\n \t\tmy $text = $_->{TEXT};\n-\t\tmy ($o_ofs) = parse_hunk_header($text->[0]);\n+\t\tmy ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =\n+\t\t\t\t\t\tparse_hunk_header($text->[0]);\n+\t\tunless ($_->{USE}) {\n+\t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($ofs_delta) {\n+\t\t\t$n_ofs += $ofs_delta;\n+\t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n+\t\t\t\t\t\t\t $n_ofs, $n_cnt);\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 094eeca405..e3150a4e07 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -544,7 +544,7 @@ test_expect_success 'set up pathological context' '\n \ttest_write_lines +b \" a\" >patch\n '\n \n-test_expect_failure 'add -p works with pathological context lines' '\n+test_expect_success 'add -p works with pathological context lines' '\n \tgit reset &&\n \tprintf \"%s\\n\" n y |\n \tgit add -p &&\n-- \n2.16.2\n\n"},{"id":"341010","messageId":"20180305105630.14407-6-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 5/9] t3701: add failing test for pathological context lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:26Z","receivedAt":"2018-03-05T10:57:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a hunk is skipped by add -i the offsets of subsequent hunks are\nnot adjusted to account for any missing insertions due to the skipped\nhunk. Most of the time this does not matter as apply uses the context\nlines to apply the subsequent hunks in the correct place, however in\npathological cases the context lines will match at the now incorrect\noffset and the hunk will be applied in the wrong place. The offsets of\nhunks following an edited hunk that has had the number of insertions\nor deletions changed also need to be updated in the same way. Add\nfailing tests to demonstrate this.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - removed test_set_editor as it is already set\n \n changes since v1:\n - changed edit test to use the existing fake editor and to strip\n the hunk header and some context lines as well as deleting an\n insertion\n - style fixes\n\n t/t3701-add-interactive.sh | 30 ++++++++++++++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex f95714230b..094eeca405 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -531,4 +531,34 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '\n \t! grep dirty-otherwise output\n '\n \n+test_expect_success 'set up pathological context' '\n+\tgit reset --hard &&\n+\ttest_write_lines a a a a a a a a a a a >a &&\n+\tgit add a &&\n+\tgit commit -m a &&\n+\ttest_write_lines c b a a a a a a a b a a a a >a &&\n+\ttest_write_lines a a a a a a a b a a a a >expected-1 &&\n+\ttest_write_lines b a a a a a a a b a a a a >expected-2 &&\n+\t# check editing can cope with missing header and deleted context lines\n+\t# as well as changes to other lines\n+\ttest_write_lines +b \" a\" >patch\n+'\n+\n+test_expect_failure 'add -p works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" n y |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-1 actual\n+'\n+\n+test_expect_failure 'add -p patch editing works with pathological context lines' '\n+\tgit reset &&\n+\t# n q q below is in case edit fails\n+\tprintf \"%s\\n\" e y n q q |\n+\tgit add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"341011","messageId":"20180305105630.14407-5-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:25Z","receivedAt":"2018-03-05T10:57:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nUse a filter when comparing diffs to fix the value of non-zero hashes\nin diff index lines so we're not hard coding sha1 hash values in the\nexpected output. This makes it easier to change the expected output if\na test is edited as we don't need to worry about the exact hash value\nand means the tests will work when the hash algorithm is transitioned\naway from sha1.\n\nThanks-to: Junio C Hamano \nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v3:\n - fix zero hash values to seven digits\n changes since v2:\n - fix hash values in index lines rather than removing the line\n - reworded commit message\n\n t/t3701-add-interactive.sh | 33 +++++++++++++++++++++++----------\n 1 file changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 836ce346ed..f95714230b 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -10,6 +10,19 @@ then\n \ttest_done\n fi\n \n+diff_cmp () {\n+\tfor x\n+\tdo\n+\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n+\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n+\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n+\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n+\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n+\t\t \"$x\" >\"$x.filtered\"\n+\tdone\n+\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n+}\n+\n test_expect_success 'setup (initial)' '\n \techo content >file &&\n \tgit add file &&\n@@ -35,7 +48,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (initial)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/new file/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (initial)' '\n \tgit add file &&\n@@ -72,7 +85,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'diff works (commit)' '\n \t(echo d; echo 1) | git add -i >output &&\n \tsed -ne \"/^index/,/content/p\" diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n test_expect_success 'revert works (commit)' '\n \tgit add file &&\n@@ -91,7 +104,7 @@ test_expect_success 'dummy edit works' '\n \ttest_set_editor : &&\n \t(echo e; echo a) | git add -p &&\n \tgit diff > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup patch' '\n@@ -159,7 +172,7 @@ test_expect_success 'setup expected' '\n test_expect_success 'real edit works' '\n \t(echo e; echo n; echo d) | git add -p &&\n \tgit diff >output &&\n-\ttest_cmp expected output\n+\tdiff_cmp expected output\n '\n \n test_expect_success 'skip files similarly as commit -a' '\n@@ -171,7 +184,7 @@ test_expect_success 'skip files similarly as commit -a' '\n \tgit reset &&\n \tgit commit -am commit &&\n \tgit diff >expected &&\n-\ttest_cmp expected output &&\n+\tdiff_cmp expected output &&\n \tgit reset --hard HEAD^\n '\n rm -f .gitignore\n@@ -248,7 +261,7 @@ test_expect_success 'add first line works' '\n \tgit apply patch &&\n \t(echo s; echo y; echo y) | git add -p file &&\n \tgit diff --cached > diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -271,7 +284,7 @@ test_expect_success 'deleting a non-empty file' '\n \trm non-empty &&\n \techo y | git add -p non-empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'setup expected' '\n@@ -290,7 +303,7 @@ test_expect_success 'deleting an empty file' '\n \trm empty &&\n \techo y | git add -p empty &&\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success 'split hunk setup' '\n@@ -355,7 +368,7 @@ test_expect_success 'patch mode ignores unmerged entries' '\n \t+changed\n \tEOF\n \tgit diff --cached >diff &&\n-\ttest_cmp expected diff\n+\tdiff_cmp expected diff\n '\n \n test_expect_success TTY 'diffs can be colorized' '\n@@ -384,7 +397,7 @@ test_expect_success 'patch-mode via -i prompts for files' '\n \n \techo test >expect &&\n \tgit diff --cached --name-only >actual &&\n-\ttest_cmp expect actual\n+\tdiff_cmp expect actual\n '\n \n test_expect_success 'add -p handles globs' '\n-- \n2.16.2\n\n"},{"id":"341012","messageId":"20180305105630.14407-8-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 7/9] add -p: calculate offset delta for edited patches","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:28Z","receivedAt":"2018-03-05T10:57:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nRecount the number of preimage and postimage lines in a hunk after it\nhas been edited so any change in the number of insertions or deletions\ncan be used to adjust the offsets of subsequent hunks. If an edited\nhunk is subsequently split then the offset correction will be lost. It\nwould be possible to fix this if it is a problem, however the code\nhere is still an improvement on the status quo for the common case\nwhere an edited hunk is applied without being split.\n\nThis is also a necessary step to removing '--recount' and\n'--allow-overlap' from the invocation of 'git apply'. Before\n'--recount' can be removed the splitting and coalescing counting needs\nto be fixed to handle a missing newline at the end of a file. In order\nto remove '--allow-overlap' there needs to be i) some way of verifying\nthe offset data in the edited hunk (probably by correlating the\npreimage (or postimage if the patch is going to be applied in reverse)\nlines of the edited and unedited versions to see if they are offset or\nif any leading/trailing context lines have been removed) and ii) a way of\ndealing with edited hunks that change context lines that are shared\nwith neighbouring hunks.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v4:\n - replaced instances of\n <> and <>;\n with\n if (<>) {\n \t<>;\n }\n \n changes since v1\n - edited hunks are now recounted before seeing if they apply in\n preparation for removing '--recount' when invoking 'git apply'\n - added sentence to commit message about the offset data being lost\n if an edited hunk is split\n\n git-add--interactive.perl | 59 +++++++++++++++++++++++++++++++++++++++-------\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 51 insertions(+), 10 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7a0a5896bb..f80372d3ad 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -938,13 +938,23 @@ sub coalesce_overlapping_hunks {\n \t\t\t\t\t\tparse_hunk_header($text->[0]);\n \t\tunless ($_->{USE}) {\n \t\t\t$ofs_delta += $o_cnt - $n_cnt;\n+\t\t\t# If this hunk has been edited then subtract\n+\t\t\t# the delta that is due to the edit.\n+\t\t\tif ($_->{OFS_DELTA}) {\n+\t\t\t\t$ofs_delta -= $_->{OFS_DELTA};\n+\t\t\t}\n \t\t\tnext;\n \t\t}\n \t\tif ($ofs_delta) {\n \t\t\t$n_ofs += $ofs_delta;\n \t\t\t$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,\n \t\t\t\t\t\t\t $n_ofs, $n_cnt);\n \t\t}\n+\t\t# If this hunk was edited then adjust the offset delta\n+\t\t# to reflect the edit.\n+\t\tif ($_->{OFS_DELTA}) {\n+\t\t\t$ofs_delta += $_->{OFS_DELTA};\n+\t\t}\n \t\tif (defined $last_o_ctx &&\n \t\t $o_ofs <= $last_o_ctx &&\n \t\t !$_->{DIRTY} &&\n@@ -1016,6 +1026,30 @@ marked for discarding.\"),\n marked for applying.\"),\n );\n \n+sub recount_edited_hunk {\n+\tlocal $_;\n+\tmy ($oldtext, $newtext) = @_;\n+\tmy ($o_cnt, $n_cnt) = (0, 0);\n+\tfor (@{$newtext}[1..$#{$newtext}]) {\n+\t\tmy $mode = substr($_, 0, 1);\n+\t\tif ($mode eq '-') {\n+\t\t\t$o_cnt++;\n+\t\t} elsif ($mode eq '+') {\n+\t\t\t$n_cnt++;\n+\t\t} elsif ($mode eq ' ') {\n+\t\t\t$o_cnt++;\n+\t\t\t$n_cnt++;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, undef, $n_ofs, undef) =\n+\t\t\t\t\tparse_hunk_header($newtext->[0]);\n+\t$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy (undef, $orig_o_cnt, undef, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($oldtext->[0]);\n+\t# Return the change in the number of lines inserted by this hunk\n+\treturn $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;\n+}\n+\n sub edit_hunk_manually {\n \tmy ($oldtext) = @_;\n \n@@ -1114,25 +1148,32 @@ sub prompt_yesno {\n }\n \n sub edit_hunk_loop {\n-\tmy ($head, $hunk, $ix) = @_;\n-\tmy $text = $hunk->[$ix]->{TEXT};\n+\tmy ($head, $hunks, $ix) = @_;\n+\tmy $hunk = $hunks->[$ix];\n+\tmy $text = $hunk->{TEXT};\n \n \twhile (1) {\n-\t\t$text = edit_hunk_manually($text);\n-\t\tif (!defined $text) {\n+\t\tmy $newtext = edit_hunk_manually($text);\n+\t\tif (!defined $newtext) {\n \t\t\treturn undef;\n \t\t}\n \t\tmy $newhunk = {\n-\t\t\tTEXT => $text,\n-\t\t\tTYPE => $hunk->[$ix]->{TYPE},\n+\t\t\tTEXT => $newtext,\n+\t\t\tTYPE => $hunk->{TYPE},\n \t\t\tUSE => 1,\n \t\t\tDIRTY => 1,\n \t\t};\n+\t\t$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);\n+\t\t# If this hunk has already been edited then add the\n+\t\t# offset delta of the previous edit to get the real\n+\t\t# delta from the original unedited hunk.\n+\t\t$hunk->{OFS_DELTA} and\n+\t\t\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n \t\tif (diff_applies($head,\n-\t\t\t\t @{$hunk}[0..$ix-1],\n+\t\t\t\t @{$hunks}[0..$ix-1],\n \t\t\t\t $newhunk,\n-\t\t\t\t @{$hunk}[$ix+1..$#{$hunk}])) {\n-\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$text})];\n+\t\t\t\t @{$hunks}[$ix+1..$#{$hunks}])) {\n+\t\t\t$newhunk->{DISPLAY} = [color_diff(@{$newtext})];\n \t\t\treturn $newhunk;\n \t\t}\n \t\telse {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex e3150a4e07..4cc9517eda 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -552,7 +552,7 @@ test_expect_success 'add -p works with pathological context lines' '\n \ttest_cmp expected-1 actual\n '\n \n-test_expect_failure 'add -p patch editing works with pathological context lines' '\n+test_expect_success 'add -p patch editing works with pathological context lines' '\n \tgit reset &&\n \t# n q q below is in case edit fails\n \tprintf \"%s\\n\" e y n q q |\n-- \n2.16.2\n\n"},{"id":"341013","messageId":"20180305105630.14407-10-phillip.wood@talktalk.net","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"[PATCH v5 9/9] add -p: don't rely on apply's '--recount' option","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:56:30Z","receivedAt":"2018-03-05T10:57:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nNow that add -p counts patches properly it should be possible to turn\noff the '--recount' option when invoking 'git apply'\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n I can't think of a reason why this shouldn't be OK but I can't help\n feeling slightly nervous about it. I've made it a separate patch so it\n can be easily dropped or reverted if I've missed something.\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex d1b550d4d8..f83e7450ad 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -677,7 +677,7 @@ sub add_untracked_cmd {\n sub run_git_apply {\n \tmy $cmd = shift;\n \tmy $fh;\n-\topen $fh, '| git ' . $cmd . \" --recount --allow-overlap\";\n+\topen $fh, '| git ' . $cmd . \" --allow-overlap\";\n \tprint $fh @_;\n \treturn close $fh;\n }\n-- \n2.16.2\n\n"},{"id":"341014","messageId":"bb6be000-7a62-7ad5-0b59-90a75fd6dca9@talktalk.net","threadId":"47825","inReplyTo":"xmqqa7vqe9xn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 4/9] t3701: don't hard code sha1 hash values","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-05T10:59:15Z","receivedAt":"2018-03-05T10:59:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 02/03/18 16:09, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n>>> +diff_cmp () {\n>>> +\tfor x\n>>> +\tdo\n>>> +\t\tsed -e '/^index/s/[0-9a-f]*[1-9a-f][0-9a-f]*\\.\\./1234567../' \\\n>>> +\t\t -e '/^index/s/\\.\\.[0-9a-f]*[1-9a-f][0-9a-f]*/..9abcdef/' \\\n>>> +\t\t -e '/^index/s/ 00*\\.\\./ 0000000../' \\\n>>> +\t\t -e '/^index/s/\\.\\.00*$/..0000000/' \\\n>>> +\t\t -e '/^index/s/\\.\\.00* /..0000000 /' \\\n>>> +\t\t \"$x\" >\"$x.filtered\"\n>>> +\tdone\n>>> +\ttest_cmp \"$1.filtered\" \"$2.filtered\"\n>>> +}\n>>\n>> t3701 is not the only test script comparing diffs, many other\n>> tests do so:\n>>\n>> $ git grep 'diff --git' t/ |wc -l\n>> 835\n>>\n>> It's totally inaccurate, but a good ballpark figure.\n>>\n>> I think this function should be a widely available test helper\n>> function in 'test-lib-functions.sh', perhaps renamed to\n>> 'test_cmp_diff', so those other tests could use it as well.\n> \n> I am on the fence. \n> \n> The above does not redact enough (e.g. rename/copy score should be\n> redacted while comparing) to serve as a generic filter.\n> \n> We could certainly extend it when we make code in other tests use\n> the helper, but then we can do the moving to test-lib-functions when\n> that happens, too.\n> \n> Those who will be writing new tests that need to compare two diff\n> outputs are either (1) people who are careful and try to find\n> existing tests that do the same, to locate the helper we already\n> have to be reused in theirs, or (2) people who don't and roll their\n> own. The latter camp cannot be helped either way, but the former\n> will run \"git grep 'diff --git' t/\" and find the helper whether it\n> is in this script or in test-lib-functions, I would think.\n> \n> So, I certainly do not mind a reroll to move it to a more generic\n> place, but I do not think I would terribly mind if we leave it in\n> its current place, later to be moved by the first new caller that\n> wants to use it from outside this script.\n> \n\nI did wonder about putting this function in a library when I first wrote\nit but decided to wait and see if it is useful instead. As Junio points\nout it would need to be improved to act as a generic filter, so I'll\ntake the easy option and leave it where it is at the moment.\n\nBest Wishes\n\nPhillip\n"},{"id":"341016","messageId":"CAM0VKjkNqMrQKXh4KgtvhSzdENDoDOQwGqxLtsYhbm3SMnPLRw@mail.gmail.com","threadId":"47825","inReplyTo":"bb6be000-7a62-7ad5-0b59-90a75fd6dca9@talktalk.net","subject":"Re: [PATCH v4 4/9] t3701: don't hard code sha1 hash values","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-05T12:32:22Z","receivedAt":"2018-03-05T12:32:28Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Mar 5, 2018 at 11:59 AM, Phillip Wood wrote:\n> I did wonder about putting this function in a library when I first wrote\n> it but decided to wait and see if it is useful instead. As Junio points\n> out it would need to be improved to act as a generic filter, so I'll\n> take the easy option and leave it where it is at the moment.\n\nMakes sense. I just pointed it out, because I could have used it\nalready in its current form for a test I was working on last week.\n"},{"id":"341049","messageId":"xmqqbmg29x1n.fsf@gitster-ct.c.googlers.com","threadId":"47825","inReplyTo":"20180305105630.14407-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v5 0/9] Correct offsets of hunks when one is skipped","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-05T18:50:28Z","receivedAt":"2018-03-05T18:50:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> I've updated these to clean up the perl style in response to Junio's\n> comments on v4.\n\nI've considered the use of \"apply --recount\" in this script (eh,\nrather, the existence of that option in \"apply\" command itself ;-))\na bug that need to be eventually fixed for a long time, so the copy\nof earlier rounds I've been carrying in my tree were forked from\n'maint'.\n\nI'll queue this round on the same base commit as before to replace;\nI _think_ this is ready for 'next'.\n\nThanks for working on this.\n"},{"id":"341116","messageId":"4744ba34-1bfb-4162-dd80-bae92bdd40e1@talktalk.net","threadId":"47825","inReplyTo":"xmqqbmg29x1n.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 0/9] Correct offsets of hunks when one is skipped","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-06T10:25:41Z","receivedAt":"2018-03-06T10:25:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/03/18 18:50, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> I've updated these to clean up the perl style in response to Junio's\n>> comments on v4.\n> \n> I've considered the use of \"apply --recount\" in this script (eh,\n> rather, the existence of that option in \"apply\" command itself ;-))\n> a bug that need to be eventually fixed for a long time, so the copy\n> of earlier rounds I've been carrying in my tree were forked from\n> 'maint'.\n\nOh I've been basing this on master, I hope that wasn't a problem.\n\n> I'll queue this round on the same base commit as before to replace;\n> I _think_ this is ready for 'next'.\n\nYes I think so (I've not come up with any new ways to break it, lets see\nif someone else can)\n\n> Thanks for working on this.\n> \nThanks, I use add -p quite a lot so it's nice to be able to contribute\nsomething back.\n\nBest Wishes\n\nPhillip\n"}]}