user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH 0/8] www: diff viewing enhancements
@ 2022-08-23  8:31 Eric Wong
  2022-08-23  8:31 ` [PATCH 1/8] view: generate query in single-message and commit views Eric Wong
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:31 UTC (permalink / raw)
  To: meta

1/8 is another step towards making df*: prefixes more visible
and useful to users.  It took me a loong while to arrive at the
current UI + behaviors and I'm still not 100% sure it's ideal or
if it would bloat pages too much...

2/8 is something I noticed while scanning through LKML to ensure
1/8 was working reasonably well.  Another step towards bigger,
and scarier regexps :P  But it's less code!

3/8 is from me having second thoughts on patchids...

4/8 makes navigation easier and something I've wanted for a while

5/8 helps users of giant fonts read long names + email addresses

6/8 because Perl has altered my brain :P

7/8 more use of this will follow in coming days

8/8 I couldn't resist low-hanging fruit

1/8 took longer for me than all the other changes here combined

Eric Wong (8):
  view: generate query in single-message and commit views
  viewdiff: linkify diffstats for non-format-patch emails
  viewvcs: remove patchid line from commit header
  viewvcs: show commit titles for parent commits
  viewvcs: separate email and date with spaces
  ibx_async_cat: access ->{git} directly
  gzip_filter: ->zmore and ->zflush support multiple args
  viewvcs: don't wait on slow disks for git commit titles

 lib/PublicInbox/GitAsyncCat.pm |   2 +-
 lib/PublicInbox/GzipFilter.pm  |  15 ++--
 lib/PublicInbox/View.pm        |  37 ++++++++-
 lib/PublicInbox/ViewDiff.pm    |  45 ++++++-----
 lib/PublicInbox/ViewVCS.pm     | 140 +++++++++++++++++++++++----------
 5 files changed, 167 insertions(+), 72 deletions(-)

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

* [PATCH 1/8] view: generate query in single-message and commit views
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
@ 2022-08-23  8:31 ` Eric Wong
  2022-08-24  1:09   ` Kyle Meyer
  2022-08-23  8:31 ` [PATCH 2/8] viewdiff: linkify diffstats for non-format-patch emails Eric Wong
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:31 UTC (permalink / raw)
  To: meta

The dfblob: search prefix is probably under-utilized, but is
extremely powerful IMHO.  To make it easier-to-use, add a search
textarea with it prefilled with values for the existing patch
message.  This allows users to easily run a query for all
patches which alter or result in either pre or post-image
blobs in the current patch.

Behavior changes are as follows: "changed" in the diffstat
jumps to the bottom of the message.  For /T/ and /t/, it
goes to the "related" anchor which is just above the reply
instructions in the single-message view.  For the single
message view, it'll jump to the textarea search form.

I initially wanted to use a normal `<a href=' link, but
figured the textarea is advantageous for two reasons:

1) users should be able to edit the query before submitting
2) crawlers are less likely to waste CPU/disk on forms

It's probably too noisy to add this directly to the /T/ and /t/
views, but seems like a good place to put above the reply
instructions in the single message view.

Note that the queries used by the /$COMMIT_OID/s/ view is
subtly different than the /$MSGID/ view since git will lengthen
its abbreviations over time, while emails are immutable.

I tried adding dfn: (filename) and s: (subject) support, but
couldn't come up with cases where it really made sense for
/$MSGID/.  /$COMMIT_OID/s/ may benefit from it, since patchid:
could be flaky due to non-standard diff generation options.
---
 lib/PublicInbox/View.pm     | 37 +++++++++++++++++++++++++++++++++----
 lib/PublicInbox/ViewDiff.pm | 22 ++++++++++++++++------
 lib/PublicInbox/ViewVCS.pm  | 28 ++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index c28505f1..6bfaf1bb 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -7,6 +7,7 @@ package PublicInbox::View;
 use strict;
 use v5.10.1;
 use List::Util qw(max);
+use Text::Wrap qw(wrap); # stdlib, we need Perl 5.6+ for $huge
 use PublicInbox::MsgTime qw(msg_datestamp);
 use PublicInbox::Hval qw(ascii_html obfuscate_addrs prurl mid_href
 			ts2str fmt_ts);
@@ -246,6 +247,7 @@ sub eml_entry {
 
 	# scan through all parts, looking for displayable text
 	$ctx->{mhref} = $mhref;
+	$ctx->{end_id} = "e$id";
 	$ctx->{obuf} = \$rv;
 	$eml->each_part(\&add_text_body, $ctx, 1);
 	delete $ctx->{obuf};
@@ -256,6 +258,9 @@ sub eml_entry {
 		" <a\nhref=\"${mhref}raw\">raw</a>" .
 		" <a\nhref=\"${mhref}#R\">reply</a>";
 
+	delete($ctx->{-qry}) and
+		$rv .= qq[ <a\nhref="${mhref}#related">related</a>];
+
 	my $hr;
 	if (defined(my $pct = $smsg->{pct})) { # used by SearchView.pm
 		$rv .= "\t[relevance $pct%]";
@@ -820,15 +825,38 @@ sub _parent_headers {
 # returns a string buffer
 sub html_footer {
 	my ($ctx, $hdr) = @_;
-	my $ibx = $ctx->{ibx};
 	my $upfx = '../';
 	my $skel;
 	my $rv = '<pre>';
-	if ($ibx->over) {
+	my $related;
+	my $qry = delete $ctx->{-qry};
+	if ($qry && $ctx->{ibx}->isrch) {
+		my $q = ''; # search for either ancestor or descendent patches
+		for (@{$qry->{dfpre}}, @{$qry->{dfpost}}) {
+			chop if length > 7; # include 1 abbrev "older" patches
+			$q .= "dfblob:$_ ";
+		}
+		chop $q; # omit trailing SP
+		local $Text::Wrap::columns = COLS;
+		local $Text::Wrap::huge = 'overflow';
+		$q = wrap('', '', $q);
+		my $rows = ($q =~ tr/\n/\n/) + 1;
+		$q = ascii_html($q);
+		$related = <<EOM;
+<form id=related
+action=$upfx
+><pre>find likely ancestor, descendant, or conflicting patches:
+<textarea name=q cols=${\COLS} rows=$rows>$q</textarea>
+<input type=submit value=search
+/>\t(<a href=${upfx}_/text/help/>help</a>)</pre></form>
+EOM
+	}
+	if ($ctx->{ibx}->over) {
 		my $t = ts2str($ctx->{-t_max});
 		my $t_fmt = fmt_ts($ctx->{-t_max});
-		$skel .= <<EOF;
-	other threads:[<a
+		my $fallback = $related ? "\t" : "<a id=related>\t</a>";
+		$skel = <<EOF;
+${fallback}other threads:[<a
 href="$upfx?t=$t">~$t_fmt UTC</a>|<a
 href="$upfx">newest</a>]
 EOF
@@ -869,6 +897,7 @@ EOF
 	$rv .= qq(<a\nhref="#R">reply</a>);
 	$rv .= $skel;
 	$rv .= '</pre>';
+	$rv .= $related // '';
 	$rv .= msg_reply($ctx, $hdr);
 }
 
diff --git a/lib/PublicInbox/ViewDiff.pm b/lib/PublicInbox/ViewDiff.pm
index fb394b7c..960f7e61 100644
--- a/lib/PublicInbox/ViewDiff.pm
+++ b/lib/PublicInbox/ViewDiff.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2019-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 #
 # used by PublicInbox::View
@@ -141,13 +141,16 @@ sub diff_header ($$$) {
 
 	# no need to capture oid_a and oid_b on add/delete,
 	# we just linkify OIDs directly via s///e in conditional
-	if (($$x =~ s/$NULL_TO_BLOB/$1 . oid($dctx, $spfx, $2)/e) ||
-		($$x =~ s/$BLOB_TO_NULL/
-			'index ' . oid($dctx, $spfx, $1) . $2/e)) {
+	if ($$x =~ s/$NULL_TO_BLOB/$1 . oid($dctx, $spfx, $2)/e) {
+		push @{$ctx->{-qry}->{dfpost}}, $2;
+	} elsif ($$x =~ s/$BLOB_TO_NULL/'index '.oid($dctx, $spfx, $1).$2/e) {
+		push @{$ctx->{-qry}->{dfpre}}, $1;
 	} elsif ($$x =~ $BLOB_TO_BLOB) {
 		# modification-only, not add/delete:
 		# linkify hunk headers later using oid_a and oid_b
 		@$dctx{qw(oid_a oid_b)} = ($1, $2);
+		push @{$ctx->{-qry}->{dfpre}}, $1;
+		push @{$ctx->{-qry}->{dfpost}}, $2;
 	} else {
 		warn "BUG? <$$x> had no ^index line";
 	}
@@ -172,9 +175,16 @@ sub diff_before_or_after ($$) {
 			# ok, looks like a diffstat, go line-by-line:
 			for my $l (split(/^/m, $y)) {
 				if ($l =~ /^ (.+)( +\| .*\z)/s) {
-					anchor0($dst, $ctx, $1, $2) and next;
+					anchor0($dst, $ctx, $1, $2) or
+						$$dst .= $linkify->to_html($l);
+				} elsif ($l =~ s/^( [0-9]+ files? )changed,//) {
+					$$dst .= $1;
+					my $end = $ctx->{end_id} // 'related';
+					$$dst .= "<a href=#$end>changed</a>,";
+					$$dst .= ascii_html($l);
+				} else {
+					$$dst .= $linkify->to_html($l);
 				}
-				$$dst .= $linkify->to_html($l);
 			}
 		} else { # commit message, notes, etc
 			$$dst .= $linkify->to_html($y);
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 19d34092..dcb5c67f 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -21,6 +21,8 @@ use PublicInbox::WwwStream qw(html_oneshot);
 use PublicInbox::Linkify;
 use PublicInbox::Tmpfile;
 use PublicInbox::ViewDiff qw(flush_diff);
+use PublicInbox::View;
+use Text::Wrap qw(wrap);
 use PublicInbox::Hval qw(ascii_html to_filename);
 my $hl = eval {
 	require PublicInbox::HlMod;
@@ -157,6 +159,32 @@ EOM
 		$bdy =~ s/\r?\n/\n/gs;
 		flush_diff($ctx, \$bdy);
 		$ctx->zmore($buf);
+		undef $buf;
+		# TODO: should there be another textarea which attempts to
+		# search for the exact email which was applied to make this
+		# commit?
+		if (my $qry = delete $ctx->{-qry}) {
+			my $q = '';
+			for (@{$qry->{dfpost}}, @{$qry->{dfpre}}) {
+				# keep blobs as short as reasonable, emails
+				# are going to be older than what's in git
+				substr($_, 7, 64, '');
+				$q .= "dfblob:$_ ";
+			}
+			chop $q; # no trailing SP
+			local $Text::Wrap::columns = PublicInbox::View::COLS;
+			local $Text::Wrap::huge = 'overflow';
+			$q = wrap('', '', $q);
+			my $rows = ($q =~ tr/\n/\n/) + 1;
+			$q = ascii_html($q);
+			$ctx->zmore(<<EOM);
+<hr><form action=$upfx
+id=related><pre>find related emails, including ancestors/descendants/conflicts
+<textarea name=q cols=${\PublicInbox::View::COLS} rows=$rows>$q</textarea>
+<input type=submit value=search
+/>\t(<a href=${upfx}_/text/help/>help</a>)</pre></form>
+EOM
+		}
 	}
 	$x = $ctx->zflush($ctx->_html_end);
 	my $res_hdr = delete $ctx->{-res_hdr};

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

* [PATCH 2/8] viewdiff: linkify diffstats for non-format-patch emails
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
  2022-08-23  8:31 ` [PATCH 1/8] view: generate query in single-message and commit views Eric Wong
@ 2022-08-23  8:31 ` Eric Wong
  2022-08-23  8:31 ` [PATCH 3/8] viewvcs: remove patchid line from commit header Eric Wong
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:31 UTC (permalink / raw)
  To: meta

Some folks unfortunately use "git diff --stat -p" to generate
patches.  These messages lack the /^---$/ line and causing
diffstats to not get linkified properly.  We now treat the
/^---$/ as optional and rely on the presence of file lines with
/ \| / proceeding a /\d+ files? changed,/ line.
---
 lib/PublicInbox/ViewDiff.pm | 41 +++++++++++++++++--------------------
 lib/PublicInbox/ViewVCS.pm  |  2 +-
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/lib/PublicInbox/ViewDiff.pm b/lib/PublicInbox/ViewDiff.pm
index 960f7e61..ee2d688c 100644
--- a/lib/PublicInbox/ViewDiff.pm
+++ b/lib/PublicInbox/ViewDiff.pm
@@ -166,29 +166,26 @@ sub diff_before_or_after ($$) {
 	my ($ctx, $x) = @_;
 	my $linkify = $ctx->{-linkify};
 	my $dst = $ctx->{obuf};
-	my $anchors = exists($ctx->{-anchors}) ? 1 : 0;
-	for my $y (split(/(^---\n)/sm, $$x)) {
-		if ($y =~ /\A---\n\z/s) {
-			$$dst .= "---\n"; # all HTML is "\r\n" => "\n"
-			$anchors |= 2;
-		} elsif ($anchors == 3 && $y =~ /^ [0-9]+ files? changed, /sm) {
-			# ok, looks like a diffstat, go line-by-line:
-			for my $l (split(/^/m, $y)) {
-				if ($l =~ /^ (.+)( +\| .*\z)/s) {
-					anchor0($dst, $ctx, $1, $2) or
-						$$dst .= $linkify->to_html($l);
-				} elsif ($l =~ s/^( [0-9]+ files? )changed,//) {
-					$$dst .= $1;
-					my $end = $ctx->{end_id} // 'related';
-					$$dst .= "<a href=#$end>changed</a>,";
-					$$dst .= ascii_html($l);
-				} else {
-					$$dst .= $linkify->to_html($l);
-				}
-			}
-		} else { # commit message, notes, etc
-			$$dst .= $linkify->to_html($y);
+	if (exists $ctx->{-anchors} && $$x =~ /\A(.*?) # likely "---\n"
+			# diffstat lines:
+			((?:^\x20(?:[^\n]+?)(?:\x20+\|\x20[^\n]*\n))+)
+			(\x20[0-9]+\x20files?\x20)changed,([^\n]+\n)
+			(.*?)\z/msx) { # notes, commit message, etc
+		undef $$x;
+		my @x = ($5, $4, $3, $2, $1);
+		$$dst .= $linkify->to_html(pop @x); # uninteresting prefix
+		for my $l (split(/^/m, pop(@x))) { # per-file diffstat lines
+			$l =~ /^ (.+)( +\| .*\z)/s and
+				anchor0($dst, $ctx, $1, $2) and next;
+			$$dst .= $linkify->to_html($l);
 		}
+		$$dst .= $x[2]; # $3 /^ \d+ files? /
+		my $end = $ctx->{end_id} // 'related';
+		$$dst .= "<a href=#$end>changed</a>,";
+		$$dst .= ascii_html($x[1]); # $4: insertions/deletions
+		$$dst .= $linkify->to_html($x[0]); # notes, commit message, etc
+	} else {
+		$$dst .= $linkify->to_html($$x);
 	}
 }
 
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index dcb5c67f..4165a1de 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -153,10 +153,10 @@ EOM
 		$buf = '';
 		$ctx->{obuf} = \$buf;
 		$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
-		$ctx->{-anchors} = {};
 		$bdy = '';
 		read($fh, $bdy, -s _);
 		$bdy =~ s/\r?\n/\n/gs;
+		$ctx->{-anchors} = {} if $bdy =~ /^diff --git /sm;
 		flush_diff($ctx, \$bdy);
 		$ctx->zmore($buf);
 		undef $buf;

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

* [PATCH 3/8] viewvcs: remove patchid line from commit header
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
  2022-08-23  8:31 ` [PATCH 1/8] view: generate query in single-message and commit views Eric Wong
  2022-08-23  8:31 ` [PATCH 2/8] viewdiff: linkify diffstats for non-format-patch emails Eric Wong
@ 2022-08-23  8:31 ` Eric Wong
  2022-08-23  8:31 ` [PATCH 4/8] viewvcs: show commit titles for parent commits Eric Wong
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:31 UTC (permalink / raw)
  To: meta

I'm considering dropping this entirely since dfpre:, dfpost:
dfn:, and s: can be just as powerful, if not more.  patchid: is
inaccurate if either non-standard diff generation options are
used (e.g. -W or -U6); or if a MUAs mangle whitespace.

We'll keep patchid: at the top search input box for now, but the
textarea at the bottom (and possibly another textarea for a more
exact match) is probably more useful and flexible.
---
 lib/PublicInbox/ViewVCS.pm | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 4165a1de..a73fbf0f 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -102,12 +102,7 @@ sub show_commit_result ($$) {
 	}
 	my $upfx = $ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
 	my $patchid = (split(/ /, $$bref))[0]; # ignore commit
-	if (defined $patchid) {
-		$ctx->{-q_value_html} = "patchid:$patchid";
-		$patchid = "\n  patchid $patchid";
-	} else {
-		$patchid = '';
-	}
+	$ctx->{-q_value_html} = "patchid:$patchid" if defined $patchid;
 	my $l = $ctx->{-linkify} = PublicInbox::Linkify->new;
 	open my $fh, '<:utf8', "$tmp/h" or die "open $tmp/h: $!";
 	chop(my $buf = do { local $/ = "\0"; <$fh> });
@@ -141,7 +136,7 @@ sub show_commit_result ($$) {
 <pre>   commit $H$P
      tree <a href="$upfx$T/s/">$T</a>
    author $au
-committer $co$patchid
+committer $co
 
 <b>$s</b>\n
 EOM

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

* [PATCH 4/8] viewvcs: show commit titles for parent commits
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
                   ` (2 preceding siblings ...)
  2022-08-23  8:31 ` [PATCH 3/8] viewvcs: remove patchid line from commit header Eric Wong
@ 2022-08-23  8:31 ` Eric Wong
  2022-08-23  8:32 ` [PATCH 5/8] viewvcs: separate email and date with spaces Eric Wong
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:31 UTC (permalink / raw)
  To: meta

It's hard to know which commit I'm following a link to without
having the title of the commit handy.
---
 lib/PublicInbox/ViewVCS.pm | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index a73fbf0f..2c447909 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -32,7 +32,7 @@ my $hl = eval {
 my %QP_MAP = ( A => 'oid_a', a => 'path_a', b => 'path_b' );
 our $MAX_SIZE = 1024 * 1024; # TODO: configurable
 my $BIN_DETECT = 8000; # same as git
-my $SHOW_FMT = '--pretty=format:'.join('%n', '%H', '%T', '%P', '%s',
+my $SHOW_FMT = '--pretty=format:'.join('%n', '%H', '%T', '%P', '%p', '%s',
 	'%an <%ae>%x09%ai', '%cn <%ce>%x09%ci', '%b%x00');
 
 sub html_page ($$$) {
@@ -93,6 +93,17 @@ sub show_other_result ($$) {
 	html_page($ctx, 200, $bref);
 }
 
+sub cmt_title { # git->cat_async callback
+	my ($bref, $oid, $type, $size, $pt) = @_;
+	utf8::decode($$bref);
+	if ($$bref =~ /\r?\n\r?\n([^\r\n]+)\r?\n?/) {
+		push @$pt, $1;
+		ascii_html($pt->[-1]);
+	} else {
+		push @$pt, ''; # need a placeholder if blank commit
+	}
+}
+
 sub show_commit_result ($$) {
 	my ($bref, $ctx) = @_;
 	my ($qsp_err, $logref, $tmp) = @$ctx{qw(-qsp_err -logref -tmp)};
@@ -106,7 +117,7 @@ sub show_commit_result ($$) {
 	my $l = $ctx->{-linkify} = PublicInbox::Linkify->new;
 	open my $fh, '<:utf8', "$tmp/h" or die "open $tmp/h: $!";
 	chop(my $buf = do { local $/ = "\0"; <$fh> });
-	my ($H, $T, $P, $s, $au, $co, $bdy) = split(/\n/, $buf, 7);
+	my ($H, $T, $P, $p, $s, $au, $co, $bdy) = split(/\n/, $buf, 8);
 	chomp $bdy;
 	# try to keep author and committer dates lined up
 	my $x = length($au) - length($co);
@@ -120,13 +131,19 @@ sub show_commit_result ($$) {
 	$_ = ascii_html($_) for ($au, $co);
 	$_ = $l->to_html($_) for ($s, $bdy);
 	$ctx->{-title_html} = $s;
-	my @p = split(/ /, $P);
-	if (@p == 1) {
-		$P = qq(\n   parent <a href="$upfx$P/s/">$P</a>);
-	} elsif (@p > 1) {
-		$P = qq(\n  parents <a href="$upfx$p[0]/s/">$p[0]</a>\n);
-		shift @p;
-		$P .= qq(          <a href="$upfx$_/s/">$_</a>\n) for @p;
+	my @P = split(/ /, $P);
+	my @p = split(/ /, $p); # abbreviated
+	my @pt;
+	my $git = delete $ctx->{code_git};
+	$git->cat_async($_, \&cmt_title, \@pt) for @P;
+	$git->cat_async_wait;
+	$_ = qq(<a href="$upfx$_/s/">).shift(@p).'</a> '.shift(@pt) for @P;
+	if (@P == 1) {
+		$P = qq(\n   parent $P[0]);
+	} elsif (@P > 1) {
+		$P = qq(\n  parents $P[0]\n);
+		shift @P;
+		$P .= qq(          $_\n) for @P;
 		chop $P;
 	} else { # root commit
 		$P = ' (root commit)';
@@ -206,6 +223,7 @@ sub show_commit ($$$$) {
 	$ctx->{-logref} = $logref;
 	$ctx->{-tmp} = $tmp;
 	$ctx->{env}->{'qspawn.wcb'} = delete $ctx->{-wcb};
+	$ctx->{code_git} = $git;
 	$qsp->psgi_qx($ctx->{env}, undef, \&show_commit_result, $ctx);
 }
 

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

* [PATCH 5/8] viewvcs: separate email and date with spaces
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
                   ` (3 preceding siblings ...)
  2022-08-23  8:31 ` [PATCH 4/8] viewvcs: show commit titles for parent commits Eric Wong
@ 2022-08-23  8:32 ` Eric Wong
  2022-08-23  8:32 ` [PATCH 6/8] ibx_async_cat: access ->{git} directly Eric Wong
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:32 UTC (permalink / raw)
  To: meta

Horizontal tabs can be too much for narrow displays, so just add
an extra space to keep the date separate from the email address.
---
 lib/PublicInbox/ViewVCS.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 2c447909..4533af3a 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -33,7 +33,7 @@ my %QP_MAP = ( A => 'oid_a', a => 'path_a', b => 'path_b' );
 our $MAX_SIZE = 1024 * 1024; # TODO: configurable
 my $BIN_DETECT = 8000; # same as git
 my $SHOW_FMT = '--pretty=format:'.join('%n', '%H', '%T', '%P', '%p', '%s',
-	'%an <%ae>%x09%ai', '%cn <%ce>%x09%ci', '%b%x00');
+	'%an <%ae>  %ai', '%cn <%ce>  %ci', '%b%x00');
 
 sub html_page ($$$) {
 	my ($ctx, $code, $strref) = @_;
@@ -123,10 +123,10 @@ sub show_commit_result ($$) {
 	my $x = length($au) - length($co);
 	if ($x > 0) {
 		$x = ' ' x $x;
-		$co =~ s/\t/$x\t/;
+		$co =~ s/>/>$x/;
 	} elsif ($x < 0) {
 		$x = ' ' x (-$x);
-		$au =~ s/\t/$x\t/;
+		$au =~ s/>/>$x/;
 	}
 	$_ = ascii_html($_) for ($au, $co);
 	$_ = $l->to_html($_) for ($s, $bdy);

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

* [PATCH 6/8] ibx_async_cat: access ->{git} directly
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
                   ` (4 preceding siblings ...)
  2022-08-23  8:32 ` [PATCH 5/8] viewvcs: separate email and date with spaces Eric Wong
@ 2022-08-23  8:32 ` Eric Wong
  2022-08-23  8:32 ` [PATCH 7/8] gzip_filter: ->zmore and ->zflush support multiple args Eric Wong
  2022-08-23  8:32 ` [PATCH 8/8] viewvcs: don't wait on slow disks for git commit titles Eric Wong
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:32 UTC (permalink / raw)
  To: meta

This will enable callers to pass non-Inbox-ish hashrefs as the
arg.  This benefits existing Inbox-ish objects, too, as it
avoids a slow method dispatch for both ExtSearch and Inbox.
---
 lib/PublicInbox/GitAsyncCat.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/PublicInbox/GitAsyncCat.pm b/lib/PublicInbox/GitAsyncCat.pm
index 6b7425f6..c12c4ec2 100644
--- a/lib/PublicInbox/GitAsyncCat.pm
+++ b/lib/PublicInbox/GitAsyncCat.pm
@@ -47,7 +47,7 @@ sub event_step {
 
 sub ibx_async_cat ($$$$) {
 	my ($ibx, $oid, $cb, $arg) = @_;
-	my $git = $ibx->git;
+	my $git = $ibx->{git} // $ibx->git;
 	# {topdir} means ExtSearch (likely [extindex "all"]) with potentially
 	# 100K alternates.  git(1) has a proposed patch for 100K alternates:
 	# <https://lore.kernel.org/git/20210624005806.12079-1-e@80x24.org/>

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

* [PATCH 7/8] gzip_filter: ->zmore and ->zflush support multiple args
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
                   ` (5 preceding siblings ...)
  2022-08-23  8:32 ` [PATCH 6/8] ibx_async_cat: access ->{git} directly Eric Wong
@ 2022-08-23  8:32 ` Eric Wong
  2022-08-23  8:32 ` [PATCH 8/8] viewvcs: don't wait on slow disks for git commit titles Eric Wong
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:32 UTC (permalink / raw)
  To: meta

This will make writev-like use easier for the next commit,
and also future changes where I'll rely more on zlib for
buffering.
---
 lib/PublicInbox/GzipFilter.pm | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/GzipFilter.pm b/lib/PublicInbox/GzipFilter.pm
index bdd313f5..86d34183 100644
--- a/lib/PublicInbox/GzipFilter.pm
+++ b/lib/PublicInbox/GzipFilter.pm
@@ -139,19 +139,22 @@ sub write {
 sub zmore {
 	my $self = $_[0]; # $_[1] => input
 	http_out($self);
-	my $err = $self->{gz}->deflate($_[1], $self->{zbuf});
-	die "gzip->deflate: $err" if $err != Z_OK;
+	my $err;
+	for (1..$#_) {
+		$err = $self->{gz}->deflate($_[$_], $self->{zbuf});
+		die "gzip->deflate: $err" if $err != Z_OK;
+	}
 	undef;
 }
 
 # flushes and returns the final bit of gzipped data
-sub zflush ($;$) {
-	my $self = $_[0]; # $_[1] => final input (optional)
+sub zflush ($;@) {
+	my $self = $_[0]; # $_[1..Inf] => final input (optional)
 	my $zbuf = delete $self->{zbuf};
 	my $gz = delete $self->{gz};
 	my $err;
-	if (defined $_[1]) { # it's a bug iff $gz is undef w/ $_[1]
-		$err = $gz->deflate($_[1], $zbuf);
+	for (1..$#_) { # it's a bug iff $gz is undef w/ $_[1..]
+		$err = $gz->deflate($_[$_], $zbuf);
 		die "gzip->deflate: $err" if $err != Z_OK;
 	}
 	$gz // return ''; # not a bug, recursing on DS->write failure

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

* [PATCH 8/8] viewvcs: don't wait on slow disks for git commit titles
  2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
                   ` (6 preceding siblings ...)
  2022-08-23  8:32 ` [PATCH 7/8] gzip_filter: ->zmore and ->zflush support multiple args Eric Wong
@ 2022-08-23  8:32 ` Eric Wong
  7 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2022-08-23  8:32 UTC (permalink / raw)
  To: meta

We can easily wire into the DS event loop to take advantage
of epoll/kevent notifications and serve other clients in
the mean time.
---
 lib/PublicInbox/ViewVCS.pm | 107 +++++++++++++++++++++----------------
 1 file changed, 61 insertions(+), 46 deletions(-)

diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 4533af3a..9bf010c2 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -17,6 +17,7 @@ use strict;
 use v5.10.1;
 use File::Temp 0.19 (); # newdir
 use PublicInbox::SolverGit;
+use PublicInbox::GitAsyncCat;
 use PublicInbox::WwwStream qw(html_oneshot);
 use PublicInbox::Linkify;
 use PublicInbox::Tmpfile;
@@ -32,7 +33,7 @@ my $hl = eval {
 my %QP_MAP = ( A => 'oid_a', a => 'path_a', b => 'path_b' );
 our $MAX_SIZE = 1024 * 1024; # TODO: configurable
 my $BIN_DETECT = 8000; # same as git
-my $SHOW_FMT = '--pretty=format:'.join('%n', '%H', '%T', '%P', '%p', '%s',
+my $SHOW_FMT = '--pretty=format:'.join('%n', '%P', '%p', '%H', '%T', '%s',
 	'%an <%ae>  %ai', '%cn <%ce>  %ci', '%b%x00');
 
 sub html_page ($$$) {
@@ -94,32 +95,49 @@ sub show_other_result ($$) {
 }
 
 sub cmt_title { # git->cat_async callback
-	my ($bref, $oid, $type, $size, $pt) = @_;
+	my ($bref, $oid, $type, $size, $ctx) = @_;
 	utf8::decode($$bref);
-	if ($$bref =~ /\r?\n\r?\n([^\r\n]+)\r?\n?/) {
-		push @$pt, $1;
-		ascii_html($pt->[-1]);
-	} else {
-		push @$pt, ''; # need a placeholder if blank commit
-	}
+	my $title = $$bref =~ /\r?\n\r?\n([^\r\n]+)\r?\n?/ ? $1 : '';
+	push(@{$ctx->{-cmt_pt}} , ascii_html($title)) == @{$ctx->{-cmt_P}} and
+		cmt_finalize($ctx);
 }
 
-sub show_commit_result ($$) {
+sub show_commit_start { # ->psgi_qx callback
 	my ($bref, $ctx) = @_;
-	my ($qsp_err, $logref, $tmp) = @$ctx{qw(-qsp_err -logref -tmp)};
+	my ($qsp_err, $logref) = delete @$ctx{qw(-qsp_err -logref)};
 	if ($qsp_err) {
 		$$logref .= "git show/patch-id error:$qsp_err";
 		return html_page($ctx, 500, $logref);
 	}
-	my $upfx = $ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
 	my $patchid = (split(/ /, $$bref))[0]; # ignore commit
 	$ctx->{-q_value_html} = "patchid:$patchid" if defined $patchid;
-	my $l = $ctx->{-linkify} = PublicInbox::Linkify->new;
-	open my $fh, '<:utf8', "$tmp/h" or die "open $tmp/h: $!";
+	open my $fh, '<:utf8', "$ctx->{-tmp}/h" or
+		die "open $ctx->{-tmp}/h: $!";
 	chop(my $buf = do { local $/ = "\0"; <$fh> });
-	my ($H, $T, $P, $p, $s, $au, $co, $bdy) = split(/\n/, $buf, 8);
-	chomp $bdy;
+	chomp $buf;
+	my ($P, $p);
+	($P, $p, @$ctx{qw(cmt_H cmt_T cmt_s cmt_au cmt_co cmt_b)})
+		= split(/\n/, $buf, 8);
+	return cmt_finalize($ctx) if !$P;
+	@{$ctx->{-cmt_P}} = split(/ /, $P);
+	@{$ctx->{-cmt_p}} = split(/ /, $p); # abbreviated
+	if ($ctx->{env}->{'pi-httpd.async'}) {
+		for (@{$ctx->{-cmt_P}}) {
+			ibx_async_cat($ctx, $_, \&cmt_title, $ctx);
+		}
+	} else { # synchronous
+		for (@{$ctx->{-cmt_P}}) {
+			$ctx->{git}->cat_async($_, \&cmt_title, $ctx);
+		}
+		$ctx->{git}->cat_async_wait;
+	}
+}
+
+sub cmt_finalize {
+	my ($ctx) = @_;
+	$ctx->{-linkify} = PublicInbox::Linkify->new;
 	# try to keep author and committer dates lined up
+	my ($au, $co) = delete @$ctx{qw(cmt_au cmt_co)};
 	my $x = length($au) - length($co);
 	if ($x > 0) {
 		$x = ' ' x $x;
@@ -129,49 +147,46 @@ sub show_commit_result ($$) {
 		$au =~ s/>/>$x/;
 	}
 	$_ = ascii_html($_) for ($au, $co);
-	$_ = $l->to_html($_) for ($s, $bdy);
+	my $s = $ctx->{-linkify}->to_html(delete $ctx->{cmt_s});
 	$ctx->{-title_html} = $s;
-	my @P = split(/ /, $P);
-	my @p = split(/ /, $p); # abbreviated
-	my @pt;
-	my $git = delete $ctx->{code_git};
-	$git->cat_async($_, \&cmt_title, \@pt) for @P;
-	$git->cat_async_wait;
-	$_ = qq(<a href="$upfx$_/s/">).shift(@p).'</a> '.shift(@pt) for @P;
-	if (@P == 1) {
-		$P = qq(\n   parent $P[0]);
-	} elsif (@P > 1) {
-		$P = qq(\n  parents $P[0]\n);
-		shift @P;
-		$P .= qq(          $_\n) for @P;
-		chop $P;
-	} else { # root commit
-		$P = ' (root commit)';
+	my $upfx = $ctx->{-upfx} = '../../'; # from "/$INBOX/$OID/s/"
+	my ($P, $p, $pt) = delete @$ctx{qw(-cmt_P -cmt_p -cmt_pt)};
+	$_ = qq(<a href="$upfx$_/s/">).shift(@$p).'</a> '.shift(@$pt) for @$P;
+	if (@$P == 1) {
+		$x = qq(\n   parent $P->[0]);
+	} elsif (@$P > 1) {
+		$x = qq(\n  parents $P->[0]\n);
+		shift @$P;
+		$x .= qq(          $_\n) for @$P;
+		chop $x;
+	} else {
+		$x = ' (root commit)';
 	}
 	PublicInbox::WwwStream::html_init($ctx);
 	$ctx->zmore(<<EOM);
-<pre>   commit $H$P
-     tree <a href="$upfx$T/s/">$T</a>
+<pre>   commit $ctx->{cmt_H}$x
+     tree <a href="$upfx$ctx->{cmt_T}/s/">$ctx->{cmt_T}</a>
    author $au
 committer $co
 
-<b>$s</b>\n
+<b>$s</b>
 EOM
-	$ctx->zmore($bdy);
-	open $fh, '<:utf8', "$tmp/p" or die "open $tmp/p: $!";
+	$x = delete $ctx->{cmt_b};
+	$ctx->zmore("\n", $ctx->{-linkify}->to_html($x)) if length($x);
+	undef $x;
+	open my $fh, '<:utf8', "$ctx->{-tmp}/p" or
+		die "open $ctx->{-tmp}/p: $!";
 	if (-s $fh > $MAX_SIZE) {
 		$ctx->zmore("---\n patch is too large to show\n");
 	} else { # prepare flush_diff:
-		$buf = '';
-		$ctx->{obuf} = \$buf;
+		$ctx->{obuf} = \$x;
 		$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
-		$bdy = '';
-		read($fh, $bdy, -s _);
+		read($fh, my $bdy, -s _);
 		$bdy =~ s/\r?\n/\n/gs;
 		$ctx->{-anchors} = {} if $bdy =~ /^diff --git /sm;
-		flush_diff($ctx, \$bdy);
-		$ctx->zmore($buf);
-		undef $buf;
+		flush_diff($ctx, \$bdy); # undefs $bdy
+		$ctx->zmore($x);
+		undef $x;
 		# TODO: should there be another textarea which attempts to
 		# search for the exact email which was applied to make this
 		# commit?
@@ -223,8 +238,8 @@ sub show_commit ($$$$) {
 	$ctx->{-logref} = $logref;
 	$ctx->{-tmp} = $tmp;
 	$ctx->{env}->{'qspawn.wcb'} = delete $ctx->{-wcb};
-	$ctx->{code_git} = $git;
-	$qsp->psgi_qx($ctx->{env}, undef, \&show_commit_result, $ctx);
+	$ctx->{git} = $git;
+	$qsp->psgi_qx($ctx->{env}, undef, \&show_commit_start, $ctx);
 }
 
 sub show_other ($$$$) {

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

* Re: [PATCH 1/8] view: generate query in single-message and commit views
  2022-08-23  8:31 ` [PATCH 1/8] view: generate query in single-message and commit views Eric Wong
@ 2022-08-24  1:09   ` Kyle Meyer
  0 siblings, 0 replies; 10+ messages in thread
From: Kyle Meyer @ 2022-08-24  1:09 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

Eric Wong writes:

> Behavior changes are as follows: "changed" in the diffstat
> jumps to the bottom of the message.  For /T/ and /t/, it
> goes to the "related" anchor which is just above the reply
> instructions in the single-message view.  For the single
> message view, it'll jump to the textarea search form.

This is a very nice addition.  Thank you!

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

end of thread, other threads:[~2022-08-24  1:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23  8:31 [PATCH 0/8] www: diff viewing enhancements Eric Wong
2022-08-23  8:31 ` [PATCH 1/8] view: generate query in single-message and commit views Eric Wong
2022-08-24  1:09   ` Kyle Meyer
2022-08-23  8:31 ` [PATCH 2/8] viewdiff: linkify diffstats for non-format-patch emails Eric Wong
2022-08-23  8:31 ` [PATCH 3/8] viewvcs: remove patchid line from commit header Eric Wong
2022-08-23  8:31 ` [PATCH 4/8] viewvcs: show commit titles for parent commits Eric Wong
2022-08-23  8:32 ` [PATCH 5/8] viewvcs: separate email and date with spaces Eric Wong
2022-08-23  8:32 ` [PATCH 6/8] ibx_async_cat: access ->{git} directly Eric Wong
2022-08-23  8:32 ` [PATCH 7/8] gzip_filter: ->zmore and ->zflush support multiple args Eric Wong
2022-08-23  8:32 ` [PATCH 8/8] viewvcs: don't wait on slow disks for git commit titles Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).