[PATCH] lei: do label/keyword parsing in optparse

[PATCH] lei: do label/keyword parsing in optparse

[Eric Wong]

Calling vmd_mod_extract after optparse causes the implicit
stdin-as-input functionality to fail, as the implicit stdin
requires a lack of inputs remaining in argv after option
parsing (along with a regular file or pipe as stdin).

This allows commands such as `lei import -F eml +kw:seen'
to work without `--stdin', `-' or any path names when
importing a single message.  This also ensures commands like
`lei import +kw:seen' without any inputs/locations will fail
reliably, as the extra +kw: arg won't be a false-positive.
---
  I noticed this while attempting to write a test for
  speeding up non-thread queries:
  https://public-inbox.org/meta/20231002145807.2665296-1-e@80x24.org/

 lib/PublicInbox/LEI.pm         | 18 +++++++++++++-----
 lib/PublicInbox/LeiAddWatch.pm |  4 +---
 lib/PublicInbox/LeiImport.pm   |  4 +---
 lib/PublicInbox/LeiInput.pm    | 11 +++++------
 lib/PublicInbox/LeiTag.pm      |  7 ++-----
 t/lei-import.t                 | 12 ++++++++++++
 6 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 06bc7ebd..817772f7 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -234,7 +234,7 @@ our %CMD = ( # sorted in order of importance/use:
 'plonk' => [ '--threads|--from=IDENT',
 	'exclude mail matching From: or threads from non-Message-ID searches',
 	qw(stdin| threads|t from|f=s mid=s oid=s), @c_opt ],
-'tag' => [ 'KEYWORDS...',
+'tag' => [ 'KEYWORDS... LOCATION...|--stdin',
 	'set/unset keywords and/or labels on message(s)',
 	qw(stdin| in-format|F=s input|i=s@ oid=s@ mid=s@),
 	@net_opt, @c_opt, pass_through('-kw:foo for delete') ],
@@ -243,7 +243,8 @@ our %CMD = ( # sorted in order of importance/use:
 	'remove imported messages from IMAP, Maildirs, and MH',
 	qw(exact! all jobs:i indexed), @c_opt ],
 
-'add-watch' => [ 'LOCATION...', 'watch for new messages and flag changes',
+'add-watch' => [ 'LOCATION... [LABELS...]',
+	'watch for new messages and flag changes',
 	qw(poll-interval=s state=s recursive|r), @c_opt ],
 'rm-watch' => [ 'LOCATION...', 'remove specified watch(es)',
 	qw(recursive|r), @c_opt ],
@@ -260,7 +261,7 @@ our %CMD = ( # sorted in order of importance/use:
 	qw(in-format|F=s kw! offset=i recursive|r exclude=s include|I=s
 	verbose|v+ incremental!), @net_opt, # mainly for --proxy=
 	 @c_opt ],
-'import' => [ 'LOCATION...|--stdin',
+'import' => [ 'LOCATION...|--stdin [LABELS...]',
 	'one-time import/update from URL or filesystem',
 	qw(stdin| offset=i recursive|r exclude=s include|I=s new-only
 	lock=s@ in-format|F=s kw! verbose|v+ incremental! mail-sync!),
@@ -711,6 +712,14 @@ sub optparse ($$$) {
 	# "-" aliases "stdin" or "clear"
 	$OPT->{$lone_dash} = ${$OPT->{$lone_dash}} if defined $lone_dash;
 
+	if ($proto =~ s/\s*\[?(?:KEYWORDS|LABELS)\.\.\.\]?\s*//g) {
+		require PublicInbox::LeiInput;
+		my @err = PublicInbox::LeiInput::vmd_mod_extract($self, $argv);
+		return $self->fail(join("\n", @err)) if @err;
+	} else {
+		warn "proto $proto\n" if $cmd =~ /(add-watch|tag|index)/;
+	}
+
 	my $i = 0;
 	my $POS_ARG = '[A-Z][A-Z0-9_]+';
 	my ($err, $inf);
@@ -741,8 +750,7 @@ sub optparse ($$$) {
 							 -f _) && -r _) {
 						$OPT->{stdin} //= 1;
 					}
-					$ok = defined($OPT->{$sw});
-					last if $ok;
+					$ok = defined($OPT->{$sw}) and last;
 				} elsif (defined($argv->[$i])) {
 					$ok = 1;
 					$i++;
diff --git a/lib/PublicInbox/LeiAddWatch.pm b/lib/PublicInbox/LeiAddWatch.pm
index f61e2de4..e2be5cee 100644
--- a/lib/PublicInbox/LeiAddWatch.pm
+++ b/lib/PublicInbox/LeiAddWatch.pm
@@ -15,11 +15,9 @@ sub lei_add_watch {
 	my $state = $lei->{opt}->{'state'} // 'import-rw';
 	$lei->watch_state_ok($state) or
 		return $lei->fail("invalid state: $state");
-	my $vmd_mod = $self->vmd_mod_extract(\@argv);
-	return $lei->fail(join("\n", @{$vmd_mod->{err}})) if $vmd_mod->{err};
 	$self->prepare_inputs($lei, \@argv) or return;
 	my @vmd;
-	while (my ($type, $vals) = each %$vmd_mod) {
+	while (my ($type, $vals) = each %{$lei->{vmd_mod}}) {
 		push @vmd, "$type:$_" for @$vals;
 	}
 	my $vmd0 = shift @vmd;
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index a324a652..c2552bf0 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -71,9 +71,7 @@ sub do_import_index ($$@) {
 	my $sto = $lei->_lei_store(1);
 	$sto->write_prepare($lei);
 	$self->{-import_kw} = $lei->{opt}->{kw} // 1;
-	my $vmd_mod = $self->vmd_mod_extract(\@inputs);
-	return $lei->fail(join("\n", @{$vmd_mod->{err}})) if $vmd_mod->{err};
-	$self->{all_vmd} = $vmd_mod if scalar keys %$vmd_mod;
+	$self->{all_vmd} = $lei->{vmd_mod} if keys %{$lei->{vmd_mod}};
 	$lei->ale; # initialize for workers to read (before LeiPmdir->new)
 	$self->{-mail_sync} = $lei->{opt}->{'mail-sync'} // 1;
 	$self->prepare_inputs($lei, \@inputs) or return;
diff --git a/lib/PublicInbox/LeiInput.pm b/lib/PublicInbox/LeiInput.pm
index 58069b0a..91383265 100644
--- a/lib/PublicInbox/LeiInput.pm
+++ b/lib/PublicInbox/LeiInput.pm
@@ -491,23 +491,22 @@ sub input_only_net_merge_all_done {
 # for update_xvmd -> update_vmd
 # returns something like { "+L" => [ @Labels ], ... }
 sub vmd_mod_extract {
-	my $argv = $_[-1];
-	my $vmd_mod = {};
-	my @new_argv;
+	my ($lei, $argv) = @_;
+	my (@new_argv, @err);
 	for my $x (@$argv) {
 		if ($x =~ /\A(\+|\-)(kw|L):(.+)\z/) {
 			my ($op, $pfx, $val) = ($1, $2, $3);
 			if (my $err = $ERR{$pfx}->($val)) {
-				push @{$vmd_mod->{err}}, $err;
+				push @err, $err;
 			} else { # set "+kw", "+L", "-L", "-kw"
-				push @{$vmd_mod->{$op.$pfx}}, $val;
+				push @{$lei->{vmd_mod}->{$op.$pfx}}, $val;
 			}
 		} else {
 			push @new_argv, $x;
 		}
 	}
 	@$argv = @new_argv;
-	$vmd_mod;
+	@err;
 }
 
 1;
diff --git a/lib/PublicInbox/LeiTag.pm b/lib/PublicInbox/LeiTag.pm
index 8ce96a10..76bd2d70 100644
--- a/lib/PublicInbox/LeiTag.pm
+++ b/lib/PublicInbox/LeiTag.pm
@@ -13,7 +13,7 @@ sub input_eml_cb { # used by PublicInbox::LeiInput::input_fh
 	if (my $xoids = $self->{lse}->xoids_for($eml) // # tries LeiMailSync
 			$self->{lei}->{ale}->xoids_for($eml)) {
 		$self->{lei}->{sto}->wq_do('update_xvmd', $xoids, $eml,
-						$self->{vmd_mod});
+						$self->{lei}->{vmd_mod});
 	} else {
 		++$self->{unimported};
 	}
@@ -31,11 +31,8 @@ sub lei_tag { # the "lei tag" method
 	my $sto = $lei->_lei_store(1)->write_prepare($lei);
 	my $self = bless {}, __PACKAGE__;
 	$lei->ale; # refresh and prepare
-	my $vmd_mod = $self->vmd_mod_extract(\@argv);
-	return $lei->fail(join("\n", @{$vmd_mod->{err}})) if $vmd_mod->{err};
-	$self->{vmd_mod} = $vmd_mod; # before LeiPmdir->new in prepare_inputs
 	$self->prepare_inputs($lei, \@argv) or return;
-	grep(defined, @$vmd_mod{qw(+kw +L -L -kw)}) or
+	grep(defined, @{$lei->{vmd_mod}}{qw(+kw +L -L -kw)}) or
 		return $lei->fail('no keywords or labels specified');
 	$lei->{-err_type} = 'non-fatal';
 	$lei->wq1_start($self);
diff --git a/t/lei-import.t b/t/lei-import.t
index c9e668a3..30d8b531 100644
--- a/t/lei-import.t
+++ b/t/lei-import.t
@@ -126,6 +126,18 @@ $res = json_utf8->decode($lei_out);
 is_deeply($res->[0]->{kw}, [qw(answered seen)], 'keyword added');
 is_deeply($res->[0]->{L}, [qw(boombox inbox)], 'labels preserved');
 
+# +kw:seen is not a location
+ok(!lei(qw(import -F eml +kw:seen)), 'import fails w/ only kw arg');
+like($lei_err, qr/\bLOCATION\.\.\. or --stdin must be set/, 'error message');
+
+lei_ok([qw(import -F eml +kw:flagged)], # no lone dash (`-')
+	undef, { %$lei_opt, 0 => \$eml_str },
+	'import succeeds with implicit --stdin');
+lei_ok(qw(q m:inbox@example.com));
+$res = json_utf8->decode($lei_out);
+is_deeply($res->[0]->{kw}, [qw(answered flagged seen)], 'keyword added');
+is_deeply($res->[0]->{L}, [qw(boombox inbox)], 'labels preserved');
+
 # see t/lei_to_mail.t for "import -F mbox*"
 });
 done_testing;

Re: [PATCH] lei: do label/keyword parsing in optparse

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> +++ b/t/lei-import.t
> @@ -126,6 +126,18 @@ $res = json_utf8->decode($lei_out);
>  is_deeply($res->[0]->{kw}, [qw(answered seen)], 'keyword added');
>  is_deeply($res->[0]->{L}, [qw(boombox inbox)], 'labels preserved');
>  
> +# +kw:seen is not a location
> +ok(!lei(qw(import -F eml +kw:seen)), 'import fails w/ only kw arg');
> +like($lei_err, qr/\bLOCATION\.\.\. or --stdin must be set/, 'error message');

That's unreliable because stdin could be pointed to a
regular file or pipe while running tests, so we shouldn't
inherit.

So I think I'll squash the following in and use autodie a bit
more while I'm at it, too.
---
 t/lei-import.t | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/t/lei-import.t b/t/lei-import.t
index 30d8b531..8b09d3aa 100644
--- a/t/lei-import.t
+++ b/t/lei-import.t
@@ -1,14 +1,15 @@
 #!perl -w
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
-use strict; use v5.10.1; use PublicInbox::TestCommon;
+use v5.12; use PublicInbox::TestCommon;
+use autodie qw(open close);
 test_lei(sub {
 ok(!lei(qw(import -F bogus), 't/plack-qp.eml'), 'fails with bogus format');
 like($lei_err, qr/\bis `eml', not --in-format/, 'gave error message');
 
 lei_ok(qw(q s:boolean), \'search miss before import');
 unlike($lei_out, qr/boolean/i, 'no results, yet');
-open my $fh, '<', 't/data/0001.patch' or BAIL_OUT $!;
+open my $fh, '<', 't/data/0001.patch';
 lei_ok([qw(import -F eml -)], undef, { %$lei_opt, 0 => $fh },
 	\'import single file from stdin') or diag $lei_err;
 close $fh;
@@ -18,7 +19,7 @@ lei_ok(qw(q s:boolean -f mboxrd), \'blob accessible after import');
 	my $expect = [ eml_load('t/data/0001.patch') ];
 	require PublicInbox::MboxReader;
 	my @cmp;
-	open my $fh, '<', \$lei_out or BAIL_OUT "open :scalar: $!";
+	open my $fh, '<', \$lei_out;
 	PublicInbox::MboxReader->mboxrd($fh, sub {
 		my ($eml) = @_;
 		$eml->header_set('Status');
@@ -127,8 +128,10 @@ is_deeply($res->[0]->{kw}, [qw(answered seen)], 'keyword added');
 is_deeply($res->[0]->{L}, [qw(boombox inbox)], 'labels preserved');
 
 # +kw:seen is not a location
-ok(!lei(qw(import -F eml +kw:seen)), 'import fails w/ only kw arg');
-like($lei_err, qr/\bLOCATION\.\.\. or --stdin must be set/, 'error message');
+open my $null, '<', '/dev/null';
+ok(!lei([qw(import -F eml +kw:seen)], undef, { %$lei_opt, 0 => $null }),
+	'import fails w/ only kw arg');
+like($lei_err, qr/\bLOCATION\.\.\. or --stdin must be set/s, 'error message');
 
 lei_ok([qw(import -F eml +kw:flagged)], # no lone dash (`-')
 	undef, { %$lei_opt, 0 => \$eml_str },