[PATCH] www_coderepo: tree: do not break #n$LINENO

[PATCH] www_coderepo: tree: do not break #n$LINENO

[Eric Wong]

We can't use 302 redirects at the /tree/ endpoint as originally
intended since "#n$LINENO" fragment links aren't preserved
across redirects (since clients don't typically send that part
of the URL in requests).

So we'll have to make sure we handle prefixes properly and show
trees directly.  Oh well :<  At least the history-aware 404
handling remains :>
---
 lib/PublicInbox/RepoTree.pm    | 23 +++++++++++++---------
 lib/PublicInbox/ViewVCS.pm     | 36 ++++++++++++++++++++++------------
 lib/PublicInbox/WwwCoderepo.pm |  4 ++--
 t/solver_git.t                 | 14 ++++++++++---
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/lib/PublicInbox/RepoTree.pm b/lib/PublicInbox/RepoTree.pm
index cec71eb6..84e20589 100644
--- a/lib/PublicInbox/RepoTree.pm
+++ b/lib/PublicInbox/RepoTree.pm
@@ -48,15 +48,20 @@ sub find_missing {
 	$qsp->psgi_qx($ctx->{env}, undef, \&rd_404_log, $ctx);
 }
 
-sub tree_30x { # git check_async callback
+sub tree_show { # git check_async callback
 	my ($oid, $type, $size, $ctx) = @_;
 	return find_missing($ctx) if $type eq 'missing';
-	my $wcb = delete $ctx->{-wcb};
-	my $u = $ctx->{git}->base_url($ctx->{env});
-	my $path = uri_escape_path(delete $ctx->{-path});
-	$u .= "$oid/s/?b=$path";
-	$wcb->([ 302, [ Location => $u, 'Content-Type' => 'text/plain' ],
-		[ "Redirecting to $u\n" ] ])
+
+	open $ctx->{lh}, '<', \(my $dbg_log = '') or die "open(scalar): $!";
+	my $res = [ $ctx->{git}, $oid, $type, $size ];
+	my ($bn) = ($ctx->{-path} =~ m!/?([^/]+)\z!);
+	if ($type eq 'blob') {
+		my $obj = ascii_html($ctx->{-obj});
+		$ctx->{-paths} = [ $bn, qq[(<a
+href="$ctx->{-upfx}$oid/s/$bn">raw</a>)
+\$ git show $obj\t# shows this blob on the CLI] ];
+	}
+	PublicInbox::ViewVCS::solve_result($res, $ctx);
 }
 
 sub srv_tree {
@@ -74,9 +79,9 @@ sub srv_tree {
 	sub {
 		$ctx->{-wcb} = $_[0]; # HTTP::{Chunked,Identity}
 		if ($ctx->{env}->{'pi-httpd.async'}) {
-			async_check($ctx, $obj, \&tree_30x, $ctx);
+			async_check($ctx, $obj, \&tree_show, $ctx);
 		} else {
-			$ctx->{git}->check_async($obj, \&tree_30x, $ctx);
+			$ctx->{git}->check_async($obj, \&tree_show, $ctx);
 			$ctx->{git}->async_wait_all;
 		}
 	};
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 7ac719bc..e0fdf639 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -49,7 +49,7 @@ my %GIT_MODE = (
 sub html_page ($$;@) {
 	my ($ctx, $code) = @_[0, 1];
 	my $wcb = delete $ctx->{-wcb};
-	$ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
+	$ctx->{-upfx} //= '../../'; # from "/$INBOX/$OID/s/"
 	my $res = html_oneshot($ctx, $code, @_[2..$#_]);
 	$wcb ? $wcb->($res) : $res;
 }
@@ -65,6 +65,7 @@ sub dbg_log ($) {
 		warn "readline(log): $!";
 		return '<pre>debug log read error</pre>';
 	};
+	return '' if $log eq '';
 	$ctx->{-linkify} //= PublicInbox::Linkify->new;
 	"<hr><pre>debug log:\n\n".
 		$ctx->{-linkify}->to_html($log).'</pre>';
@@ -369,10 +370,18 @@ sub show_tree_result ($$) {
 	my @ent = split(/\0/, $$bref);
 	my $qp = delete $ctx->{qp};
 	my $l = $ctx->{-linkify} //= PublicInbox::Linkify->new;
-	my $pfx = $qp->{b};
+	my $pfx = $ctx->{-path} // $qp->{b}; # {-path} is from RepoTree
 	$$bref = "<pre><a href=#tree>tree</a> $ctx->{tree_oid}";
+	# $REPO/tree/$path already sets {-upfx}
+	my $upfx = $ctx->{-upfx} //= '../../';
 	if (defined $pfx) {
-		if ($pfx eq '') {
+		$pfx =~ s!/+\z!!s;
+		if (my $t = $ctx->{-obj}) {
+			my $t = ascii_html($t);
+			$$bref .= <<EOM
+\n\$ git ls-tree -l $t	# shows similar output on the CLI
+EOM
+		} elsif ($pfx eq '') {
 			$$bref .= "  (root)\n";
 		} else {
 			my $x = ascii_html($pfx);
@@ -400,7 +409,7 @@ sub show_tree_result ($$) {
 		if ($m eq 'd') { $n .= '/' }
 		elsif ($m eq 'x') { $n = "<b>$n</b>" }
 		elsif ($m eq 'l') { $n = "<i>$n</i>" }
-		$$bref .= qq(\n$m\t$sz\t<a\nhref="../../$oid/s/?$q">$n</a>);
+		$$bref .= qq(\n$m\t$sz\t<a\nhref="$upfx$oid/s/?$q">$n</a>);
 	}
 	$$bref .= dbg_log($ctx);
 	$$bref .= <<EOM;
@@ -423,7 +432,7 @@ EOM
 	html_page($ctx, 200, $$bref);
 }
 
-sub show_tree ($$) {
+sub show_tree ($$) { # also used by RepoTree
 	my ($ctx, $res) = @_;
 	my ($git, $oid, undef, $size) = @$res;
 	$size > $MAX_SIZE and return html_page($ctx, 200,
@@ -484,16 +493,19 @@ sub solve_result {
 	return show_tree($ctx, $res) if $type eq 'tree';
 	return show_tag($ctx, $res) if $type eq 'tag';
 	return show_other($ctx, $res) if $type ne 'blob';
-	my $path = to_filename($di->{path_b} // $hints->{path_b} // 'blob');
-	my $raw_link = "(<a\nhref=$path>raw</a>)";
+	my $paths = $ctx->{-paths} //= do {
+		my $path = to_filename($di->{path_b}//$hints->{path_b}//'blob');
+		my $raw_more = qq[(<a\nhref="$path">raw</a>)];
+		[ $path, $raw_more ];
+	};
+
 	if ($size > $MAX_SIZE) {
 		return stream_large_blob($ctx, $res) if defined $ctx->{fn};
 		return html_page($ctx, 200, <<EOM . dbg_log($ctx));
 <pre><b>Too big to show, download available</b>
-blob $oid $size bytes $raw_link</pre>
+blob $oid $size bytes $paths->[1]</pre>
 EOM
 	}
-	@{$ctx->{-paths}} = ($path, $raw_link);
 	bless $ctx, 'PublicInbox::WwwStream'; # for DESTROY
 	$ctx->{git} = $git;
 	if ($ctx->{env}->{'pi-httpd.async'}) {
@@ -519,10 +531,10 @@ sub show_blob { # git->cat_async callback
 		return delete($ctx->{-wcb})->([200, $h, [ $$blob ]]);
 	}
 
-	my ($path, $raw_link) = @{delete $ctx->{-paths}};
+	my ($path, $raw_more) = @{delete $ctx->{-paths}};
 	$bin and return html_page($ctx, 200,
 				"<pre>blob $oid $size bytes (binary)" .
-				" $raw_link</pre>".dbg_log($ctx));
+				" $raw_more</pre>".dbg_log($ctx));
 
 	# TODO: detect + convert to ensure validity
 	utf8::decode($$blob);
@@ -538,7 +550,7 @@ sub show_blob { # git->cat_async callback
 	}
 
 	# using some of the same CSS class names and ids as cgit
-	my $x = "<pre>blob $oid $size bytes $raw_link</pre>" .
+	my $x = "<pre>blob $oid $size bytes $raw_more</pre>" .
 		"<hr /><table\nclass=blob>".
 		"<tr><td\nclass=linenumbers><pre>";
 	# scratchpad in this loop is faster here than `printf $zfh':
diff --git a/lib/PublicInbox/WwwCoderepo.pm b/lib/PublicInbox/WwwCoderepo.pm
index 668b6398..f9c150c0 100644
--- a/lib/PublicInbox/WwwCoderepo.pm
+++ b/lib/PublicInbox/WwwCoderepo.pm
@@ -223,9 +223,9 @@ sub srv { # endpoint called by PublicInbox::WWW
 	}
 	$path_info =~ m!\A/(.+?)/\z! and
 		($ctx->{git} = $cr->{$1}) and return summary($self, $ctx);
-	$path_info =~ m!\A/(.+?)/([a-f0-9]+)/s/\z! and
+	$path_info =~ m!\A/(.+?)/([a-f0-9]+)/s/([^/]+)?\z! and
 			($ctx->{git} = $cr->{$1}) and
-		return PublicInbox::ViewVCS::show($ctx, $2);
+		return PublicInbox::ViewVCS::show($ctx, $2, $3);
 
 	if ($path_info =~ m!\A/(.+?)/tree/(.*)\z! and
 			($ctx->{git} = $cr->{$1})) {
diff --git a/t/solver_git.t b/t/solver_git.t
index 5519fa18..8faa7309 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -383,11 +383,19 @@ EOF
 		}
 
 		$res = $cb->(GET('/public-inbox/tree/'));
-		is($res->code, 302, 'got redirect');
+		is($res->code, 200, 'got 200 for root listing');
+		$got = $res->content;
+		like($got, qr/\bgit ls-tree\b/, 'ls-tree help shown');
+
 		$res = $cb->(GET('/public-inbox/tree/README'));
-		is($res->code, 302, 'got redirect for regular file');
+		is($res->code, 200, 'got 200 for regular file');
+		$got = $res->content;
+		like($got, qr/\bgit show\b/, 'git show help shown');
+
 		$res = $cb->(GET('/public-inbox/tree/Documentation'));
-		is($res->code, 302, 'got redirect for directory');
+		is($res->code, 200, 'got 200 for a directory');
+		$got = $res->content;
+		like($got, qr/\bgit ls-tree\b/, 'ls-tree help shown');
 	};
 	test_psgi(sub { $www->call(@_) }, $client);
 	my $env = { PI_CONFIG => $cfgpath, TMPDIR => $tmpdir };