git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH 0/4] Correct offsets of hunks when one is skipped
@ 2018-02-13 10:44 Phillip Wood
  2018-02-13 10:44 ` [PATCH 1/4] add -i: add function to format hunk header Phillip Wood
                   ` (5 more replies)
  0 siblings, 6 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-13 10:44 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

While working on a patch series to stage selected lines from a hunk
without having to edit it I got worried that subsequent patches would
be applied in the wrong place which lead to this series to correct the
offsets of hunks following those that are skipped or edited.

Phillip Wood (4):
  add -i: add function to format hunk header
  t3701: add failing test for pathological context lines
  add -p: Adjust offsets of subsequent hunks when one is skipped
  add -p: calculate offset delta for edited patches

 git-add--interactive.perl  | 93 +++++++++++++++++++++++++++++++++++-----------
 t/t3701-add-interactive.sh | 30 +++++++++++++++
 2 files changed, 102 insertions(+), 21 deletions(-)

-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/4] add -i: add function to format hunk header
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
@ 2018-02-13 10:44 ` Phillip Wood
  2018-02-13 10:44 ` [PATCH 2/4] t3701: add failing test for pathological context lines Phillip Wood
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-13 10:44 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This code is duplicated in a couple of places so make it into a
function as we're going to add some more callers shortly.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -751,6 +751,15 @@ sub parse_hunk_header {
 	return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 }
 
+sub format_hunk_header {
+	my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;
+	return ("@@ -$o_ofs" .
+		(($o_cnt != 1) ? ",$o_cnt" : '') .
+		" +$n_ofs" .
+		(($n_cnt != 1) ? ",$n_cnt" : '') .
+		" @@\n");
+}
+
 sub split_hunk {
 	my ($text, $display) = @_;
 	my @split = ();
@@ -838,11 +847,7 @@ sub split_hunk {
 		my $o_cnt = $hunk->{OCNT};
 		my $n_cnt = $hunk->{NCNT};
 
-		my $head = ("@@ -$o_ofs" .
-			    (($o_cnt != 1) ? ",$o_cnt" : '') .
-			    " +$n_ofs" .
-			    (($n_cnt != 1) ? ",$n_cnt" : '') .
-			    " @@\n");
+		my $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 		my $display_head = $head;
 		unshift @{$hunk->{TEXT}}, $head;
 		if ($diff_use_color) {
@@ -912,11 +917,7 @@ sub merge_hunk {
 		}
 		push @line, $line;
 	}
-	my $head = ("@@ -$o0_ofs" .
-		    (($o_cnt != 1) ? ",$o_cnt" : '') .
-		    " +$n0_ofs" .
-		    (($n_cnt != 1) ? ",$n_cnt" : '') .
-		    " @@\n");
+	my $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);
 	@{$prev->{TEXT}} = ($head, @line);
 }
 
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 2/4] t3701: add failing test for pathological context lines
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
  2018-02-13 10:44 ` [PATCH 1/4] add -i: add function to format hunk header Phillip Wood
@ 2018-02-13 10:44 ` Phillip Wood
  2018-02-13 20:35   ` Junio C Hamano
  2018-02-13 10:44 ` [PATCH 3/4] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-13 10:44 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When a hunk is skipped by add -i the offsets of subsequent hunks are
not adjusted to account for any missing insertions due to the skipped
hunk. Most of the time this does not matter as apply uses the context
lines to apply the subsequent hunks in the correct place, however in
pathological cases the context lines will match at the now incorrect
offset and the hunk will be applied in the wrong place. The offsets of
hunks following an edited hunk that has had the number of insertions
or deletions changed also need to be updated in the same way. Add
failing tests to demonstrate this.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3701-add-interactive.sh | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 058698df6a4a9811b9db84fb5900472c47c61798..6838698a1382b24724cfbd3be04a7054489b94af 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -541,4 +541,34 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '
 	! grep dirty-otherwise output
 '
 
+test_expect_success 'set up pathological context' '
+	git reset --hard &&
+	test_write_lines a a a a a a a a a a a >a &&
+	git add a &&
+	git commit -m a &&
+	test_write_lines c b a a a a a a a b a a a a >a &&
+	test_write_lines     a a a a a a a b a a a a >expected-1 &&
+	test_write_lines   b a a a a a a a b a a a a >expected-2 &&
+	write_script editor <<-\EOF
+	sed /^+c/d "$1" >"$1.tmp"
+	mv "$1.tmp" "$1"
+	EOF
+'
+
+test_expect_failure 'add -p works with pathological context lines' '
+	git reset &&
+	printf "%s\n" n y |
+	git add -p &&
+	git cat-file blob :a > actual &&
+	test_cmp expected-1 actual
+'
+
+test_expect_failure 'add -p patch editing works with pathological context lines' '
+	git reset &&
+	printf "%s\n" e y |
+	GIT_EDITOR=./editor git add -p &&
+	git cat-file blob :a > actual &&
+	test_cmp expected-2 actual
+'
+
 test_done
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/4] add -p: Adjust offsets of subsequent hunks when one is skipped
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
  2018-02-13 10:44 ` [PATCH 1/4] add -i: add function to format hunk header Phillip Wood
  2018-02-13 10:44 ` [PATCH 2/4] t3701: add failing test for pathological context lines Phillip Wood
@ 2018-02-13 10:44 ` Phillip Wood
  2018-02-13 10:44 ` [PATCH 4/4] add -p: calculate offset delta for edited patches Phillip Wood
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-13 10:44 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Since commit 8cbd431082 ("git-add--interactive: replace hunk
recounting with apply --recount", 2008-7-2) if a hunk is skipped then
we rely on the context lines to apply subsequent hunks in the right
place. While this works most of the time it is possible for hunks to
end up being applied in the wrong place. To fix this adjust the offset
of subsequent hunks to correct for any change in the number of
insertions or deletions due to the skipped hunk. The change in offset
due to edited hunks that have the number of insertions or deletions
changed is ignored here, it will be fixed in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl  | 15 +++++++++++++--
 t/t3701-add-interactive.sh |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8ababa6453ac4f57a925aacbb8b9bb1c973e4a54..7a0a5896bb8fa063ace29b85840849c867b874f5 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {
 	my @out = ();
 
 	my ($last_o_ctx, $last_was_dirty);
+	my $ofs_delta = 0;
 
-	for (grep { $_->{USE} } @in) {
+	for (@in) {
 		if ($_->{TYPE} ne 'hunk') {
 			push @out, $_;
 			next;
 		}
 		my $text = $_->{TEXT};
-		my ($o_ofs) = parse_hunk_header($text->[0]);
+		my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
+						parse_hunk_header($text->[0]);
+		unless ($_->{USE}) {
+			$ofs_delta += $o_cnt - $n_cnt;
+			next;
+		}
+		if ($ofs_delta) {
+			$n_ofs += $ofs_delta;
+			$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,
+							     $n_ofs, $n_cnt);
+		}
 		if (defined $last_o_ctx &&
 		    $o_ofs <= $last_o_ctx &&
 		    !$_->{DIRTY} &&
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6838698a1382b24724cfbd3be04a7054489b94af..5401d74d5804b5e43286d08a905fb96c68b02e42 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -555,7 +555,7 @@ test_expect_success 'set up pathological context' '
 	EOF
 '
 
-test_expect_failure 'add -p works with pathological context lines' '
+test_expect_success 'add -p works with pathological context lines' '
 	git reset &&
 	printf "%s\n" n y |
 	git add -p &&
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 4/4] add -p: calculate offset delta for edited patches
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
                   ` (2 preceding siblings ...)
  2018-02-13 10:44 ` [PATCH 3/4] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
@ 2018-02-13 10:44 ` Phillip Wood
  2018-02-13 23:56 ` [PATCH 0/4] Correct offsets of hunks when one is skipped brian m. carlson
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
  5 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-13 10:44 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Recount the number of preimage and postimage lines in a hunk after it
has been edited so any change in the number of insertions or deletions
can be used to adjust the offsets of subsequent hunks. If an edited
hunk is subsequently split then the offset correction will be lost. It
would be possible to fix this if it is a problem, however the code
here is still an improvement on the status quo for the common case
where an edited hunk is applied without being split.

This is also a necessary step to removing '--recount' and
'--allow-overlap' from the invocation of 'git apply'. Before
'--recount' can be removed the splitting and coalescing counting needs
to be fixed to handle a missing newline at the end of a file. In order
to remove '--allow-overlap' there needs to be i) some way of verifying
the offset data in the edited hunk (probably by correlating the
preimage (or postimage if the patch is going to be applied in reverse)
lines of the edited and unedited versions to see if they are offset or
if any leading/trailing context lines have been removed) and ii) a way of
dealing with edited hunks that change context lines that are shared
with neighbouring hunks.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl  | 57 ++++++++++++++++++++++++++++++++++++++--------
 t/t3701-add-interactive.sh |  2 +-
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 7a0a5896bb8fa063ace29b85840849c867b874f5..f139e545a4fb9cbaa757208a44cb2b5c3ad3678a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -938,6 +938,9 @@ sub coalesce_overlapping_hunks {
 						parse_hunk_header($text->[0]);
 		unless ($_->{USE}) {
 			$ofs_delta += $o_cnt - $n_cnt;
+			# If this hunk has been edited then subtract
+			# the delta that is due to the edit.
+			$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};
 			next;
 		}
 		if ($ofs_delta) {
@@ -945,6 +948,9 @@ sub coalesce_overlapping_hunks {
 			$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,
 							     $n_ofs, $n_cnt);
 		}
+		# If this hunk was edited then adjust the offset delta
+		# to reflect the edit.
+		$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};
 		if (defined $last_o_ctx &&
 		    $o_ofs <= $last_o_ctx &&
 		    !$_->{DIRTY} &&
@@ -1016,6 +1022,30 @@ marked for discarding."),
 marked for applying."),
 );
 
+sub recount_edited_hunk {
+	local $_;
+	my ($oldtext, $newtext) = @_;
+	my ($o_cnt, $n_cnt) = (0, 0);
+	for (@{$newtext}[1..$#{$newtext}]) {
+		my $mode = substr($_, 0, 1);
+		if ($mode eq '-') {
+			$o_cnt++;
+		} elsif ($mode eq '+') {
+			$n_cnt++;
+		} elsif ($mode eq ' ') {
+			$o_cnt++;
+			$n_cnt++;
+		}
+	}
+	my ($o_ofs, undef, $n_ofs, undef) =
+					parse_hunk_header($newtext->[0]);
+	$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
+	my (undef, $orig_o_cnt, undef, $orig_n_cnt) =
+					parse_hunk_header($oldtext->[0]);
+	# Return the change in the number of lines inserted by this hunk
+	return $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;
+}
+
 sub edit_hunk_manually {
 	my ($oldtext) = @_;
 
@@ -1114,25 +1144,34 @@ sub prompt_yesno {
 }
 
 sub edit_hunk_loop {
-	my ($head, $hunk, $ix) = @_;
-	my $text = $hunk->[$ix]->{TEXT};
+	my ($head, $hunks, $ix) = @_;
+	my $hunk = $hunks->[$ix];
+	my $text = $hunk->{TEXT};
 
 	while (1) {
-		$text = edit_hunk_manually($text);
-		if (!defined $text) {
+		my $newtext = edit_hunk_manually($text);
+		if (!defined $newtext) {
 			return undef;
 		}
 		my $newhunk = {
-			TEXT => $text,
-			TYPE => $hunk->[$ix]->{TYPE},
+			TEXT => $newtext,
+			TYPE => $hunk->{TYPE},
 			USE => 1,
 			DIRTY => 1,
 		};
 		if (diff_applies($head,
-				 @{$hunk}[0..$ix-1],
+				 @{$hunks}[0..$ix-1],
 				 $newhunk,
-				 @{$hunk}[$ix+1..$#{$hunk}])) {
-			$newhunk->{DISPLAY} = [color_diff(@{$text})];
+				 @{$hunks}[$ix+1..$#{$hunks}])) {
+			$newhunk->{OFS_DELTA} =
+					recount_edited_hunk($text, $newtext);
+			# If this hunk has already been edited then
+			# add the offset delta of the previous edit to
+			# get the real delta from the original
+			# unedited hunk.
+			$hunk->{OFS_DELTA} and
+				$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};
+			$newhunk->{DISPLAY} = [color_diff(@{$newtext})];
 			return $newhunk;
 		}
 		else {
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 5401d74d5804b5e43286d08a905fb96c68b02e42..5681bb2bb7e9a9c0702f8ebbcd97efbb5c5d1d58 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -563,7 +563,7 @@ test_expect_success 'add -p works with pathological context lines' '
 	test_cmp expected-1 actual
 '
 
-test_expect_failure 'add -p patch editing works with pathological context lines' '
+test_expect_success 'add -p patch editing works with pathological context lines' '
 	git reset &&
 	printf "%s\n" e y |
 	GIT_EDITOR=./editor git add -p &&
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 2/4] t3701: add failing test for pathological context lines
  2018-02-13 10:44 ` [PATCH 2/4] t3701: add failing test for pathological context lines Phillip Wood
@ 2018-02-13 20:35   ` Junio C Hamano
  0 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2018-02-13 20:35 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Phillip Wood

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> When a hunk is skipped by add -i the offsets of subsequent hunks are
> not adjusted to account for any missing insertions due to the skipped
> hunk. Most of the time this does not matter as apply uses the context
> lines to apply the subsequent hunks in the correct place, however in
> pathological cases the context lines will match at the now incorrect
> offset and the hunk will be applied in the wrong place. The offsets of

Good.  The --recount "feature" on the receiving end does not have
enough information to do a job as good as the code sitting at the
side of producing a patch to be applied, and this goes in the right
direction.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/4] Correct offsets of hunks when one is skipped
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
                   ` (3 preceding siblings ...)
  2018-02-13 10:44 ` [PATCH 4/4] add -p: calculate offset delta for edited patches Phillip Wood
@ 2018-02-13 23:56 ` brian m. carlson
  2018-02-19 13:01   ` Phillip Wood
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
  5 siblings, 1 reply; 29+ messages in thread
From: brian m. carlson @ 2018-02-13 23:56 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Tue, Feb 13, 2018 at 10:44:04AM +0000, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> While working on a patch series to stage selected lines from a hunk
> without having to edit it I got worried that subsequent patches would
> be applied in the wrong place which lead to this series to correct the
> offsets of hunks following those that are skipped or edited.
> 
> Phillip Wood (4):
>   add -i: add function to format hunk header
>   t3701: add failing test for pathological context lines
>   add -p: Adjust offsets of subsequent hunks when one is skipped
>   add -p: calculate offset delta for edited patches
> 
>  git-add--interactive.perl  | 93 +++++++++++++++++++++++++++++++++++-----------
>  t/t3701-add-interactive.sh | 30 +++++++++++++++
>  2 files changed, 102 insertions(+), 21 deletions(-)

This looks reasonably sane to me.  I really like that you managed to
produce failing tests for this situation.  I know pathological cases
like this have bit GCC in the past, so it's good that you fixed this.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 867 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 0/9] Correct offsets of hunks when one is skipped
  2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
                   ` (4 preceding siblings ...)
  2018-02-13 23:56 ` [PATCH 0/4] Correct offsets of hunks when one is skipped brian m. carlson
@ 2018-02-19 11:29 ` " Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 1/9] add -i: add function to format hunk header Phillip Wood
                     ` (8 more replies)
  5 siblings, 9 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Since v1 I've added some test cleanups for t3701, fixed the counting
when splitting and coalescing hunks containing "\ No newline at end of
file" lines and added a patch to remove '--recount' from the
invocation of 'git apply'. There are minor changes to patches 5
(previously patch 2) and patch 7 (previously patch 4) which I've
explained in the comments on those patches. Otherwise the original
patches are unchanged.

Cover letter to v1:

While working on a patch series to stage selected lines from a hunk
without having to edit it I got worried that subsequent patches would
be applied in the wrong place which lead to this series to correct the
offsets of hunks following those that are skipped or edited.


Phillip Wood (9):
  add -i: add function to format hunk header
  t3701: indent here documents
  t3701: use test_write_lines and write_script
  t3701: don't hard code sha1 hash values
  t3701: add failing test for pathological context lines
  add -p: Adjust offsets of subsequent hunks when one is skipped
  add -p: calculate offset delta for edited patches
  add -p: fix counting when splitting and coalescing
  add -p: don't rely on apply's '--recount' option

 git-add--interactive.perl  | 106 ++++++++++++-----
 t/t3701-add-interactive.sh | 281 ++++++++++++++++++++++++---------------------
 2 files changed, 229 insertions(+), 158 deletions(-)

-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 1/9] add -i: add function to format hunk header
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-20 18:21     ` Junio C Hamano
  2018-02-19 11:29   ` [PATCH v2 2/9] t3701: indent here documents Phillip Wood
                     ` (7 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This code is duplicated in a couple of places so make it into a
function as we're going to add some more callers shortly.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -751,6 +751,15 @@ sub parse_hunk_header {
 	return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 }
 
+sub format_hunk_header {
+	my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) = @_;
+	return ("@@ -$o_ofs" .
+		(($o_cnt != 1) ? ",$o_cnt" : '') .
+		" +$n_ofs" .
+		(($n_cnt != 1) ? ",$n_cnt" : '') .
+		" @@\n");
+}
+
 sub split_hunk {
 	my ($text, $display) = @_;
 	my @split = ();
@@ -838,11 +847,7 @@ sub split_hunk {
 		my $o_cnt = $hunk->{OCNT};
 		my $n_cnt = $hunk->{NCNT};
 
-		my $head = ("@@ -$o_ofs" .
-			    (($o_cnt != 1) ? ",$o_cnt" : '') .
-			    " +$n_ofs" .
-			    (($n_cnt != 1) ? ",$n_cnt" : '') .
-			    " @@\n");
+		my $head = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
 		my $display_head = $head;
 		unshift @{$hunk->{TEXT}}, $head;
 		if ($diff_use_color) {
@@ -912,11 +917,7 @@ sub merge_hunk {
 		}
 		push @line, $line;
 	}
-	my $head = ("@@ -$o0_ofs" .
-		    (($o_cnt != 1) ? ",$o_cnt" : '') .
-		    " +$n0_ofs" .
-		    (($n_cnt != 1) ? ",$n_cnt" : '') .
-		    " @@\n");
+	my $head = format_hunk_header($o0_ofs, $o_cnt, $n0_ofs, $n_cnt);
 	@{$prev->{TEXT}} = ($head, @line);
 }
 
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 2/9] t3701: indent here documents
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 1/9] add -i: add function to format hunk header Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 18:36     ` Eric Sunshine
  2018-02-19 11:29   ` [PATCH v2 3/9] t3701: use test_write_lines and write_script Phillip Wood
                     ` (6 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Indent here documents in line with the current style for tests.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3701-add-interactive.sh | 174 ++++++++++++++++++++++-----------------------
 1 file changed, 87 insertions(+), 87 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 058698df6a4a9811b9db84fb5900472c47c61798..861ea2e08cce750515f59fc424b3f8336fd9b1a9 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '
 '
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-new file mode 100644
-index 0000000..d95f3ad
---- /dev/null
-+++ b/file
-@@ -0,0 +1 @@
-+content
-EOF
+	cat >expected <<-EOF
+	new file mode 100644
+	index 0000000..d95f3ad
+	--- /dev/null
+	+++ b/file
+	@@ -0,0 +1 @@
+	+content
+	EOF
 '
 
 test_expect_success 'diff works (initial)' '
@@ -59,14 +59,14 @@ test_expect_success 'status works (commit)' '
 '
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-index 180b47c..b6f2c08 100644
---- a/file
-+++ b/file
-@@ -1 +1,2 @@
- baseline
-+content
-EOF
+	cat >expected <<-EOF
+	index 180b47c..b6f2c08 100644
+	--- a/file
+	+++ b/file
+	@@ -1 +1,2 @@
+	 baseline
+	+content
+	EOF
 '
 
 test_expect_success 'diff works (commit)' '
@@ -83,8 +83,8 @@ test_expect_success 'revert works (commit)' '
 
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-EOF
+	cat >expected <<-EOF
+	EOF
 '
 
 test_expect_success 'setup fake editor' '
@@ -100,21 +100,21 @@ test_expect_success 'dummy edit works' '
 '
 
 test_expect_success 'setup patch' '
-cat >patch <<EOF
-@@ -1,1 +1,4 @@
- this
-+patch
--does not
- apply
-EOF
+	cat >patch <<-EOF
+	@@ -1,1 +1,4 @@
+	 this
+	+patch
+	-does not
+	 apply
+	EOF
 '
 
 test_expect_success 'setup fake editor' '
 	echo "#!$SHELL_PATH" >fake_editor.sh &&
-	cat >>fake_editor.sh <<\EOF &&
-mv -f "$1" oldpatch &&
-mv -f patch "$1"
-EOF
+	cat >>fake_editor.sh <<-\EOF &&
+	mv -f "$1" oldpatch &&
+	mv -f patch "$1"
+	EOF
 	chmod a+x fake_editor.sh &&
 	test_set_editor "$(pwd)/fake_editor.sh"
 '
@@ -126,10 +126,10 @@ test_expect_success 'bad edit rejected' '
 '
 
 test_expect_success 'setup patch' '
-cat >patch <<EOF
-this patch
-is garbage
-EOF
+	cat >patch <<-EOF
+	this patch
+	is garbage
+	EOF
 '
 
 test_expect_success 'garbage edit rejected' '
@@ -139,28 +139,28 @@ test_expect_success 'garbage edit rejected' '
 '
 
 test_expect_success 'setup patch' '
-cat >patch <<EOF
-@@ -1,0 +1,0 @@
- baseline
-+content
-+newcontent
-+lines
-EOF
+	cat >patch <<-EOF
+	@@ -1,0 +1,0 @@
+	 baseline
+	+content
+	+newcontent
+	+lines
+	EOF
 '
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-diff --git a/file b/file
-index b5dd6c9..f910ae9 100644
---- a/file
-+++ b/file
-@@ -1,4 +1,4 @@
- baseline
- content
--newcontent
-+more
- lines
-EOF
+	cat >expected <<-EOF
+	diff --git a/file b/file
+	index b5dd6c9..f910ae9 100644
+	--- a/file
+	+++ b/file
+	@@ -1,4 +1,4 @@
+	 baseline
+	 content
+	-newcontent
+	+more
+	 lines
+	EOF
 '
 
 test_expect_success 'real edit works' '
@@ -222,31 +222,31 @@ test_expect_success 'setup again' '
 
 # Write the patch file with a new line at the top and bottom
 test_expect_success 'setup patch' '
-cat >patch <<EOF
-index 180b47c..b6f2c08 100644
---- a/file
-+++ b/file
-@@ -1,2 +1,4 @@
-+firstline
- baseline
- content
-+lastline
-EOF
+	cat >patch <<-EOF
+	index 180b47c..b6f2c08 100644
+	--- a/file
+	+++ b/file
+	@@ -1,2 +1,4 @@
+	+firstline
+	 baseline
+	 content
+	+lastline
+	EOF
 '
 
 # Expected output, similar to the patch but w/ diff at the top
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-diff --git a/file b/file
-index b6f2c08..61b9053 100755
---- a/file
-+++ b/file
-@@ -1,2 +1,4 @@
-+firstline
- baseline
- content
-+lastline
-EOF
+	cat >expected <<-EOF
+	diff --git a/file b/file
+	index b6f2c08..61b9053 100755
+	--- a/file
+	+++ b/file
+	@@ -1,2 +1,4 @@
+	+firstline
+	 baseline
+	 content
+	+lastline
+	EOF
 '
 
 # Test splitting the first patch, then adding both
@@ -259,15 +259,15 @@ test_expect_success 'add first line works' '
 '
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-diff --git a/non-empty b/non-empty
-deleted file mode 100644
-index d95f3ad..0000000
---- a/non-empty
-+++ /dev/null
-@@ -1 +0,0 @@
--content
-EOF
+	cat >expected <<-EOF
+	diff --git a/non-empty b/non-empty
+	deleted file mode 100644
+	index d95f3ad..0000000
+	--- a/non-empty
+	+++ /dev/null
+	@@ -1 +0,0 @@
+	-content
+	EOF
 '
 
 test_expect_success 'deleting a non-empty file' '
@@ -282,11 +282,11 @@ test_expect_success 'deleting a non-empty file' '
 '
 
 test_expect_success 'setup expected' '
-cat >expected <<EOF
-diff --git a/empty b/empty
-deleted file mode 100644
-index e69de29..0000000
-EOF
+	cat >expected <<-EOF
+	diff --git a/empty b/empty
+	deleted file mode 100644
+	index e69de29..0000000
+	EOF
 '
 
 test_expect_success 'deleting an empty file' '
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 3/9] t3701: use test_write_lines and write_script
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 1/9] add -i: add function to format hunk header Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 2/9] t3701: indent here documents Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 18:47     ` Eric Sunshine
  2018-02-19 11:29   ` [PATCH v2 4/9] t3701: don't hard code sha1 hash values Phillip Wood
                     ` (5 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Simplify things slightly by using the above helpers.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3701-add-interactive.sh | 36 +++++++-----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 861ea2e08cce750515f59fc424b3f8336fd9b1a9..b73a9598ab3eaed074735e99f3dcbc8f88d86f4c 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '
 	EOF
 '
 
-test_expect_success 'setup fake editor' '
-	>fake_editor.sh &&
-	chmod a+x fake_editor.sh &&
-	test_set_editor "$(pwd)/fake_editor.sh"
-'
-
 test_expect_success 'dummy edit works' '
+	test_set_editor : &&
 	(echo e; echo a) | git add -p &&
 	git diff > diff &&
 	test_cmp expected diff
@@ -110,13 +105,12 @@ test_expect_success 'setup patch' '
 '
 
 test_expect_success 'setup fake editor' '
-	echo "#!$SHELL_PATH" >fake_editor.sh &&
-	cat >>fake_editor.sh <<-\EOF &&
+	FAKE_EDITOR="$(pwd)/fake-editor.sh" &&
+	write_script "$FAKE_EDITOR" <<-\EOF &&
 	mv -f "$1" oldpatch &&
 	mv -f patch "$1"
 	EOF
-	chmod a+x fake_editor.sh &&
-	test_set_editor "$(pwd)/fake_editor.sh"
+	test_set_editor "$FAKE_EDITOR"
 '
 
 test_expect_success 'bad edit rejected' '
@@ -302,18 +296,12 @@ test_expect_success 'deleting an empty file' '
 
 test_expect_success 'split hunk setup' '
 	git reset --hard &&
-	for i in 10 20 30 40 50 60
-	do
-		echo $i
-	done >test &&
+	test_write_lines 10 20 30 40 50 60 >test &&
 	git add test &&
 	test_tick &&
 	git commit -m test &&
 
-	for i in 10 15 20 21 22 23 24 30 40 50 60
-	do
-		echo $i
-	done >test
+	test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test
 '
 
 test_expect_success 'split hunk "add -p (edit)"' '
@@ -334,17 +322,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
 '
 
 test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
-	cat >test <<-\EOF &&
-	5
-	10
-	20
-	21
-	30
-	31
-	40
-	50
-	60
-	EOF
+	test_write_lines 5 10 20 21 30 31 40 50 60 >test &&
 	git reset &&
 	# test sequence is s(plit), n(o), y(es), e(dit)
 	# q n q q is there to make sure we exit at the end.
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 4/9] t3701: don't hard code sha1 hash values
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (2 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 3/9] t3701: use test_write_lines and write_script Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 18:52     ` Eric Sunshine
  2018-02-20 17:39     ` Junio C Hamano
  2018-02-19 11:29   ` [PATCH v2 5/9] t3701: add failing test for pathological context lines Phillip Wood
                     ` (4 subsequent siblings)
  8 siblings, 2 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Purge the index lines from diffs so we're not hard coding sha1 hash
values in the expected output.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t3701-add-interactive.sh | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index b73a9598ab3eaed074735e99f3dcbc8f88d86f4c..70748393f28c93f4bc5d43b07bd96bd208aba3e9 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -24,7 +24,6 @@ test_expect_success 'status works (initial)' '
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
 	new file mode 100644
-	index 0000000..d95f3ad
 	--- /dev/null
 	+++ b/file
 	@@ -0,0 +1 @@
@@ -34,7 +33,7 @@ test_expect_success 'setup expected' '
 
 test_expect_success 'diff works (initial)' '
 	(echo d; echo 1) | git add -i >output &&
-	sed -ne "/new file/,/content/p" <output >diff &&
+	sed -ne /^index/d -e "/new file/,/content/p" <output >diff &&
 	test_cmp expected diff
 '
 test_expect_success 'revert works (initial)' '
@@ -60,7 +59,6 @@ test_expect_success 'status works (commit)' '
 
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
-	index 180b47c..b6f2c08 100644
 	--- a/file
 	+++ b/file
 	@@ -1 +1,2 @@
@@ -71,7 +69,7 @@ test_expect_success 'setup expected' '
 
 test_expect_success 'diff works (commit)' '
 	(echo d; echo 1) | git add -i >output &&
-	sed -ne "/^index/,/content/p" <output >diff &&
+	sed -ne "/^---/,/content/p" <output >diff &&
 	test_cmp expected diff
 '
 test_expect_success 'revert works (commit)' '
@@ -90,7 +88,7 @@ test_expect_success 'setup expected' '
 test_expect_success 'dummy edit works' '
 	test_set_editor : &&
 	(echo e; echo a) | git add -p &&
-	git diff > diff &&
+	git diff | sed /^index/d >diff &&
 	test_cmp expected diff
 '
 
@@ -145,7 +143,6 @@ test_expect_success 'setup patch' '
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
 	diff --git a/file b/file
-	index b5dd6c9..f910ae9 100644
 	--- a/file
 	+++ b/file
 	@@ -1,4 +1,4 @@
@@ -159,7 +156,7 @@ test_expect_success 'setup expected' '
 
 test_expect_success 'real edit works' '
 	(echo e; echo n; echo d) | git add -p &&
-	git diff >output &&
+	git diff | sed /^index/d >output &&
 	test_cmp expected output
 '
 
@@ -168,10 +165,10 @@ test_expect_success 'skip files similarly as commit -a' '
 	echo file >.gitignore &&
 	echo changed >file &&
 	echo y | git add -p file &&
-	git diff >output &&
+	git diff | sed /^index/d >output &&
 	git reset &&
 	git commit -am commit &&
-	git diff >expected &&
+	git diff | sed /^index/d >expected &&
 	test_cmp expected output &&
 	git reset --hard HEAD^
 '
@@ -217,7 +214,6 @@ test_expect_success 'setup again' '
 # Write the patch file with a new line at the top and bottom
 test_expect_success 'setup patch' '
 	cat >patch <<-EOF
-	index 180b47c..b6f2c08 100644
 	--- a/file
 	+++ b/file
 	@@ -1,2 +1,4 @@
@@ -232,7 +228,6 @@ test_expect_success 'setup patch' '
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
 	diff --git a/file b/file
-	index b6f2c08..61b9053 100755
 	--- a/file
 	+++ b/file
 	@@ -1,2 +1,4 @@
@@ -248,15 +243,14 @@ test_expect_success 'add first line works' '
 	git commit -am "clear local changes" &&
 	git apply patch &&
 	(echo s; echo y; echo y) | git add -p file &&
-	git diff --cached > diff &&
+	git diff --cached | sed /^index/d >diff &&
 	test_cmp expected diff
 '
 
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
 	diff --git a/non-empty b/non-empty
 	deleted file mode 100644
-	index d95f3ad..0000000
 	--- a/non-empty
 	+++ /dev/null
 	@@ -1 +0,0 @@
@@ -271,15 +265,14 @@ test_expect_success 'deleting a non-empty file' '
 	git commit -m non-empty &&
 	rm non-empty &&
 	echo y | git add -p non-empty &&
-	git diff --cached >diff &&
+	git diff --cached | sed /^index/d >diff &&
 	test_cmp expected diff
 '
 
 test_expect_success 'setup expected' '
 	cat >expected <<-EOF
 	diff --git a/empty b/empty
 	deleted file mode 100644
-	index e69de29..0000000
 	EOF
 '
 
@@ -290,7 +283,7 @@ test_expect_success 'deleting an empty file' '
 	git commit -m empty &&
 	rm empty &&
 	echo y | git add -p empty &&
-	git diff --cached >diff &&
+	git diff --cached | sed /^index/d >diff &&
 	test_cmp expected diff
 '
 
@@ -317,7 +310,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
 	#    exits.
 	printf "%s\n" s e     q n q q |
 	EDITOR=: git add -p &&
-	git diff >actual &&
+	git diff | sed /^index/d >actual &&
 	! grep "^+15" actual
 '
 
@@ -329,7 +322,7 @@ test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
 	printf "%s\n" s n y e   q n q q |
 	EDITOR=: git add -p 2>error &&
 	test_must_be_empty error &&
-	git diff >actual &&
+	git diff | sed /^index/d >actual &&
 	! grep "^+31" actual
 '
 
@@ -348,14 +341,13 @@ test_expect_success 'patch mode ignores unmerged entries' '
 	cat >expected <<-\EOF &&
 	* Unmerged path conflict.t
 	diff --git a/non-conflict.t b/non-conflict.t
-	index f766221..5ea2ed4 100644
 	--- a/non-conflict.t
 	+++ b/non-conflict.t
 	@@ -1 +1 @@
 	-non-conflict
 	+changed
 	EOF
-	git diff --cached >diff &&
+	git diff --cached | sed /^index/d >diff &&
 	test_cmp expected diff
 '
 
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 5/9] t3701: add failing test for pathological context lines
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (3 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 4/9] t3701: don't hard code sha1 hash values Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-21 11:28     ` Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
                     ` (3 subsequent siblings)
  8 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When a hunk is skipped by add -i the offsets of subsequent hunks are
not adjusted to account for any missing insertions due to the skipped
hunk. Most of the time this does not matter as apply uses the context
lines to apply the subsequent hunks in the correct place, however in
pathological cases the context lines will match at the now incorrect
offset and the hunk will be applied in the wrong place. The offsets of
hunks following an edited hunk that has had the number of insertions
or deletions changed also need to be updated in the same way. Add
failing tests to demonstrate this.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

Notes:
    changes since v1:
     - changed edit test to use the existing fake editor and to strip
       the hunk header and some context lines as well as deleting an
       insertion
     - style fixes

 t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 70748393f28c93f4bc5d43b07bd96bd208aba3e9..06c4747f506a1b05ccad0f9387e1fbd69cfd7784 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -511,4 +511,35 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '
 	! grep dirty-otherwise output
 '
 
+test_expect_success 'set up pathological context' '
+	git reset --hard &&
+	test_write_lines a a a a a a a a a a a >a &&
+	git add a &&
+	git commit -m a &&
+	test_write_lines c b a a a a a a a b a a a a >a &&
+	test_write_lines     a a a a a a a b a a a a >expected-1 &&
+	test_write_lines   b a a a a a a a b a a a a >expected-2 &&
+	# check editing can cope with missing header and deleted context lines
+	# as well as changes to other lines
+	test_write_lines +b " a" >patch
+'
+
+test_expect_failure 'add -p works with pathological context lines' '
+	git reset &&
+	printf "%s\n" n y |
+	git add -p &&
+	git cat-file blob :a >actual &&
+	test_cmp expected-1 actual
+'
+
+test_expect_failure 'add -p patch editing works with pathological context lines' '
+	git reset &&
+	test_set_editor "$FAKE_EDITOR" &&
+	# n q q below is in case edit fails
+	printf "%s\n" e y    n q q |
+	git add -p &&
+	git cat-file blob :a >actual &&
+	test_cmp expected-2 actual
+'
+
 test_done
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (4 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 5/9] t3701: add failing test for pathological context lines Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 7/9] add -p: calculate offset delta for edited patches Phillip Wood
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Since commit 8cbd431082 ("git-add--interactive: replace hunk
recounting with apply --recount", 2008-7-2) if a hunk is skipped then
we rely on the context lines to apply subsequent hunks in the right
place. While this works most of the time it is possible for hunks to
end up being applied in the wrong place. To fix this adjust the offset
of subsequent hunks to correct for any change in the number of
insertions or deletions due to the skipped hunk. The change in offset
due to edited hunks that have the number of insertions or deletions
changed is ignored here, it will be fixed in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl  | 15 +++++++++++++--
 t/t3701-add-interactive.sh |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8ababa6453ac4f57a925aacbb8b9bb1c973e4a54..7a0a5896bb8fa063ace29b85840849c867b874f5 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -926,14 +926,25 @@ sub coalesce_overlapping_hunks {
 	my @out = ();
 
 	my ($last_o_ctx, $last_was_dirty);
+	my $ofs_delta = 0;
 
-	for (grep { $_->{USE} } @in) {
+	for (@in) {
 		if ($_->{TYPE} ne 'hunk') {
 			push @out, $_;
 			next;
 		}
 		my $text = $_->{TEXT};
-		my ($o_ofs) = parse_hunk_header($text->[0]);
+		my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
+						parse_hunk_header($text->[0]);
+		unless ($_->{USE}) {
+			$ofs_delta += $o_cnt - $n_cnt;
+			next;
+		}
+		if ($ofs_delta) {
+			$n_ofs += $ofs_delta;
+			$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,
+							     $n_ofs, $n_cnt);
+		}
 		if (defined $last_o_ctx &&
 		    $o_ofs <= $last_o_ctx &&
 		    !$_->{DIRTY} &&
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 06c4747f506a1b05ccad0f9387e1fbd69cfd7784..e153a02605df25ea40e49dd48b7c9fd981713b9d 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -524,7 +524,7 @@ test_expect_success 'set up pathological context' '
 	test_write_lines +b " a" >patch
 '
 
-test_expect_failure 'add -p works with pathological context lines' '
+test_expect_success 'add -p works with pathological context lines' '
 	git reset &&
 	printf "%s\n" n y |
 	git add -p &&
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 7/9] add -p: calculate offset delta for edited patches
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (5 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 8/9] add -p: fix counting when splitting and coalescing Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option Phillip Wood
  8 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Recount the number of preimage and postimage lines in a hunk after it
has been edited so any change in the number of insertions or deletions
can be used to adjust the offsets of subsequent hunks. If an edited
hunk is subsequently split then the offset correction will be lost. It
would be possible to fix this if it is a problem, however the code
here is still an improvement on the status quo for the common case
where an edited hunk is applied without being split.

This is also a necessary step to removing '--recount' and
'--allow-overlap' from the invocation of 'git apply'. Before
'--recount' can be removed the splitting and coalescing counting needs
to be fixed to handle a missing newline at the end of a file. In order
to remove '--allow-overlap' there needs to be i) some way of verifying
the offset data in the edited hunk (probably by correlating the
preimage (or postimage if the patch is going to be applied in reverse)
lines of the edited and unedited versions to see if they are offset or
if any leading/trailing context lines have been removed) and ii) a way of
dealing with edited hunks that change context lines that are shared
with neighbouring hunks.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

Notes:
    changes since v1
     - edited hunks are now recounted before seeing if they apply in
       preparation for removing '--recount' when invoking 'git apply'
     - added sentence to commit message about the offset data being lost
       if an edited hunk is split

 git-add--interactive.perl  | 55 ++++++++++++++++++++++++++++++++++++++--------
 t/t3701-add-interactive.sh |  2 +-
 2 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 7a0a5896bb8fa063ace29b85840849c867b874f5..0df0c2aa065af88e159f8e9a2febe12f4ef8ee75 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -938,13 +938,19 @@ sub coalesce_overlapping_hunks {
 						parse_hunk_header($text->[0]);
 		unless ($_->{USE}) {
 			$ofs_delta += $o_cnt - $n_cnt;
+			# If this hunk has been edited then subtract
+			# the delta that is due to the edit.
+			$_->{OFS_DELTA} and $ofs_delta -= $_->{OFS_DELTA};
 			next;
 		}
 		if ($ofs_delta) {
 			$n_ofs += $ofs_delta;
 			$_->{TEXT}->[0] = format_hunk_header($o_ofs, $o_cnt,
 							     $n_ofs, $n_cnt);
 		}
+		# If this hunk was edited then adjust the offset delta
+		# to reflect the edit.
+		$_->{OFS_DELTA} and $ofs_delta += $_->{OFS_DELTA};
 		if (defined $last_o_ctx &&
 		    $o_ofs <= $last_o_ctx &&
 		    !$_->{DIRTY} &&
@@ -1016,6 +1022,30 @@ marked for discarding."),
 marked for applying."),
 );
 
+sub recount_edited_hunk {
+	local $_;
+	my ($oldtext, $newtext) = @_;
+	my ($o_cnt, $n_cnt) = (0, 0);
+	for (@{$newtext}[1..$#{$newtext}]) {
+		my $mode = substr($_, 0, 1);
+		if ($mode eq '-') {
+			$o_cnt++;
+		} elsif ($mode eq '+') {
+			$n_cnt++;
+		} elsif ($mode eq ' ') {
+			$o_cnt++;
+			$n_cnt++;
+		}
+	}
+	my ($o_ofs, undef, $n_ofs, undef) =
+					parse_hunk_header($newtext->[0]);
+	$newtext->[0] = format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
+	my (undef, $orig_o_cnt, undef, $orig_n_cnt) =
+					parse_hunk_header($oldtext->[0]);
+	# Return the change in the number of lines inserted by this hunk
+	return $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt;
+}
+
 sub edit_hunk_manually {
 	my ($oldtext) = @_;
 
@@ -1114,25 +1144,32 @@ sub prompt_yesno {
 }
 
 sub edit_hunk_loop {
-	my ($head, $hunk, $ix) = @_;
-	my $text = $hunk->[$ix]->{TEXT};
+	my ($head, $hunks, $ix) = @_;
+	my $hunk = $hunks->[$ix];
+	my $text = $hunk->{TEXT};
 
 	while (1) {
-		$text = edit_hunk_manually($text);
-		if (!defined $text) {
+		my $newtext = edit_hunk_manually($text);
+		if (!defined $newtext) {
 			return undef;
 		}
 		my $newhunk = {
-			TEXT => $text,
-			TYPE => $hunk->[$ix]->{TYPE},
+			TEXT => $newtext,
+			TYPE => $hunk->{TYPE},
 			USE => 1,
 			DIRTY => 1,
 		};
+		$newhunk->{OFS_DELTA} = recount_edited_hunk($text, $newtext);
+		# If this hunk has already been edited then add the
+		# offset delta of the previous edit to get the real
+		# delta from the original unedited hunk.
+		$hunk->{OFS_DELTA} and
+				$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};
 		if (diff_applies($head,
-				 @{$hunk}[0..$ix-1],
+				 @{$hunks}[0..$ix-1],
 				 $newhunk,
-				 @{$hunk}[$ix+1..$#{$hunk}])) {
-			$newhunk->{DISPLAY} = [color_diff(@{$text})];
+				 @{$hunks}[$ix+1..$#{$hunks}])) {
+			$newhunk->{DISPLAY} = [color_diff(@{$newtext})];
 			return $newhunk;
 		}
 		else {
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index e153a02605df25ea40e49dd48b7c9fd981713b9d..bbda771ba7e516aa37a204beffba7eeb0c85a2f4 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -532,7 +532,7 @@ test_expect_success 'add -p works with pathological context lines' '
 	test_cmp expected-1 actual
 '
 
-test_expect_failure 'add -p patch editing works with pathological context lines' '
+test_expect_success 'add -p patch editing works with pathological context lines' '
 	git reset &&
 	test_set_editor "$FAKE_EDITOR" &&
 	# n q q below is in case edit fails
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 8/9] add -p: fix counting when splitting and coalescing
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (6 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 7/9] add -p: calculate offset delta for edited patches Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-19 11:29   ` [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option Phillip Wood
  8 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

When a file has no trailing new line at the end diff records this by
appending "\ No newline at end of file" below the last line of the
file. This line should not be counted in the hunk header. Fix the
splitting and coalescing code to count files without a trailing new line
properly and change one of the tests to test splitting without a
trailing new line.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl  | 13 +++++++++----
 t/t3701-add-interactive.sh | 28 ++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 0df0c2aa065af88e159f8e9a2febe12f4ef8ee75..3226c2c4f02d5f8679d77b8eede984fc727b422d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -793,6 +793,11 @@ sub split_hunk {
 		while (++$i < @$text) {
 			my $line = $text->[$i];
 			my $display = $display->[$i];
+			if ($line =~ /^\\/) {
+				push @{$this->{TEXT}}, $line;
+				push @{$this->{DISPLAY}}, $display;
+				next;
+			}
 			if ($line =~ /^ /) {
 				if ($this->{ADDDEL} &&
 				    !defined $next_hunk_start) {
@@ -887,8 +892,8 @@ sub merge_hunk {
 	$o_cnt = $n_cnt = 0;
 	for ($i = 1; $i < @{$prev->{TEXT}}; $i++) {
 		my $line = $prev->{TEXT}[$i];
-		if ($line =~ /^\+/) {
-			$n_cnt++;
+		if ($line =~ /^[+\\]/) {
+			$n_cnt++ if ($line =~ /^\+/);
 			push @line, $line;
 			next;
 		}
@@ -905,8 +910,8 @@ sub merge_hunk {
 
 	for ($i = 1; $i < @{$this->{TEXT}}; $i++) {
 		my $line = $this->{TEXT}[$i];
-		if ($line =~ /^\+/) {
-			$n_cnt++;
+		if ($line =~ /^[+\\]/) {
+			$n_cnt++ if ($line =~ /^\+/);
 			push @line, $line;
 			next;
 		}
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index bbda771ba7e516aa37a204beffba7eeb0c85a2f4..0fb9c0e0f140e21ef7ad467c40b9211d29f53db6 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -221,30 +221,46 @@ test_expect_success 'setup patch' '
 	 baseline
 	 content
 	+lastline
+	\ No newline at end of file
 	EOF
 '
 
-# Expected output, similar to the patch but w/ diff at the top
+# Expected output, diff is similar to the patch but w/ diff at the top
 test_expect_success 'setup expected' '
-	cat >expected <<-EOF
-	diff --git a/file b/file
+	echo diff --git a/file b/file >expected &&
+	cat patch >>expected &&
+	cat >expected-output <<-EOF
 	--- a/file
 	+++ b/file
 	@@ -1,2 +1,4 @@
 	+firstline
 	 baseline
 	 content
 	+lastline
+	\ No newline at end of file
+	@@ -1,2 +1,3 @@
+	+firstline
+	 baseline
+	 content
+	@@ -1,2 +2,3 @@
+	 baseline
+	 content
+	+lastline
+	\ No newline at end of file
 	EOF
 '
 
 # Test splitting the first patch, then adding both
-test_expect_success 'add first line works' '
+test_expect_success C_LOCALE_OUTPUT 'add first line works' '
 	git commit -am "clear local changes" &&
 	git apply patch &&
-	(echo s; echo y; echo y) | git add -p file &&
+	printf "%s\n" s y y | git add -p file 2>error |
+		sed -n -e "s/^Stage this hunk[^@]*\(@@ .*\)/\1/" \
+		       -e "/^[-+@ \\\\]"/p  >output &&
+	test_must_be_empty error &&
 	git diff --cached | sed /^index/d >diff &&
-	test_cmp expected diff
+	test_cmp expected diff &&
+	test_cmp expected-output output
 '
 
 test_expect_success 'setup expected' '
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option
  2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
                     ` (7 preceding siblings ...)
  2018-02-19 11:29   ` [PATCH v2 8/9] add -p: fix counting when splitting and coalescing Phillip Wood
@ 2018-02-19 11:29   ` Phillip Wood
  2018-02-20 18:50     ` Junio C Hamano
  8 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 11:29 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Brian M. Carlson, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Now that add -p counts patches properly it should be possible to turn
off the '--recount' option when invoking 'git apply'

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---

Notes:
    I can't think of a reason why this shouldn't be OK but I can't help
    feeling slightly nervous about it. I've made it a separate patch so it
    can be easily dropped or reverted if I've missed something.

 git-add--interactive.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 3226c2c4f02d5f8679d77b8eede984fc727b422d..a64c0db57d62ab02ef718b8c8f821105132d9920 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -677,7 +677,7 @@ sub add_untracked_cmd {
 sub run_git_apply {
 	my $cmd = shift;
 	my $fh;
-	open $fh, '| git ' . $cmd . " --recount --allow-overlap";
+	open $fh, '| git ' . $cmd . " --allow-overlap";
 	print $fh @_;
 	return close $fh;
 }
-- 
2.16.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 0/4] Correct offsets of hunks when one is skipped
  2018-02-13 23:56 ` [PATCH 0/4] Correct offsets of hunks when one is skipped brian m. carlson
@ 2018-02-19 13:01   ` Phillip Wood
  0 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-19 13:01 UTC (permalink / raw)
  To: brian m. carlson, Phillip Wood, Git Mailing List

On 13/02/18 23:56, brian m. carlson wrote:
> On Tue, Feb 13, 2018 at 10:44:04AM +0000, Phillip Wood wrote:
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> While working on a patch series to stage selected lines from a hunk
>> without having to edit it I got worried that subsequent patches would
>> be applied in the wrong place which lead to this series to correct the
>> offsets of hunks following those that are skipped or edited.
>>
>> Phillip Wood (4):
>>   add -i: add function to format hunk header
>>   t3701: add failing test for pathological context lines
>>   add -p: Adjust offsets of subsequent hunks when one is skipped
>>   add -p: calculate offset delta for edited patches
>>
>>  git-add--interactive.perl  | 93 +++++++++++++++++++++++++++++++++++-----------
>>  t/t3701-add-interactive.sh | 30 +++++++++++++++
>>  2 files changed, 102 insertions(+), 21 deletions(-)
> 
> This looks reasonably sane to me.  I really like that you managed to
> produce failing tests for this situation.  I know pathological cases
> like this have bit GCC in the past, so it's good that you fixed this.
> 

Thanks Brain, it's interesting to hear that GCC has been bitten in the past

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 2/9] t3701: indent here documents
  2018-02-19 11:29   ` [PATCH v2 2/9] t3701: indent here documents Phillip Wood
@ 2018-02-19 18:36     ` Eric Sunshine
  0 siblings, 0 replies; 29+ messages in thread
From: Eric Sunshine @ 2018-02-19 18:36 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Junio C Hamano, Brian M. Carlson

On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood <phillip.wood@talktalk.net> wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Indent here documents in line with the current style for tests.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> @@ -22,14 +22,14 @@ test_expect_success 'status works (initial)' '
>  test_expect_success 'setup expected' '
> -cat >expected <<EOF
> -new file mode 100644
> -index 0000000..d95f3ad
> ---- /dev/null
> -+++ b/file
> -@@ -0,0 +1 @@
> -+content
> -EOF
> +       cat >expected <<-EOF

Minor: You could take the opportunity to update these to use -\EOF
(rather than -EOF) to document that no variable interpolation is
expected inside the 'here' document. Probably itself not worth a
re-roll.

> +       new file mode 100644
> +       index 0000000..d95f3ad
> +       --- /dev/null
> +       +++ b/file
> +       @@ -0,0 +1 @@
> +       +content
> +       EOF
>  '

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script
  2018-02-19 11:29   ` [PATCH v2 3/9] t3701: use test_write_lines and write_script Phillip Wood
@ 2018-02-19 18:47     ` Eric Sunshine
  2018-02-20 17:19       ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Eric Sunshine @ 2018-02-19 18:47 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Junio C Hamano, Brian M. Carlson

On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood <phillip.wood@talktalk.net> wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Simplify things slightly by using the above helpers.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> @@ -87,13 +87,8 @@ test_expect_success 'setup expected' '
> @@ -110,13 +105,12 @@ test_expect_success 'setup patch' '
>  test_expect_success 'setup fake editor' '
> -       echo "#!$SHELL_PATH" >fake_editor.sh &&
> -       cat >>fake_editor.sh <<-\EOF &&
> +       FAKE_EDITOR="$(pwd)/fake-editor.sh" &&
> +       write_script "$FAKE_EDITOR" <<-\EOF &&
>         mv -f "$1" oldpatch &&
>         mv -f patch "$1"
>         EOF
> -       chmod a+x fake_editor.sh &&
> -       test_set_editor "$(pwd)/fake_editor.sh"
> +       test_set_editor "$FAKE_EDITOR"
>  '

The very first thing that test_set_editor() does is set FAKE_EDITOR to
the value of $1, so it is confusing to see it getting setting it here
first; the reader has to spend extra brain cycles wondering if
something non-obvious is going on that requires this manual
assignment. Perhaps drop the assignment altogether and just write
literal "fake_editor.sh" in the couple places it's needed (as was done
in the original code)?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values
  2018-02-19 11:29   ` [PATCH v2 4/9] t3701: don't hard code sha1 hash values Phillip Wood
@ 2018-02-19 18:52     ` Eric Sunshine
  2018-02-20 17:39     ` Junio C Hamano
  1 sibling, 0 replies; 29+ messages in thread
From: Eric Sunshine @ 2018-02-19 18:52 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Junio C Hamano, Brian M. Carlson

On Mon, Feb 19, 2018 at 6:29 AM, Phillip Wood <phillip.wood@talktalk.net> wrote:
> Purge the index lines from diffs so we're not hard coding sha1 hash
> values in the expected output.

Perhaps the commit message could provide a bit more explanation about
why this is a good idea. For instance, briefly mention that doing so
will ease Git's transition from SHA1 to some newer hash function.

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script
  2018-02-19 18:47     ` Eric Sunshine
@ 2018-02-20 17:19       ` Junio C Hamano
  2018-02-21 11:26         ` Phillip Wood
  0 siblings, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2018-02-20 17:19 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Phillip Wood, Git Mailing List, Brian M. Carlson

Eric Sunshine <sunshine@sunshineco.com> writes:

>>  test_expect_success 'setup fake editor' '
>> -       echo "#!$SHELL_PATH" >fake_editor.sh &&
>> -       cat >>fake_editor.sh <<-\EOF &&
>> +       FAKE_EDITOR="$(pwd)/fake-editor.sh" &&
>> +       write_script "$FAKE_EDITOR" <<-\EOF &&
>>         mv -f "$1" oldpatch &&
>>         mv -f patch "$1"
>>         EOF
>> -       chmod a+x fake_editor.sh &&
>> -       test_set_editor "$(pwd)/fake_editor.sh"
>> +       test_set_editor "$FAKE_EDITOR"
>>  '
>
> The very first thing that test_set_editor() does is set FAKE_EDITOR to
> the value of $1, so it is confusing to see it getting setting it here
> first; the reader has to spend extra brain cycles wondering if
> something non-obvious is going on that requires this manual
> assignment. Perhaps drop the assignment altogether and just write
> literal "fake_editor.sh" in the couple places it's needed (as was done
> in the original code)?

Yeah, I think $(pwd)/ prefix is needed at the final step (i.e. as
the first argument to test_set_editor) for this to be a faithful
rewrite but it is distracting having to write it anywhere else.

Other than that, this looks like a quite straight-forward cleanup.

Thanks, both.  Here is what I'd be queuing tentatively.

 t/t3701-add-interactive.sh | 33 +++++----------------------------
 1 file changed, 5 insertions(+), 28 deletions(-)

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 39c7423069..4a369fcb51 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '
 	EOF
 '
 
-test_expect_success 'setup fake editor' '
-	>fake_editor.sh &&
-	chmod a+x fake_editor.sh &&
-	test_set_editor "$(pwd)/fake_editor.sh"
-'
-
 test_expect_success 'dummy edit works' '
+	test_set_editor : &&
 	(echo e; echo a) | git add -p &&
 	git diff > diff &&
 	test_cmp expected diff
@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '
 '
 
 test_expect_success 'setup fake editor' '
-	echo "#!$SHELL_PATH" >fake_editor.sh &&
-	cat >>fake_editor.sh <<-\EOF &&
+	write_script "fake_editor.sh" <<-\EOF &&
 	mv -f "$1" oldpatch &&
 	mv -f patch "$1"
 	EOF
-	chmod a+x fake_editor.sh &&
 	test_set_editor "$(pwd)/fake_editor.sh"
 '
 
@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '
 
 test_expect_success 'split hunk setup' '
 	git reset --hard &&
-	for i in 10 20 30 40 50 60
-	do
-		echo $i
-	done >test &&
+	test_write_lines 10 20 30 40 50 60 >test &&
 	git add test &&
 	test_tick &&
 	git commit -m test &&
 
-	for i in 10 15 20 21 22 23 24 30 40 50 60
-	do
-		echo $i
-	done >test
+	test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test
 '
 
 test_expect_success 'split hunk "add -p (edit)"' '
@@ -334,17 +321,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
 '
 
 test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
-	cat >test <<-\EOF &&
-	5
-	10
-	20
-	21
-	30
-	31
-	40
-	50
-	60
-	EOF
+	test_write_lines 5 10 20 21 30 31 40 50 60 >test &&
 	git reset &&
 	# test sequence is s(plit), n(o), y(es), e(dit)
 	# q n q q is there to make sure we exit at the end.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values
  2018-02-19 11:29   ` [PATCH v2 4/9] t3701: don't hard code sha1 hash values Phillip Wood
  2018-02-19 18:52     ` Eric Sunshine
@ 2018-02-20 17:39     ` Junio C Hamano
  2018-02-21 11:42       ` Phillip Wood
  1 sibling, 1 reply; 29+ messages in thread
From: Junio C Hamano @ 2018-02-20 17:39 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Brian M. Carlson, Phillip Wood

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Purge the index lines from diffs so we're not hard coding sha1 hash
> values in the expected output.

The motivation of this patch is clear, but all-zero object name for
missing side of deletion or creation patch should not change when we
transition to any hash function.  Neither the permission bits shown
in the output (and whether the index line has the bits are shown on
it in the first place, i.e. the index line of a creation patch does
not, whilethe one in a modification patch does).

So I am a bit ambivalent about this change.

Perhaps have a filter that redacts, instead of removes, selected
pieces of information that are likely to change while hash
transition, and use that consistently to filter both the expected
output and the actual output before comparing?


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 1/9] add -i: add function to format hunk header
  2018-02-19 11:29   ` [PATCH v2 1/9] add -i: add function to format hunk header Phillip Wood
@ 2018-02-20 18:21     ` Junio C Hamano
  0 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2018-02-20 18:21 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Brian M. Carlson, Phillip Wood

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> This code is duplicated in a couple of places so make it into a
> function as we're going to add some more callers shortly.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  git-add--interactive.perl | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 964c3a75420db4751cf11125b68b6904112632f1..8ababa6453ac4f57a925aacbb8b9bb1c973e4a54 100755

This is quite a tangent, but please refrain from using full object
name on the index line.  Because it does not add much value in the
context of patch exchange for project of this size to use more than
7-12 hexdigits each, the only effect of doing so is to push the mode
bit far to the right.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option
  2018-02-19 11:29   ` [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option Phillip Wood
@ 2018-02-20 18:50     ` Junio C Hamano
  0 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2018-02-20 18:50 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Brian M. Carlson, Phillip Wood

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Now that add -p counts patches properly it should be possible to turn
> off the '--recount' option when invoking 'git apply'
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>
> Notes:
>     I can't think of a reason why this shouldn't be OK but I can't help
>     feeling slightly nervous about it. I've made it a separate patch so it
>     can be easily dropped or reverted if I've missed something.

It is a lot more preferrable approach to produce a patch with
information as precise as the producer (i.e. add -p) can and feed it
to the consumer (i.e. apply) than to rely on the --recount and the
--allow-overlap options that are mainly ways to force the consumer
make imprecise guesses and accepting potential garbage with looser
consistency checks.  So I very much like the direction (and the next
step may be to make sure we coalesce correctly so that we do not
have to use "--allow-overlap").

Thanks for working on this.  Let's see if the updated code really
"counts patches properly" as the log message claims to ;-)

>  git-add--interactive.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index 3226c2c4f02d5f8679d77b8eede984fc727b422d..a64c0db57d62ab02ef718b8c8f821105132d9920 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -677,7 +677,7 @@ sub add_untracked_cmd {
>  sub run_git_apply {
>  	my $cmd = shift;
>  	my $fh;
> -	open $fh, '| git ' . $cmd . " --recount --allow-overlap";
> +	open $fh, '| git ' . $cmd . " --allow-overlap";
>  	print $fh @_;
>  	return close $fh;
>  }

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 3/9] t3701: use test_write_lines and write_script
  2018-02-20 17:19       ` Junio C Hamano
@ 2018-02-21 11:26         ` Phillip Wood
  0 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-21 11:26 UTC (permalink / raw)
  To: Junio C Hamano, Eric Sunshine; +Cc: Phillip Wood, Git Mailing List, Brian M. Carlson

On 20/02/18 17:19, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
>>>   test_expect_success 'setup fake editor' '
>>> -       echo "#!$SHELL_PATH" >fake_editor.sh &&
>>> -       cat >>fake_editor.sh <<-\EOF &&
>>> +       FAKE_EDITOR="$(pwd)/fake-editor.sh" &&
>>> +       write_script "$FAKE_EDITOR" <<-\EOF &&
>>>          mv -f "$1" oldpatch &&
>>>          mv -f patch "$1"
>>>          EOF
>>> -       chmod a+x fake_editor.sh &&
>>> -       test_set_editor "$(pwd)/fake_editor.sh"
>>> +       test_set_editor "$FAKE_EDITOR"
>>>   '
>>
>> The very first thing that test_set_editor() does is set FAKE_EDITOR to
>> the value of $1, so it is confusing to see it getting setting it here
>> first; the reader has to spend extra brain cycles wondering if
>> something non-obvious is going on that requires this manual
>> assignment. Perhaps drop the assignment altogether and just write
>> literal "fake_editor.sh" in the couple places it's needed (as was done
>> in the original code)?
> 
> Yeah, I think $(pwd)/ prefix is needed at the final step (i.e. as
> the first argument to test_set_editor) for this to be a faithful
> rewrite but it is distracting having to write it anywhere else.
> 
> Other than that, this looks like a quite straight-forward cleanup.
> 
> Thanks, both.  Here is what I'd be queuing tentatively.

That looks good, thanks for fixing it up

Phillip
> 
>   t/t3701-add-interactive.sh | 33 +++++----------------------------
>   1 file changed, 5 insertions(+), 28 deletions(-)
> 
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 39c7423069..4a369fcb51 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -87,13 +87,8 @@ test_expect_success 'setup expected' '
>   	EOF
>   '
>   
> -test_expect_success 'setup fake editor' '
> -	>fake_editor.sh &&
> -	chmod a+x fake_editor.sh &&
> -	test_set_editor "$(pwd)/fake_editor.sh"
> -'
> -
>   test_expect_success 'dummy edit works' '
> +	test_set_editor : &&
>   	(echo e; echo a) | git add -p &&
>   	git diff > diff &&
>   	test_cmp expected diff
> @@ -110,12 +105,10 @@ test_expect_success 'setup patch' '
>   '
>   
>   test_expect_success 'setup fake editor' '
> -	echo "#!$SHELL_PATH" >fake_editor.sh &&
> -	cat >>fake_editor.sh <<-\EOF &&
> +	write_script "fake_editor.sh" <<-\EOF &&
>   	mv -f "$1" oldpatch &&
>   	mv -f patch "$1"
>   	EOF
> -	chmod a+x fake_editor.sh &&
>   	test_set_editor "$(pwd)/fake_editor.sh"
>   '
>   
> @@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '
>   
>   test_expect_success 'split hunk setup' '
>   	git reset --hard &&
> -	for i in 10 20 30 40 50 60
> -	do
> -		echo $i
> -	done >test &&
> +	test_write_lines 10 20 30 40 50 60 >test &&
>   	git add test &&
>   	test_tick &&
>   	git commit -m test &&
>   
> -	for i in 10 15 20 21 22 23 24 30 40 50 60
> -	do
> -		echo $i
> -	done >test
> +	test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test
>   '
>   
>   test_expect_success 'split hunk "add -p (edit)"' '
> @@ -334,17 +321,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
>   '
>   
>   test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
> -	cat >test <<-\EOF &&
> -	5
> -	10
> -	20
> -	21
> -	30
> -	31
> -	40
> -	50
> -	60
> -	EOF
> +	test_write_lines 5 10 20 21 30 31 40 50 60 >test &&
>   	git reset &&
>   	# test sequence is s(plit), n(o), y(es), e(dit)
>   	# q n q q is there to make sure we exit at the end.
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 5/9] t3701: add failing test for pathological context lines
  2018-02-19 11:29   ` [PATCH v2 5/9] t3701: add failing test for pathological context lines Phillip Wood
@ 2018-02-21 11:28     ` Phillip Wood
  0 siblings, 0 replies; 29+ messages in thread
From: Phillip Wood @ 2018-02-21 11:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Phillip Wood

On 19/02/18 11:29, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> When a hunk is skipped by add -i the offsets of subsequent hunks are
> not adjusted to account for any missing insertions due to the skipped
> hunk. Most of the time this does not matter as apply uses the context
> lines to apply the subsequent hunks in the correct place, however in
> pathological cases the context lines will match at the now incorrect
> offset and the hunk will be applied in the wrong place. The offsets of
> hunks following an edited hunk that has had the number of insertions
> or deletions changed also need to be updated in the same way. Add
> failing tests to demonstrate this.
> 
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> 
> Notes:
>      changes since v1:
>       - changed edit test to use the existing fake editor and to strip
>         the hunk header and some context lines as well as deleting an
>         insertion
>       - style fixes
> 
>   t/t3701-add-interactive.sh | 31 +++++++++++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
> 
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 70748393f28c93f4bc5d43b07bd96bd208aba3e9..06c4747f506a1b05ccad0f9387e1fbd69cfd7784 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -511,4 +511,35 @@ test_expect_success 'status ignores dirty submodules (except HEAD)' '
>   	! grep dirty-otherwise output
>   '
>   
> +test_expect_success 'set up pathological context' '
> +	git reset --hard &&
> +	test_write_lines a a a a a a a a a a a >a &&
> +	git add a &&
> +	git commit -m a &&
> +	test_write_lines c b a a a a a a a b a a a a >a &&
> +	test_write_lines     a a a a a a a b a a a a >expected-1 &&
> +	test_write_lines   b a a a a a a a b a a a a >expected-2 &&
> +	# check editing can cope with missing header and deleted context lines
> +	# as well as changes to other lines
> +	test_write_lines +b " a" >patch
> +'
> +
> +test_expect_failure 'add -p works with pathological context lines' '
> +	git reset &&
> +	printf "%s\n" n y |
> +	git add -p &&
> +	git cat-file blob :a >actual &&
> +	test_cmp expected-1 actual
> +'
> +
> +test_expect_failure 'add -p patch editing works with pathological context lines' '
> +	git reset &&
> +	test_set_editor "$FAKE_EDITOR" &&
In light of the discussion of patch 2, I think this line should be deleted


> +	# n q q below is in case edit fails
> +	printf "%s\n" e y    n q q |
> +	git add -p &&
> +	git cat-file blob :a >actual &&
> +	test_cmp expected-2 actual
> +'
> +
>   test_done
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values
  2018-02-20 17:39     ` Junio C Hamano
@ 2018-02-21 11:42       ` Phillip Wood
  2018-02-21 16:58         ` Junio C Hamano
  0 siblings, 1 reply; 29+ messages in thread
From: Phillip Wood @ 2018-02-21 11:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Brian M. Carlson, Phillip Wood

On 20/02/18 17:39, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> Purge the index lines from diffs so we're not hard coding sha1 hash
>> values in the expected output.
> 
> The motivation of this patch is clear, but all-zero object name for
> missing side of deletion or creation patch should not change when we
> transition to any hash function.  Neither the permission bits shown
> in the output (and whether the index line has the bits are shown on
> it in the first place, i.e. the index line of a creation patch does
> not, whilethe one in a modification patch does).
> 
> So I am a bit ambivalent about this change.
> 
> Perhaps have a filter that redacts, instead of removes, selected
> pieces of information that are likely to change while hash
> transition, and use that consistently to filter both the expected
> output and the actual output before comparing?
> 
Keeping the permission bits makes sense (I'd not thought of them when I 
created the patch) as we want to check that the file has the correct 
permissions. As for the all-zero object name, is it really worth leaving 
it in - if a file has been created or deleted then we'll still have 
/dev/null as the file name for one side or the other and the diff lines 
will show it as well. As these tests are just to check the state of the 
index then I'm not sure the hash values add anything. How do you feel 
about a filter like

sed "/^	index/s/ [0-9a-f][0-9a-f]*\.\.[0-9a-f][0-9a-f]*/x/"

Best Wishes

Phillip

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v2 4/9] t3701: don't hard code sha1 hash values
  2018-02-21 11:42       ` Phillip Wood
@ 2018-02-21 16:58         ` Junio C Hamano
  0 siblings, 0 replies; 29+ messages in thread
From: Junio C Hamano @ 2018-02-21 16:58 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Git Mailing List, Brian M. Carlson, Phillip Wood

Phillip Wood <phillip.wood@talktalk.net> writes:

> Keeping the permission bits makes sense (I'd not thought of them when
> I created the patch) as we want to check that the file has the correct
> permissions. As for the all-zero object name, is it really worth
> leaving it in - if a file has been created or deleted then we'll still
> have /dev/null as the file name for one side or the other and the diff
> lines will show it as well. As these tests are just to check the state
> of the index then I'm not sure the hash values add anything. How do
> you feel about a filter like
>
> sed "/^	index/s/ [0-9a-f][0-9a-f]*\.\.[0-9a-f][0-9a-f]*/x/"

Something like that, with perhaps the single 'x' replaced with
something more normal looking and the all-zero thing special cased,
was what I had in mind.

Special casing all-zero matters.  I know that the current code uses
all-zero on the missing side.  The thing is, tests are about
protecting the correctness we currently have, and we want to catch
the next idiot that breaks the code to stop showing all-zero when
talking about the missing side.

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, back to index

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-13 10:44 [PATCH 0/4] Correct offsets of hunks when one is skipped Phillip Wood
2018-02-13 10:44 ` [PATCH 1/4] add -i: add function to format hunk header Phillip Wood
2018-02-13 10:44 ` [PATCH 2/4] t3701: add failing test for pathological context lines Phillip Wood
2018-02-13 20:35   ` Junio C Hamano
2018-02-13 10:44 ` [PATCH 3/4] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
2018-02-13 10:44 ` [PATCH 4/4] add -p: calculate offset delta for edited patches Phillip Wood
2018-02-13 23:56 ` [PATCH 0/4] Correct offsets of hunks when one is skipped brian m. carlson
2018-02-19 13:01   ` Phillip Wood
2018-02-19 11:29 ` [PATCH v2 0/9] " Phillip Wood
2018-02-19 11:29   ` [PATCH v2 1/9] add -i: add function to format hunk header Phillip Wood
2018-02-20 18:21     ` Junio C Hamano
2018-02-19 11:29   ` [PATCH v2 2/9] t3701: indent here documents Phillip Wood
2018-02-19 18:36     ` Eric Sunshine
2018-02-19 11:29   ` [PATCH v2 3/9] t3701: use test_write_lines and write_script Phillip Wood
2018-02-19 18:47     ` Eric Sunshine
2018-02-20 17:19       ` Junio C Hamano
2018-02-21 11:26         ` Phillip Wood
2018-02-19 11:29   ` [PATCH v2 4/9] t3701: don't hard code sha1 hash values Phillip Wood
2018-02-19 18:52     ` Eric Sunshine
2018-02-20 17:39     ` Junio C Hamano
2018-02-21 11:42       ` Phillip Wood
2018-02-21 16:58         ` Junio C Hamano
2018-02-19 11:29   ` [PATCH v2 5/9] t3701: add failing test for pathological context lines Phillip Wood
2018-02-21 11:28     ` Phillip Wood
2018-02-19 11:29   ` [PATCH v2 6/9] add -p: Adjust offsets of subsequent hunks when one is skipped Phillip Wood
2018-02-19 11:29   ` [PATCH v2 7/9] add -p: calculate offset delta for edited patches Phillip Wood
2018-02-19 11:29   ` [PATCH v2 8/9] add -p: fix counting when splitting and coalescing Phillip Wood
2018-02-19 11:29   ` [PATCH v2 9/9] add -p: don't rely on apply's '--recount' option Phillip Wood
2018-02-20 18:50     ` Junio C Hamano

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox