In the process of trying to catch up with lei changes (so that I can send the manpage update that I still haven't finished), I noticed that `lei ls-mail-sync' didn't accept a filter despite looking as though it was supposed to. That's fixed in the third patch, with the other patches addressing a few more things I spotted along the way. [1/5] lei ls-mail-sync: update reference to ls-sync [2/5] lei ls-mail-sync: drop repeated -z/0 option [3/5] lei ls-mail-sync: accept a filter [4/5] lei ls-mail-sync: fix handling of non-wildcard filters [5/5] lei: add help output for --invert match lib/PublicInbox/LEI.pm | 5 +++-- lib/PublicInbox/LeiLsMailSync.pm | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) base-commit: 0b15dfc58ceaecdcb1c9285c3ad55813006c8338 -- 2.31.1
ls-sync was renamed to ls-mail-sync in cb0e9d42b799c748. Update a stale reference to the old name. --- lib/PublicInbox/LeiLsMailSync.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiLsMailSync.pm b/lib/PublicInbox/LeiLsMailSync.pm index 2b3d326d..532ea9b5 100644 --- a/lib/PublicInbox/LeiLsMailSync.pm +++ b/lib/PublicInbox/LeiLsMailSync.pm @@ -1,7 +1,7 @@ # Copyright (C) 2021 all contributors <meta@public-inbox.org> # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt> -# front-end for the "lei ls-sync" sub-command +# front-end for the "lei ls-mail-sync" sub-command package PublicInbox::LeiLsMailSync; use strict; use v5.10.1; -- 2.31.1
--- lib/PublicInbox/LEI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 599cfab2..e9c1675a 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -171,7 +171,7 @@ our %CMD = ( # sorted in order of importance/use: qw(format|f=s z|0 globoff|g invert-match|v local remote), @c_opt ], 'ls-label' => [ '', 'list labels', qw(z|0 stats:s), @c_opt ], 'ls-mail-sync' => [ '', 'list mail sync folders', - qw(z|0 z|0 globoff|g invert-match|v local remote), @c_opt ], + qw(z|0 globoff|g invert-match|v local remote), @c_opt ], 'forget-external' => [ 'LOCATION...|--prune', 'exclude further results from a publicinbox|extindex', qw(prune), @c_opt ], -- 2.31.1
lei_ls_mail_sync() is written to accept a filter, and ls-mail-sync has related command-line options (--globoff, --invert-match), but a positional argument isn't actually accepted. Add it. --- lib/PublicInbox/LEI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index e9c1675a..7be68121 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -170,7 +170,7 @@ our %CMD = ( # sorted in order of importance/use: 'ls-external' => [ '[FILTER]', 'list publicinbox|extindex locations', qw(format|f=s z|0 globoff|g invert-match|v local remote), @c_opt ], 'ls-label' => [ '', 'list labels', qw(z|0 stats:s), @c_opt ], -'ls-mail-sync' => [ '', 'list mail sync folders', +'ls-mail-sync' => [ '[FILTER]', 'list mail sync folders', qw(z|0 globoff|g invert-match|v local remote), @c_opt ], 'forget-external' => [ 'LOCATION...|--prune', 'exclude further results from a publicinbox|extindex', -- 2.31.1
If lei_ls_mail_sync() is given a filter without any wildcards and --globoff is unspecified, glob2re() will return undef, resulting in the final regular expression being undefined. Add a fallback value. --- I'm not sure if this is the cleanest approach; repeating qr/\Q$filter\E/ seems ugly. I considered something closer to what lei_ls_external() does, but decided against it because it leads to --globoff with no filter showing no output, which I think is surprising (even if passing --globoff with no filter doesn't make any sense). This probably deserves a test, but I'm out of time for tonight. If the change looks okay, I'm happy to look into adding a test tomorrow. lib/PublicInbox/LeiLsMailSync.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/PublicInbox/LeiLsMailSync.pm b/lib/PublicInbox/LeiLsMailSync.pm index 532ea9b5..06e25a63 100644 --- a/lib/PublicInbox/LeiLsMailSync.pm +++ b/lib/PublicInbox/LeiLsMailSync.pm @@ -14,7 +14,7 @@ sub lei_ls_mail_sync { my $opt = $lei->{opt}; my $re; $re = defined($filter) ? qr/\Q$filter\E/ : qr/./ if $opt->{globoff}; - $re //= $lei->glob2re($filter // '*'); + $re //= $lei->glob2re($filter // '*') // qr/\Q$filter\E/;; my @f = $lms->folders; @f = $opt->{'invert-match'} ? grep(!/$re/, @f) : grep(/$re/, @f); if ($opt->{'local'} && !$opt->{remote}) { -- 2.31.1
ls-external and ls-mail-sync accept an --invert-match option. Show it in the --help output. --- lib/PublicInbox/LEI.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 7be68121..c5fdfeb8 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -270,6 +270,7 @@ my %OPTDESC = ( 'incremental! import' => 'import already seen IMAP and NNTP articles', 'globoff|g' => "do not match locations using '*?' wildcards ". "and\xa0'[]'\x{a0}ranges", +'invert-match|v' => 'select non-matching lines', 'color!' => 'disable color (for --format=text)', 'verbose|v+' => 'be more verbose', 'external!' => 'do not use externals', -- 2.31.1
Kyle Meyer <kyle@kyleam.com> wrote: > If lei_ls_mail_sync() is given a filter without any wildcards and > --globoff is unspecified, glob2re() will return undef, resulting in > the final regular expression being undefined. Add a fallback value. > --- > > I'm not sure if this is the cleanest approach; repeating > qr/\Q$filter\E/ seems ugly. I considered something closer to what > lei_ls_external() does, but decided against it because it leads to > --globoff with no filter showing no output, which I think is > surprising (even if passing --globoff with no filter doesn't make > any sense). Thanks for bringing this up. My code there was an insomnia-generated disaster :x Mixing a ternary statement with a trailing "if" was just gross. Below is a shorter, less-redundant rewrite based on your fix. > This probably deserves a test, but I'm out of time for tonight. If > the change looks okay, I'm happy to look into adding a test > tomorrow. Yes, but probably no rush. I still need to get mail-sync working... (sidetracked into working on "lei index" :x) ----------8<------------------------ Subject: [PATCH] lei ls-mail-sync: fix handling of non-wildcard filters If lei_ls_mail_sync() is given a filter without any wildcards and --globoff is unspecified, glob2re() will return undef, resulting in the final regular expression being undefined. Always use a fallback value when there's no RE. Based-on-patch-by: Kyle Meyer <kyle@kyleam.com> Link: https://public-inbox.org/meta/20210504044559.12941-5-kyle@kyleam.com/ --- lib/PublicInbox/LeiLsMailSync.pm | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/PublicInbox/LeiLsMailSync.pm b/lib/PublicInbox/LeiLsMailSync.pm index 532ea9b5..505c0b3f 100644 --- a/lib/PublicInbox/LeiLsMailSync.pm +++ b/lib/PublicInbox/LeiLsMailSync.pm @@ -12,9 +12,8 @@ sub lei_ls_mail_sync { my $sto = $lei->_lei_store or return; my $lms = $sto->search->lms or return; my $opt = $lei->{opt}; - my $re; - $re = defined($filter) ? qr/\Q$filter\E/ : qr/./ if $opt->{globoff}; - $re //= $lei->glob2re($filter // '*'); + my $re = $opt->{globoff} ? undef : $lei->glob2re($filter // '*'); + $re //= qr/\Q$filter\E/; my @f = $lms->folders; @f = $opt->{'invert-match'} ? grep(!/$re/, @f) : grep(/$re/, @f); if ($opt->{'local'} && !$opt->{remote}) {