I think the "lei q --save" and "lei up" commands are in a good state. There's no way to edit/forget saved searches, yet, so I guess that's next... Eric Wong (6): lei: support unlinked/missing saved searches lei q: implement import-before default for --save lei_saved_search: avoid needless var shadowing config: git_config_dump blesses lei_saved_search: split "lei q --save" and "lei up" init paths lei q: --save and --augment may be combined lib/PublicInbox/Config.pm | 10 ++-- lib/PublicInbox/LEI.pm | 3 +- lib/PublicInbox/LeiLsSearch.pm | 2 +- lib/PublicInbox/LeiMirror.pm | 2 +- lib/PublicInbox/LeiSavedSearch.pm | 94 +++++++++++++++---------------- lib/PublicInbox/LeiToMail.pm | 6 +- lib/PublicInbox/LeiUp.pm | 4 +- t/lei-q-save.t | 41 ++++++++++++++ 8 files changed, 101 insertions(+), 61 deletions(-)
It's conceivable a user will want to erase all previous results but still rerun/refresh a search to get new results. We probably won't support prune functionality, here, and instead require explicit removal of saved searches. --- lib/PublicInbox/LeiSavedSearch.pm | 7 ++----- t/lei-q-save.t | 7 +++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index d67622c9..94920a4e 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -61,11 +61,8 @@ sub list { bless $cfg, 'PublicInbox::Config'; my $out = $cfg->get_all('lei.q.output') or return (); map {; - if (s!\A(?:maildir|mh|mbox.+|mmdf):!!i) { - -e $_ ? $_ : (); # TODO auto-prune somewhere? - } else { # IMAP, maybe JMAP - $_; - } + s!\A(?:maildir|mh|mbox.+|mmdf):!!i; + $_; } @$out } diff --git a/t/lei-q-save.t b/t/lei-q-save.t index 761814b4..4e6ed642 100644 --- a/t/lei-q-save.t +++ b/t/lei-q-save.t @@ -67,5 +67,12 @@ test_lei(sub { lei_ok qw(_complete lei up); like($lei_out, qr!^\Q$home/mbcl2\E$!sm, 'complete got mbcl2 output'); like($lei_out, qr!^\Q$home/md/\E$!sm, 'complete got maildir output'); + + unlink("$home/mbcl2") or xbail "unlink $!"; + lei_ok qw(_complete lei up); + like($lei_out, qr!^\Q$home/mbcl2\E$!sm, + 'mbcl2 output shown despite unlink'); + lei_ok([qw(up mbcl2)], undef, { -C => $home, %$lei_opt }); + ok(-f "$home/mbcl2" && -s _ == 0, 'up recreates on missing output'); }); done_testing;
This makes "lei q --save" as safe as "lei q" to prevent against accidental data loss when clobbering an existing output, --- lib/PublicInbox/LeiToMail.pm | 6 +++--- lib/PublicInbox/LeiUp.pm | 2 +- t/lei-q-save.t | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm index daa8084b..46a82a4b 100644 --- a/lib/PublicInbox/LeiToMail.pm +++ b/lib/PublicInbox/LeiToMail.pm @@ -375,7 +375,7 @@ sub _pre_augment_maildir { sub _do_augment_maildir { my ($self, $lei) = @_; - return if defined($lei->{opt}->{save}); + return if ($lei->{opt}->{save} // 0) < 0; my $dst = $lei->{ovv}->{dst}; my $lse = $lei->{opt}->{'import-before'} ? $lei->{lse} : undef; my $mdr = PublicInbox::MdirReader->new; @@ -406,7 +406,7 @@ sub _imap_augment_or_delete { # PublicInbox::NetReader::imap_each cb sub _do_augment_imap { my ($self, $lei) = @_; - return if defined($lei->{opt}->{save}); + return if ($lei->{opt}->{save} // 0) < 0; my $net = $lei->{net}; my $lse = $lei->{opt}->{'import-before'} ? $lei->{lse} : undef; if ($lei->{opt}->{augment}) { @@ -477,7 +477,7 @@ sub _do_augment_mbox { my ($self, $lei) = @_; return unless $self->{seekable}; my $opt = $lei->{opt}; - return if defined($opt->{save}); + return if ($opt->{save} // 0) < 0; my $out = $lei->{1}; my ($fmt, $dst) = @{$lei->{ovv}}{qw(fmt dst)}; return unless -s $out; diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm index 73286ea2..e4cbc825 100644 --- a/lib/PublicInbox/LeiUp.pm +++ b/lib/PublicInbox/LeiUp.pm @@ -38,7 +38,7 @@ sub lei_up { $lei->{lss} = $lss; # for LeiOverview->new my $lxs = $lei->lxs_prepare or return; $lei->ale->refresh_externals($lxs); - $lei->{opt}->{save} = 1; + $lei->{opt}->{save} = -1; $lei->_start_query; } diff --git a/t/lei-q-save.t b/t/lei-q-save.t index 4e6ed642..5bc8fb74 100644 --- a/t/lei-q-save.t +++ b/t/lei-q-save.t @@ -10,6 +10,15 @@ $doc2->header_set('Date', PublicInbox::Smsg::date({ds => time - (86400 * 4)})); my $doc3 = eml_load('t/msg_iter-order.eml'); $doc3->header_set('Date', PublicInbox::Smsg::date({ds => time - (86400 * 4)})); +my $pre_existing = <<'EOF'; +From x Mon Sep 17 00:00:00 2001 +Message-ID: <import-before@example.com> +Subject: pre-existing +Date: Sat, 02 Oct 2010 00:00:00 +0000 + +blah +EOF + test_lei(sub { my $home = $ENV{HOME}; my $in = $doc1->as_string; @@ -74,5 +83,17 @@ test_lei(sub { 'mbcl2 output shown despite unlink'); lei_ok([qw(up mbcl2)], undef, { -C => $home, %$lei_opt }); ok(-f "$home/mbcl2" && -s _ == 0, 'up recreates on missing output'); + + open my $mb, '>', "$home/mbrd" or xbail "open $!"; + print $mb $pre_existing; + close $mb or xbail "close: $!"; + lei_ok(qw(q --save -o mboxrd:mbrd m:qp@example.com -C), $home); + chdir($dh) or xbail "fchdir . $!"; + open $mb, '<', "$home/mbrd" or xbail "open $!"; + is_deeply([grep(/pre-existing/, <$mb>)], [], + 'pre-existing messsage gone w/o augment'); + lei_ok(qw(q m:import-before@example.com)); + is(json_utf8->decode($lei_out)->[0]->{'s'}, + 'pre-existing', '--save imported before clobbering'); }); done_testing;
While perl (5.28) doesn't complain about this, it's confusing to my easily-confused mind. --- lib/PublicInbox/LeiSavedSearch.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index 94920a4e..19f4aa5f 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -79,7 +79,7 @@ sub new { $self->{-cfg} //= PublicInbox::Config::git_config_dump($f); $self->{'-f'} = $f; } else { # new saved search "lei q --save" - my $dst = $lei->{ovv}->{dst}; + $dst = $lei->{ovv}->{dst}; $dir = lss_dir_for($lei, \$dst); require File::Path; File::Path::make_path($dir); # raises on error
I don't know if it's worth it to sub (or super)class PublicInbox::Config into something more generic for lei, but this change simplifies a good chunk of lei code that reuses the public-inbox config parsing. --- lib/PublicInbox/Config.pm | 10 +++++----- lib/PublicInbox/LEI.pm | 3 +-- lib/PublicInbox/LeiMirror.pm | 2 +- lib/PublicInbox/LeiSavedSearch.pm | 7 ++----- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm index 603dad98..016f50ec 100644 --- a/lib/PublicInbox/Config.pm +++ b/lib/PublicInbox/Config.pm @@ -24,11 +24,11 @@ sub new { if (ref($file) eq 'SCALAR') { # used by some tests open my $fh, '<', $file or die; # PerlIO::scalar $self = config_fh_parse($fh, "\n", '='); + bless $self, $class; } else { - $self = git_config_dump($file); + $self = git_config_dump($class, $file); $self->{'-f'} = $file; } - bless $self, $class; # caches $self->{-by_addr} = {}; $self->{-by_list_id} = {}; @@ -158,13 +158,13 @@ sub config_fh_parse ($$$) { } sub git_config_dump { - my ($file) = @_; - return {} unless -e $file; + my ($class, $file) = @_; + return bless {}, $class unless -e $file; my $cmd = [ qw(git config -z -l --includes), "--file=$file" ]; my $fh = popen_rd($cmd); my $rv = config_fh_parse($fh, "\0", "\n"); close $fh or die "failed to close (@$cmd) pipe: $?"; - $rv; + bless $rv, $class; } sub valid_foo_name ($;$) { diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 56640be1..f641f0d9 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -700,8 +700,7 @@ sub _lei_cfg ($;$) { $cur_st = pack('dd', $st[10], $st[7]); qerr($self, "# $f created") if $self->{cmd} ne 'config'; } - my $cfg = PublicInbox::Config::git_config_dump($f); - bless $cfg, 'PublicInbox::Config'; + my $cfg = PublicInbox::Config->git_config_dump($f); $cfg->{-st} = $cur_st; $cfg->{'-f'} = $f; if ($sto && File::Spec->canonpath($sto_dir // store_path($self)) diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm index 89574d28..15adb71b 100644 --- a/lib/PublicInbox/LeiMirror.pm +++ b/lib/PublicInbox/LeiMirror.pm @@ -110,7 +110,7 @@ sub _try_config { } return $lei->err("# @$cmd failed (non-fatal)") if $cerr; rename($f, $ce) or return $lei->err("link($f, $ce): $! (non-fatal)"); - my $cfg = PublicInbox::Config::git_config_dump($f); + my $cfg = PublicInbox::Config->git_config_dump($f); my $ibx = $self->{ibx} = {}; for my $sec (grep(/\Apublicinbox\./, @{$cfg->{-section_order}})) { for (qw(address newsgroup nntpmirror)) { diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index 19f4aa5f..d3a32d36 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -56,9 +56,8 @@ sub list { print $fh "\tpath = ", cquote_val($p), "\n"; } close $fh or die "close $f: $!"; - my $cfg = PublicInbox::Config::git_config_dump($f); + my $cfg = PublicInbox::Config->git_config_dump($f); unlink($f); - bless $cfg, 'PublicInbox::Config'; my $out = $cfg->get_all('lei.q.output') or return (); map {; s!\A(?:maildir|mh|mbox.+|mmdf):!!i; @@ -76,7 +75,6 @@ sub new { output2lssdir($self, $lei, \$dir, \$f) or return $lei->fail("--save was not used with $dst cwd=". $lei->rel2abs('.')); - $self->{-cfg} //= PublicInbox::Config::git_config_dump($f); $self->{'-f'} = $f; } else { # new saved search "lei q --save" $dst = $lei->{ovv}->{dst}; @@ -113,7 +111,6 @@ EOM } close($fh) or return $lei->fail("close $f: $!"); } - bless $self->{-cfg}, 'PublicInbox::Config'; $self->{lock_path} = "$self->{-f}.flock"; $self->{-ovf} = "$dir/over.sqlite3"; $self; @@ -198,7 +195,7 @@ sub output2lssdir { my $dir = lss_dir_for($lei, \$dst); my $f = "$dir/lei.saved-search"; if (-f $f && -r _) { - $self->{-cfg} = PublicInbox::Config::git_config_dump($f); + $self->{-cfg} = PublicInbox::Config->git_config_dump($f); $$dir_ref = $dir; $$fn_ref = $f; return 1;
They're more different than alike, and having two separate methods seems less confusing to me. --- lib/PublicInbox/LeiLsSearch.pm | 2 +- lib/PublicInbox/LeiSavedSearch.pm | 79 ++++++++++++++++--------------- lib/PublicInbox/LeiUp.pm | 2 +- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/lib/PublicInbox/LeiLsSearch.pm b/lib/PublicInbox/LeiLsSearch.pm index 2aa457c0..9ac4870f 100644 --- a/lib/PublicInbox/LeiLsSearch.pm +++ b/lib/PublicInbox/LeiLsSearch.pm @@ -29,7 +29,7 @@ sub do_ls_search_long { my @x = sort(grep(/\A\Q$pfx/, PublicInbox::LeiSavedSearch::list($lei))); while (my $x = shift @x) { $ORS = '' if !scalar(@x); - my $lss = PublicInbox::LeiSavedSearch->new($lei, $x) or next; + my $lss = PublicInbox::LeiSavedSearch->up($lei, $x) or next; my $cfg = $lss->{-cfg}; my $ent = { q => $cfg->get_all('lei.q'), diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index d3a32d36..948e4954 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -65,52 +65,57 @@ sub list { } @$out } -sub new { +sub up { # updating existing saved search via "lei up" my ($cls, $lei, $dst) = @_; + my $f; my $self = bless { ale => $lei->ale }, $cls; - my $dir; - if (defined $dst) { # updating existing saved search via "lei up" - my $f; - $dir = $dst; - output2lssdir($self, $lei, \$dir, \$f) or - return $lei->fail("--save was not used with $dst cwd=". - $lei->rel2abs('.')); - $self->{'-f'} = $f; - } else { # new saved search "lei q --save" - $dst = $lei->{ovv}->{dst}; - $dir = lss_dir_for($lei, \$dst); - require File::Path; - File::Path::make_path($dir); # raises on error - $self->{-cfg} = {}; - my $f = $self->{'-f'} = "$dir/lei.saved-search"; - open my $fh, '>', $f or return $lei->fail("open $f: $!"); - my $sq_dst = PublicInbox::Config::squote_maybe($dst); - my $q = $lei->{mset_opt}->{q_raw} // die 'BUG: {q_raw} missing'; - if (ref $q) { - $q = join("\n", map { "\tq = ".cquote_val($_) } @$q); - } else { - $q = "\tq = ".cquote_val($q); - } - $dst = "$lei->{ovv}->{fmt}:$dst" if $dst !~ m!\Aimaps?://!i; - print $fh <<EOM; + my $dir = $dst; + output2lssdir($self, $lei, \$dir, \$f) or + return $lei->fail("--save was not used with $dst cwd=". + $lei->rel2abs('.')); + $self->{-cfg} = PublicInbox::Config->git_config_dump($f); + $self->{-ovf} = "$dir/over.sqlite3"; + $self->{'-f'} = $f; + $self->{lock_path} = "$self->{-f}.flock"; + $self; +} + +sub new { # new saved search "lei q --save" + my ($cls, $lei) = @_; + my $self = bless { ale => $lei->ale }, $cls; + my $dst = $lei->{ovv}->{dst}; + my $dir = lss_dir_for($lei, \$dst); + require File::Path; + File::Path::make_path($dir); # raises on error + $self->{-cfg} = {}; + my $f = $self->{'-f'} = "$dir/lei.saved-search"; + open my $fh, '>', $f or return $lei->fail("open $f: $!"); + my $sq_dst = PublicInbox::Config::squote_maybe($dst); + my $q = $lei->{mset_opt}->{q_raw} // die 'BUG: {q_raw} missing'; + if (ref $q) { + $q = join("\n", map { "\tq = ".cquote_val($_) } @$q); + } else { + $q = "\tq = ".cquote_val($q); + } + $dst = "$lei->{ovv}->{fmt}:$dst" if $dst !~ m!\Aimaps?://!i; + print $fh <<EOM; ; to refresh with new results, run: lei up $sq_dst [lei] -$q + $q [lei "q"] output = $dst EOM - for my $k (ARRAY_FIELDS) { - my $ary = $lei->{opt}->{$k} // next; - for my $x (@$ary) { - print $fh "\t$k = ".cquote_val($x)."\n"; - } - } - for my $k (BOOL_FIELDS) { - my $val = $lei->{opt}->{$k} // next; - print $fh "\t$k = ".($val ? 1 : 0)."\n"; + for my $k (ARRAY_FIELDS) { + my $ary = $lei->{opt}->{$k} // next; + for my $x (@$ary) { + print $fh "\t$k = ".cquote_val($x)."\n"; } - close($fh) or return $lei->fail("close $f: $!"); } + for my $k (BOOL_FIELDS) { + my $val = $lei->{opt}->{$k} // next; + print $fh "\t$k = ".($val ? 1 : 0)."\n"; + } + close($fh) or return $lei->fail("close $f: $!"); $self->{lock_path} = "$self->{-f}.flock"; $self->{-ovf} = "$dir/over.sqlite3"; $self; diff --git a/lib/PublicInbox/LeiUp.pm b/lib/PublicInbox/LeiUp.pm index e4cbc825..23c5c606 100644 --- a/lib/PublicInbox/LeiUp.pm +++ b/lib/PublicInbox/LeiUp.pm @@ -11,7 +11,7 @@ use PublicInbox::LeiOverview; sub lei_up { my ($lei, $out) = @_; $lei->{lse} = $lei->_lei_store(1)->search; - my $lss = PublicInbox::LeiSavedSearch->new($lei, $out) or return; + my $lss = PublicInbox::LeiSavedSearch->up($lei, $out) or return; my $mset_opt = $lei->{mset_opt} = { relevance => -2 }; $mset_opt->{limit} = $lei->{opt}->{limit} // 10000; my $q = $mset_opt->{q_raw} = $lss->{-cfg}->{'lei.q'} //
This necessitated fixing pause_dedupe to release the handle used by ->lock_for_scope_fast, but otherwise no changes to the LeiToMail package. --- lib/PublicInbox/LeiSavedSearch.pm | 1 + t/lei-q-save.t | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index 948e4954..cd9effce 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -182,6 +182,7 @@ sub git { $_[0]->{ale}->git } sub pause_dedupe { my ($self) = @_; $self->{ale}->git->cleanup; + my $lockfh = delete $self->{lockfh}; # from lock_for_scope_fast; my $oidx = delete($self->{oidx}) // return; $oidx->commit_lazy; } diff --git a/t/lei-q-save.t b/t/lei-q-save.t index 5bc8fb74..c0c74581 100644 --- a/t/lei-q-save.t +++ b/t/lei-q-save.t @@ -84,6 +84,7 @@ test_lei(sub { lei_ok([qw(up mbcl2)], undef, { -C => $home, %$lei_opt }); ok(-f "$home/mbcl2" && -s _ == 0, 'up recreates on missing output'); + # no --augment open my $mb, '>', "$home/mbrd" or xbail "open $!"; print $mb $pre_existing; close $mb or xbail "close: $!"; @@ -92,8 +93,20 @@ test_lei(sub { open $mb, '<', "$home/mbrd" or xbail "open $!"; is_deeply([grep(/pre-existing/, <$mb>)], [], 'pre-existing messsage gone w/o augment'); + close $mb; lei_ok(qw(q m:import-before@example.com)); is(json_utf8->decode($lei_out)->[0]->{'s'}, 'pre-existing', '--save imported before clobbering'); + + # --augment + open $mb, '>', "$home/mbrd-aug" or xbail "open $!"; + print $mb $pre_existing; + close $mb or xbail "close: $!"; + lei_ok(qw(q -a --save -o mboxrd:mbrd-aug m:qp@example.com -C), $home); + chdir($dh) or xbail "fchdir . $!"; + open $mb, '<', "$home/mbrd-aug" or xbail "open $!"; + $mb = do { local $/; <$mb> }; + like($mb, qr/pre-existing/, 'pre-existing message preserved w/ -a'); + like($mb, qr/<qp\@example\.com>/, 'new result written w/ -a'); }); done_testing;