unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Minor ls-mail-sync fixes
@ 2021-05-04  4:45 Kyle Meyer
  2021-05-04  4:45 ` [PATCH 1/5] lei ls-mail-sync: update reference to ls-sync Kyle Meyer
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

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


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

* [PATCH 1/5] lei ls-mail-sync: update reference to ls-sync
  2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
@ 2021-05-04  4:45 ` Kyle Meyer
  2021-05-04  4:45 ` [PATCH 2/5] lei ls-mail-sync: drop repeated -z/0 option Kyle Meyer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

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


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

* [PATCH 2/5] lei ls-mail-sync: drop repeated -z/0 option
  2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
  2021-05-04  4:45 ` [PATCH 1/5] lei ls-mail-sync: update reference to ls-sync Kyle Meyer
@ 2021-05-04  4:45 ` Kyle Meyer
  2021-05-04  4:45 ` [PATCH 3/5] lei ls-mail-sync: accept a filter Kyle Meyer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

---
 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


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

* [PATCH 3/5] lei ls-mail-sync: accept a filter
  2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
  2021-05-04  4:45 ` [PATCH 1/5] lei ls-mail-sync: update reference to ls-sync Kyle Meyer
  2021-05-04  4:45 ` [PATCH 2/5] lei ls-mail-sync: drop repeated -z/0 option Kyle Meyer
@ 2021-05-04  4:45 ` Kyle Meyer
  2021-05-04  4:45 ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
  2021-05-04  4:45 ` [PATCH 5/5] lei: add help output for --invert match Kyle Meyer
  4 siblings, 0 replies; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

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


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

* [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters
  2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
                   ` (2 preceding siblings ...)
  2021-05-04  4:45 ` [PATCH 3/5] lei ls-mail-sync: accept a filter Kyle Meyer
@ 2021-05-04  4:45 ` Kyle Meyer
  2021-05-04  5:14   ` Eric Wong
  2021-05-04  4:45 ` [PATCH 5/5] lei: add help output for --invert match Kyle Meyer
  4 siblings, 1 reply; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

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


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

* [PATCH 5/5] lei: add help output for --invert match
  2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
                   ` (3 preceding siblings ...)
  2021-05-04  4:45 ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
@ 2021-05-04  4:45 ` Kyle Meyer
  4 siblings, 0 replies; 7+ messages in thread
From: Kyle Meyer @ 2021-05-04  4:45 UTC (permalink / raw)
  To: meta

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


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

* Re: [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters
  2021-05-04  4:45 ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
@ 2021-05-04  5:14   ` Eric Wong
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2021-05-04  5:14 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: meta

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}) {

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

end of thread, other threads:[~2021-05-04  5:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-04  4:45 [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
2021-05-04  4:45 ` [PATCH 1/5] lei ls-mail-sync: update reference to ls-sync Kyle Meyer
2021-05-04  4:45 ` [PATCH 2/5] lei ls-mail-sync: drop repeated -z/0 option Kyle Meyer
2021-05-04  4:45 ` [PATCH 3/5] lei ls-mail-sync: accept a filter Kyle Meyer
2021-05-04  4:45 ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
2021-05-04  5:14   ` Eric Wong
2021-05-04  4:45 ` [PATCH 5/5] lei: add help output for --invert match Kyle Meyer

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 NNTP newsgroup(s).