user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* Re: [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters
  2021-05-04  4:45  7% ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
@ 2021-05-04  5:14 14%   ` Eric Wong
  0 siblings, 0 replies; 3+ results
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 related	[relevance 14%]

* [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters
  2021-05-04  4:45  7% [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
@ 2021-05-04  4:45  7% ` Kyle Meyer
  2021-05-04  5:14 14%   ` Eric Wong
  0 siblings, 1 reply; 3+ results
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 related	[relevance 7%]

* [PATCH 0/5] Minor ls-mail-sync fixes
@ 2021-05-04  4:45  7% Kyle Meyer
  2021-05-04  4:45  7% ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
  0 siblings, 1 reply; 3+ results
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	[relevance 7%]

Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2021-05-04  4:45  7% [PATCH 0/5] Minor ls-mail-sync fixes Kyle Meyer
2021-05-04  4:45  7% ` [PATCH 4/5] lei ls-mail-sync: fix handling of non-wildcard filters Kyle Meyer
2021-05-04  5:14 14%   ` 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).