* [PATCH 1/2] www_listing: show ->ALL at top of HTML listing
2021-08-27 12:08 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
@ 2021-08-27 12:08 ` Eric Wong
2021-08-27 12:08 ` [PATCH 2/2] www_listing: fix odd "locate inbox" cases Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2021-08-27 12:08 UTC (permalink / raw)
To: meta
It's a special case and we can show it in the HTML display
without affecting manifest.js.gz generation.
---
lib/PublicInbox/WwwListing.pm | 45 ++++++++++++++++++++++-------------
t/extindex-psgi.t | 6 +++++
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index 8b54d724..eabda98a 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -7,22 +7,29 @@ package PublicInbox::WwwListing;
use strict;
use v5.10.1;
use PublicInbox::Hval qw(prurl fmt_ts ascii_html);
-use PublicInbox::Linkify;
use PublicInbox::GzipFilter qw(gzf_maybe);
use PublicInbox::ConfigIter;
use PublicInbox::WwwStream;
+use URI::Escape qw(uri_escape_utf8);
sub ibx_entry {
my ($ctx, $ibx, $ce) = @_;
- $ce->{description} //= $ibx->description;
+ my $desc = ascii_html($ce->{description} //= $ibx->description);
my $ts = fmt_ts($ce->{-modified} //= $ibx->modified);
- my $url = prurl($ctx->{env}, $ibx->{url});
- my $tmp = <<"";
-* $ts - $url
- $ce->{description}
-
- if (defined(my $info_url = $ibx->{infourl})) {
- $tmp .= ' ' . prurl($ctx->{env}, $info_url) . "\n";
+ my ($url, $href);
+ if (defined($ibx->{url})) {
+ $url = $href = ascii_html(prurl($ctx->{env}, $ibx->{url}));
+ } else {
+ $href = ascii_html(uri_escape_utf8($ibx->{name})) . '/';
+ $url = ascii_html($ibx->{name});
+ }
+ my $tmp = <<EOM;
+* $ts - <a\nhref="$href">$url</a>
+ $desc
+EOM
+ if (defined($url = $ibx->{infourl})) {
+ $url = ascii_html(prurl($ctx->{env}, $url));
+ $tmp .= qq( <a\nhref="$url">$url</a>\n);
}
push(@{$ctx->{-list}}, (scalar(@_) == 3 ? # $misc in use, already sorted
$tmp : [ $ce->{-modified}, $tmp ] ));
@@ -86,19 +93,24 @@ sub add_misc_ibx { # MiscSearch->retry_reopen callback
limit => $q->{l}
};
$qs .= ' type:inbox';
- if (my $user_query = $q->{'q'}) {
+
+ delete $ctx->{-list}; # reset if retried
+ my $pi_cfg = $ctx->{www}->{pi_cfg};
+ if (defined(my $user_query = $q->{'q'})) {
$qs = "( $qs ) AND ( $user_query )";
+ } else { # special case for ALL
+ $ctx->ibx_entry($pi_cfg->ALL // die('BUG: ->ALL expected'), {});
}
my $mset = $misc->mset($qs, $opt); # sorts by $MODIFIED (mtime)
- delete $ctx->{-list}; # reset if retried
- my $pi_cfg = $ctx->{www}->{pi_cfg};
+ my $hide_key = $ctx->hide_key;
+
for my $mi ($mset->items) {
my $doc = $mi->get_document;
my ($eidx_key) = PublicInbox::Search::xap_terms('Q', $doc);
$eidx_key // next;
my $ibx = $pi_cfg->lookup_eidx_key($eidx_key) // next;
- next if $ibx->{-hide}->{$ctx->hide_key};
- grep(/$re/, @{$ibx->{url}}) or next;
+ next if $ibx->{-hide}->{$hide_key};
+ grep(/$re/, @{$ibx->{url} // []}) or next;
$ctx->ibx_entry($ibx, $misc->doc2ibx_cache_ent($doc));
if ($r) { # for descriptions in search_nav_bot
my $pct = PublicInbox::Search::get_pct($mi);
@@ -203,9 +215,8 @@ sub psgi_triple {
@$list = map { $_->[1] }
sort { $b->[0] <=> $a->[0] } @$list;
}
- $list = join("\n", @$list);
- my $l = PublicInbox::Linkify->new;
- $gzf->zmore('<pre>'.$l->to_html($list));
+ $gzf->zmore('<pre>');
+ $gzf->zmore(join("\n", @$list));
$gzf->zmore(mset_footer($ctx, $mset)) if $mset;
} else {
$gzf->zmore('<pre>no inboxes, yet');
diff --git a/t/extindex-psgi.t b/t/extindex-psgi.t
index d4761641..31b04acd 100644
--- a/t/extindex-psgi.t
+++ b/t/extindex-psgi.t
@@ -28,6 +28,8 @@ run_script([qw(-extindex --all), "$tmpdir/eidx"], $env) or BAIL_OUT;
[extindex "all"]
topdir = $tmpdir/eidx
url = http://bogus.example.com/all
+[publicinbox]
+ wwwlisting = all
EOM
}
my $www = PublicInbox::WWW->new(PublicInbox::Config->new($pi_config));
@@ -55,6 +57,10 @@ my $client = sub {
$res = $cb->(GET('/all/all.mbox.gz'));
is($res->code, 200, 'all.mbox.gz');
+
+ $res = $cb->(GET('/'));
+ my $html = $res->content;
+ like($html, qr!\Qhttp://bogus.example.com/all\E!, 'html shows /all');
};
test_psgi(sub { $www->call(@_) }, $client);
%$env = (%$env, TMPDIR => $tmpdir, PI_CONFIG => $pi_config);
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/2] www_listing: fix odd "locate inbox" cases
2021-08-27 12:08 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
2021-08-27 12:08 ` [PATCH 1/2] www_listing: show ->ALL at top of HTML listing Eric Wong
@ 2021-08-27 12:08 ` Eric Wong
2021-08-27 13:03 ` [PATCH 0/2] wwwlisting shows /all/ Konstantin Ryabitsev
2021-09-26 1:30 ` [PATCH] www_listing: support /all/ search as a 302 redirect Eric Wong
3 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2021-08-27 12:08 UTC (permalink / raw)
To: meta
Searching inboxes with an empty query no longer gives 500 errors
due to Xapian. Also, improve the error message when no inboxes
match, since saying no inboxes exist yet is wrong.
---
lib/PublicInbox/WwwListing.pm | 7 ++++++-
t/extindex-psgi.t | 10 ++++++++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index eabda98a..1bb5fbd0 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -96,7 +96,8 @@ sub add_misc_ibx { # MiscSearch->retry_reopen callback
delete $ctx->{-list}; # reset if retried
my $pi_cfg = $ctx->{www}->{pi_cfg};
- if (defined(my $user_query = $q->{'q'})) {
+ my $user_query = $q->{'q'} // '';
+ if ($user_query =~ /\S/) {
$qs = "( $qs ) AND ( $user_query )";
} else { # special case for ALL
$ctx->ibx_entry($pi_cfg->ALL // die('BUG: ->ALL expected'), {});
@@ -218,6 +219,10 @@ sub psgi_triple {
$gzf->zmore('<pre>');
$gzf->zmore(join("\n", @$list));
$gzf->zmore(mset_footer($ctx, $mset)) if $mset;
+ } elsif (my $mset = delete $ctx->{-mset}) {
+ $gzf->zmore(mset_nav_top($ctx, $mset));
+ $gzf->zmore('<pre>no matching inboxes');
+ $gzf->zmore(mset_footer($ctx, $mset));
} else {
$gzf->zmore('<pre>no inboxes, yet');
}
diff --git a/t/extindex-psgi.t b/t/extindex-psgi.t
index 31b04acd..4e26962e 100644
--- a/t/extindex-psgi.t
+++ b/t/extindex-psgi.t
@@ -59,8 +59,14 @@ my $client = sub {
is($res->code, 200, 'all.mbox.gz');
$res = $cb->(GET('/'));
- my $html = $res->content;
- like($html, qr!\Qhttp://bogus.example.com/all\E!, 'html shows /all');
+ like($res->content, qr!\Qhttp://bogus.example.com/all\E!,
+ '/all listed');
+ $res = $cb->(GET('/?q='));
+ is($res->code, 200, 'no query means all inboxes');
+ $res = $cb->(GET('/?q=nonexistent'));
+ is($res->code, 404, 'no inboxes matched');
+ unlike($res->content, qr!no inboxes, yet!,
+ 'we have inboxes, just no matches');
};
test_psgi(sub { $www->call(@_) }, $client);
%$env = (%$env, TMPDIR => $tmpdir, PI_CONFIG => $pi_config);
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/2] wwwlisting shows /all/
2021-08-27 12:08 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
2021-08-27 12:08 ` [PATCH 1/2] www_listing: show ->ALL at top of HTML listing Eric Wong
2021-08-27 12:08 ` [PATCH 2/2] www_listing: fix odd "locate inbox" cases Eric Wong
@ 2021-08-27 13:03 ` Konstantin Ryabitsev
2021-08-27 14:33 ` Konstantin Ryabitsev
2021-08-27 21:15 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
2021-09-26 1:30 ` [PATCH] www_listing: support /all/ search as a 302 redirect Eric Wong
3 siblings, 2 replies; 10+ messages in thread
From: Konstantin Ryabitsev @ 2021-08-27 13:03 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
On Fri, Aug 27, 2021 at 12:08:43PM +0000, Eric Wong wrote:
> I think that's too much vertical whitespace at the top of the
> page, and multiple <form>s or <input> boxes at the top can get
> confusing.
>
> Just making /all/ show up at the top like a normal inbox (and
> letting the admin decide on description) seems sufficient. If
> users can get to /all/ then they can search /all/ as normal.
Sure, this works for me, too, thanks!
https://x-lore.kernel.org/
One thing I noticed is that the "description" value for inbox entries isn't
coming from what is on disk in inboxname/description files but from
manifest.js entries.
E.g. if you look at "lkml" you will notice that it has "LKML Archive on
lore.kernel.org" as description on the wwwlisting page, but if you click on
it, the description is "linux-kernel.vger.kernel.org archive mirror". In
theory, they shouldn't be different, but a mirror admin may choose to give
their own descriptions to mirrored lists.
Best regards,
-K
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] wwwlisting shows /all/
2021-08-27 13:03 ` [PATCH 0/2] wwwlisting shows /all/ Konstantin Ryabitsev
@ 2021-08-27 14:33 ` Konstantin Ryabitsev
2021-08-27 22:03 ` [PATCH] www: avoid potential auto-vivification on ibx->{url} Eric Wong
2021-08-27 21:15 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
1 sibling, 1 reply; 10+ messages in thread
From: Konstantin Ryabitsev @ 2021-08-27 14:33 UTC (permalink / raw)
To: Eric Wong; +Cc: meta
On Fri, 27 Aug 2021 at 09:03, Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
> > Just making /all/ show up at the top like a normal inbox (and
> > letting the admin decide on description) seems sufficient. If
> > users can get to /all/ then they can search /all/ as normal.
>
> Sure, this works for me, too, thanks!
> https://x-lore.kernel.org/
Hmm.. I just noticed that the "all" link has vanished. The source is showing:
<pre>* 2021-08-27 13:24 - <a
href=""></a>
All of lore.kernel.org
Restarting public-inbox-httpd fixes it, but I wonder if it will happen again.
-K
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] www: avoid potential auto-vivification on ibx->{url}
2021-08-27 14:33 ` Konstantin Ryabitsev
@ 2021-08-27 22:03 ` Eric Wong
2021-08-28 18:00 ` Konstantin Ryabitsev
0 siblings, 1 reply; 10+ messages in thread
From: Eric Wong @ 2021-08-27 22:03 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Fri, 27 Aug 2021 at 09:03, Konstantin Ryabitsev
> <konstantin@linuxfoundation.org> wrote:
> > > Just making /all/ show up at the top like a normal inbox (and
> > > letting the admin decide on description) seems sufficient. If
> > > users can get to /all/ then they can search /all/ as normal.
> >
> > Sure, this works for me, too, thanks!
> > https://x-lore.kernel.org/
>
> Hmm.. I just noticed that the "all" link has vanished. The source is showing:
>
> <pre>* 2021-08-27 13:24 - <a
> href=""></a>
> All of lore.kernel.org
>
> Restarting public-inbox-httpd fixes it, but I wonder if it will happen again.
If it happens once, it'll happen again. I'm not sure which code
path triggers it, but maybe this will fix it. There's also a secondary
patch below which may be more complete, but may also be
unneeded.
--------------8<-------------
Subject: [PATCH] www: avoid potential auto-vivification on ibx->{url}
This may fix problems with the "all" link disappearing.
Link: https://public-inbox.org/meta/CAMwyc-Tw=v5yT1U1U66GSwwTK8OJXv8_YDu-=oXbZO3tHSnYWw@mail.gmail.com/
---
lib/PublicInbox/Inbox.pm | 3 ++-
lib/PublicInbox/WwwListing.pm | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index df140cac..b0bb9dcc 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -250,7 +250,8 @@ sub base_url {
}
# called from a non-PSGI environment (e.g. NNTP/POP3):
$self->{-base_url} ||= do {
- my $url = $self->{url}->[0] or return undef;
+ my $url = $self->{url} // return undef;
+ $url = $url->[0] // return undef;
# expand protocol-relative URLs to HTTPS if we're
# not inside a web server
$url = "https:$url" if $url =~ m!\A//!;
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index 1bb5fbd0..fe5ee087 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -41,7 +41,7 @@ sub list_match_i { # ConfigIter callback
return if $section !~ m!\Apublicinbox\.([^/]+)\z!;
my $ibx = $cfg->lookup_name($1) or return;
if (!$ibx->{-hide}->{$ctx->hide_key} &&
- grep(/$re/, @{$ibx->{url}})) {
+ grep(/$re/, @{$ibx->{url} // []})) {
$ctx->ibx_entry($ibx);
}
} else { # undef == "EOF"
--------8<------
Secondary patch in case extindex $ibx->{url} got vivified somewhere:
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index fe5ee087..ef9048b5 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -17,7 +17,7 @@ sub ibx_entry {
my $desc = ascii_html($ce->{description} //= $ibx->description);
my $ts = fmt_ts($ce->{-modified} //= $ibx->modified);
my ($url, $href);
- if (defined($ibx->{url})) {
+ if (scalar(@{$ibx->{url} // []})) {
$url = $href = ascii_html(prurl($ctx->{env}, $ibx->{url}));
} else {
$href = ascii_html(uri_escape_utf8($ibx->{name})) . '/';
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] wwwlisting shows /all/
2021-08-27 13:03 ` [PATCH 0/2] wwwlisting shows /all/ Konstantin Ryabitsev
2021-08-27 14:33 ` Konstantin Ryabitsev
@ 2021-08-27 21:15 ` Eric Wong
1 sibling, 0 replies; 10+ messages in thread
From: Eric Wong @ 2021-08-27 21:15 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: meta
Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Fri, Aug 27, 2021 at 12:08:43PM +0000, Eric Wong wrote:
> > I think that's too much vertical whitespace at the top of the
> > page, and multiple <form>s or <input> boxes at the top can get
> > confusing.
> >
> > Just making /all/ show up at the top like a normal inbox (and
> > letting the admin decide on description) seems sufficient. If
> > users can get to /all/ then they can search /all/ as normal.
>
> Sure, this works for me, too, thanks!
> https://x-lore.kernel.org/
>
> One thing I noticed is that the "description" value for inbox entries isn't
> coming from what is on disk in inboxname/description files but from
> manifest.js entries.
It's actually coming from $EXTINDEX_DIR/misc which is written
during -extindex invocations.
> E.g. if you look at "lkml" you will notice that it has "LKML Archive on
> lore.kernel.org" as description on the wwwlisting page, but if you click on
> it, the description is "linux-kernel.vger.kernel.org archive mirror". In
> theory, they shouldn't be different, but a mirror admin may choose to give
> their own descriptions to mirrored lists.
There might be a bug in -extindex invocations, since I'm trying
to avoid opening per-inbox description files in WWW/IMAP/NNTP
with thousands of inboxes on the horizon.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] www_listing: support /all/ search as a 302 redirect
2021-08-27 12:08 ` [PATCH 0/2] wwwlisting shows /all/ Eric Wong
` (2 preceding siblings ...)
2021-08-27 13:03 ` [PATCH 0/2] wwwlisting shows /all/ Konstantin Ryabitsev
@ 2021-09-26 1:30 ` Eric Wong
3 siblings, 0 replies; 10+ messages in thread
From: Eric Wong @ 2021-09-26 1:30 UTC (permalink / raw)
To: meta; +Cc: Konstantin Ryabitsev
On Fri, 27 Aug 2021 12:08:43 +0000, Eric Wong wrote:
> Konstantin Ryabitsev wrote:
> > I switched the configuration to return wwwlisting as toplevel view (instead of
> > redirecting / to /all/), but there's some discontent, because the easy
> > interface to "search everything" is gone and unless someone knows about /all/,
> > they wouldn't find out about it from the wwwlisting view.
> >
> > How about, if there is an extindex "all" defined, the wwwlisting adds an extra
> > search box:
> >
> > [_________________] [search all] help
> >
> > * 2021-08-26 16:54 - https://x-lore.kernel.org/all/
> > All of lore.kernel.org
> >
> > ------------------------------------------------------
> >
> > [_________________] [locate inbox]
> >
>
> I think that's too much vertical whitespace at the top of the
> page, and multiple <form>s or <input> boxes at the top can get
> confusing.
>
> Just making /all/ show up at the top like a normal inbox (and
> letting the admin decide on description) seems sufficient. If
> users can get to /all/ then they can search /all/ as normal.
>
> I tried moving the <input>s next to infourl, but mixing <form>
> and <pre> introduces a lot of vertical whitespace.
>
>
> I also tried adding radio button (or drop-down):
>
> [_________________] [*] inboxes [ ] /all/ [search]
>
> But extra <form> elements always confuse me whenever I encounter
> them in any browser, so I decided to just leave things alone.
-------------8<------------
Subject: [PATCH] www_listing: support /all/ search as a 302 redirect
This allows users to search /all/ from the top-level WwwListing
without extra manual steps, although there's still extra network
roundtrips incurred.
No vertical whitespace is added, and there's no clumsy radio
buttons nor menus to deal with. Users only have to use a
different <input type=submit /> button. I forgot how to do this
until I realized we already do something similar with multiple
submit buttons for threaded vs non-threaded mboxrd.gz downloads.
Link: https://public-inbox.org/meta/20210827120845.29682-1-e@80x24.org/
---
lib/PublicInbox/WwwListing.pm | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/PublicInbox/WwwListing.pm b/lib/PublicInbox/WwwListing.pm
index a9290802..79c0a8ec 100644
--- a/lib/PublicInbox/WwwListing.pm
+++ b/lib/PublicInbox/WwwListing.pm
@@ -11,6 +11,7 @@ use PublicInbox::GzipFilter qw(gzf_maybe);
use PublicInbox::ConfigIter;
use PublicInbox::WwwStream;
use URI::Escape qw(uri_escape_utf8);
+use PublicInbox::MID qw(mid_escape);
sub ibx_entry {
my ($ctx, $ibx, $ce) = @_;
@@ -135,6 +136,13 @@ sub response {
my ($re, $qs) = $ctx->url_filter;
$re // return $ctx->psgi_triple;
if (my $ALL = $ctx->{www}->{pi_cfg}->ALL) { # fast path
+ if ($ctx->{qp}->{a} && # "search all inboxes"
+ $ctx->{qp}->{'q'}) {
+ my $u = 'all/?q='.mid_escape($ctx->{qp}->{'q'});
+ return [ 302, [ 'Location' => $u,
+ qw(Content-Type text/plain) ],
+ [ "Redirecting to $u\n" ] ];
+ }
# FIXME: test this in t/
$ALL->misc->reopen->retry_reopen(\&add_misc_ibx,
$ctx, $re, $qs);
@@ -166,11 +174,10 @@ sub mset_nav_top {
$qh = qq[\nvalue="$qh"] if $qh ne '';
my $rv = <<EOM;
<form
-action="./"><pre><input
-name=q
-type=text$qh /><input
-type=submit
-value="locate inbox" /></pre></form><pre>
+action="./"><pre><input name=q type=text$qh
+/><input type=submit value="locate inbox"
+/><input type=submit name=a value="search all inboxes"
+/></pre></form><pre>
EOM
chomp $rv;
if (defined($q->{'q'})) {
^ permalink raw reply related [flat|nested] 10+ messages in thread