[PATCH 14/16] lei config --edit: use controlling terminal

[Eric Wong <e@80x24.org>]

As with "lei edit-search", "lei config --edit" may
spawn an interactive editor which works best from
the terminal running script/lei.

So implement LeiConfig as a superclass of LeiEditSearch
so the two commands can share the same verification
hooks and retry logic.
---
 MANIFEST                          |  1 +
 lib/PublicInbox/LEI.pm            | 18 +++++-----
 lib/PublicInbox/LeiConfig.pm      | 42 ++++++++++++++++++++++
 lib/PublicInbox/LeiEditSearch.pm  | 60 +++++++++++--------------------
 lib/PublicInbox/LeiExternal.pm    |  2 +-
 lib/PublicInbox/LeiInit.pm        |  4 +--
 lib/PublicInbox/LeiSavedSearch.pm | 16 +++------
 t/lei.t                           |  3 ++
 8 files changed, 82 insertions(+), 64 deletions(-)
 create mode 100644 lib/PublicInbox/LeiConfig.pm

diff --git a/MANIFEST b/MANIFEST
index 2df743f8..8c2e964b 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -208,6 +208,7 @@ lib/PublicInbox/LeiALE.pm
 lib/PublicInbox/LeiAddWatch.pm
 lib/PublicInbox/LeiAuth.pm
 lib/PublicInbox/LeiBlob.pm
+lib/PublicInbox/LeiConfig.pm
 lib/PublicInbox/LeiConvert.pm
 lib/PublicInbox/LeiCurl.pm
 lib/PublicInbox/LeiDedupe.pm
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index b468a32c..148a5b1e 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -278,7 +278,7 @@ our %CMD = ( # sorted in order of importance/use:
 'config' => [ '[...]', sub {
 		'git-config(1) wrapper for '._config_path($_[0]);
 	}, qw(config-file|system|global|file|f=s), # for conflict detection
-	 qw(c=s@ C=s@), pass_through('git config') ],
+	 qw(edit|e c=s@ C=s@), pass_through('git config') ],
 'inspect' => [ 'ITEMS...|--stdin', 'inspect lei/store and/or local external',
 	qw(stdin| pretty ascii dir=s), @c_opt ],
 
@@ -870,14 +870,6 @@ sub _config {
 	waitpid(spawn($cmd, \%env, \%rdr), 0);
 }
 
-sub lei_config {
-	my ($self, @argv) = @_;
-	$self->{opt}->{'config-file'} and return fail $self,
-		"config file switches not supported by `lei config'";
-	_config(@_);
-	x_it($self, $?) if $?;
-}
-
 sub lei_daemon_pid { puts shift, $$ }
 
 sub lei_daemon_kill {
@@ -1504,4 +1496,12 @@ sub sto_done_request {
 	$lei->err($@) if $@;
 }
 
+sub cfg_dump ($$) {
+	my ($lei, $f) = @_;
+	my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
+	return $ret if !$@;
+	$lei->err($@);
+	undef;
+}
+
 1;
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
new file mode 100644
index 00000000..23be9aaf
--- /dev/null
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -0,0 +1,42 @@
+# Copyright (C) 2021 all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+package PublicInbox::LeiConfig;
+use strict;
+use v5.10.1;
+use PublicInbox::PktOp;
+
+sub cfg_do_edit ($;$) {
+	my ($self, $reason) = @_;
+	my $lei = $self->{lei};
+	$lei->pgr_err($reason) if defined $reason;
+	my $cmd = [ qw(git config --edit -f), $self->{-f} ];
+	my $env = { GIT_CONFIG => $self->{-f} };
+	$self->cfg_edit_begin if $self->can('cfg_edit_begin');
+	# run in script/lei foreground
+	my ($op_c, $op_p) = PublicInbox::PktOp->pair;
+	# $op_p will EOF when $EDITOR is done
+	$op_c->{ops} = { '' => [\&cfg_edit_done, $self] };
+	$lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p->{op_p} ], $cmd, $env);
+}
+
+sub cfg_edit_done { # PktOp
+	my ($self) = @_;
+	eval {
+		my $cfg = $self->{lei}->cfg_dump($self->{-f}, $self->{lei}->{2})
+			// return cfg_do_edit($self, "\n");
+		$self->cfg_verify($cfg) if $self->can('cfg_verify');
+	};
+	$self->{lei}->fail($@) if $@;
+}
+
+sub lei_config {
+	my ($lei, @argv) = @_;
+	$lei->{opt}->{'config-file'} and return $lei->fail(
+		"config file switches not supported by `lei config'");
+	return $lei->_config(@argv) unless $lei->{opt}->{edit};
+	my $f = $lei->_lei_cfg(1)->{-f};
+	my $self = bless { lei => $lei, -f => $f }, __PACKAGE__;
+	cfg_do_edit($self);
+}
+
+1;
diff --git a/lib/PublicInbox/LeiEditSearch.pm b/lib/PublicInbox/LeiEditSearch.pm
index 47166ce7..bcf7c105 100644
--- a/lib/PublicInbox/LeiEditSearch.pm
+++ b/lib/PublicInbox/LeiEditSearch.pm
@@ -7,48 +7,32 @@ use strict;
 use v5.10.1;
 use PublicInbox::LeiSavedSearch;
 use PublicInbox::LeiUp;
+use parent qw(PublicInbox::LeiConfig);
 
-sub edit_begin {
-	my ($lss, $lei) = @_;
-	if (ref($lss->{-cfg}->{'lei.q.output'})) {
-		delete $lss->{-cfg}->{'lei.q.output'}; # invalid
-		$lei->pgr_err(<<EOM);
-$lss->{-f} has multiple values of lei.q.output
+sub cfg_edit_begin {
+	my ($self) = @_;
+	if (ref($self->{lss}->{-cfg}->{'lei.q.output'})) {
+		delete $self->{lss}->{-cfg}->{'lei.q.output'}; # invalid
+		$self->{lei}->pgr_err(<<EOM);
+$self->{lss}->{-f} has multiple values of lei.q.output
 please remove redundant ones
 EOM
 	}
-	$lei->{-lss_for_edit} = $lss;
 }
 
-sub do_edit ($$;$) {
-	my ($lss, $lei, $reason) = @_;
-	$lei->pgr_err($reason) if defined $reason;
-	my @cmd = (qw(git config --edit -f), $lss->{'-f'});
-	$lei->qerr("# spawning @cmd");
-	edit_begin($lss, $lei);
-	# run in script/lei foreground
-	require PublicInbox::PktOp;
-	my ($op_c, $op_p) = PublicInbox::PktOp->pair;
-	# $op_p will EOF when $EDITOR is done
-	$op_c->{ops} = { '' => [\&op_edit_done, $lss, $lei] };
-	$lei->send_exec_cmd([ @$lei{qw(0 1 2)}, $op_p->{op_p} ], \@cmd, {});
-}
-
-sub _edit_done {
-	my ($lss, $lei) = @_;
-	my $cfg = $lss->can('cfg_dump')->($lei, $lss->{'-f'}) //
-		return do_edit($lss, $lei, <<EOM);
-$lss->{-f} is unparseable
-EOM
+sub cfg_verify {
+	my ($self, $cfg) = @_;
 	my $new_out = $cfg->{'lei.q.output'} // '';
-	return do_edit($lss, $lei, <<EOM) if ref $new_out;
-$lss->{-f} has multiple values of lei.q.output
+	return $self->cfg_do_edit(<<EOM) if ref $new_out;
+$self->{-f} has multiple values of lei.q.output
 EOM
-	return do_edit($lss, $lei, <<EOM) if $new_out eq '';
-$lss->{-f} needs lei.q.output
+	return $self->cfg_do_edit(<<EOM) if $new_out eq '';
+$self->{-f} needs lei.q.output
 EOM
+	my $lss = $self->{lss};
 	my $old_out = $lss->{-cfg}->{'lei.q.output'} // return;
 	return if $old_out eq $new_out;
+	my $lei = $self->{lei};
 	my $old_path = $old_out;
 	my $new_path = $new_out;
 	s!$PublicInbox::LeiSavedSearch::LOCAL_PFX!! for ($old_path, $new_path);
@@ -57,10 +41,10 @@ EOM
 	return if $dir_new eq $dir_old;
 
 	($old_out =~ m!\Av2:!i || $new_out =~ m!\Av2:!) and
-		return do_edit($lss, $lei, <<EOM);
+		return $self->cfg_do_edit(<<EOM);
 conversions from/to v2 inboxes not supported at this time
 EOM
-	return do_edit($lss, $lei, <<EOM) if -e $dir_new;
+	return $self->cfg_do_edit(<<EOM) if -e $dir_new;
 lei.q.output changed from `$old_out' to `$new_out'
 However, $dir_new exists
 EOM
@@ -79,16 +63,12 @@ E: rename($dir_old, $dir_new) error: $!
 EOM
 }
 
-sub op_edit_done { # PktOp
-	my ($lss, $lei) = @_;
-	eval { _edit_done($lss, $lei) };
-	$lei->fail($@) if $@;
-}
-
 sub lei_edit_search {
 	my ($lei, $out) = @_;
 	my $lss = PublicInbox::LeiSavedSearch->up($lei, $out) or return;
-	do_edit($lss, $lei);
+	my $f = $lss->{-f};
+	my $self = bless { lei => $lei, lss => $lss, -f => $f }, __PACKAGE__;
+	$self->cfg_do_edit;
 }
 
 *_complete_edit_search = \&PublicInbox::LeiUp::_complete_up;
diff --git a/lib/PublicInbox/LeiExternal.pm b/lib/PublicInbox/LeiExternal.pm
index 6fd3efef..d802f0e2 100644
--- a/lib/PublicInbox/LeiExternal.pm
+++ b/lib/PublicInbox/LeiExternal.pm
@@ -139,7 +139,7 @@ sub add_external_finish {
 	my $key = "external.$location.boost";
 	my $cur_boost = $cfg->{$key};
 	return if defined($cur_boost) && $cur_boost == $new_boost; # idempotent
-	$self->lei_config($key, $new_boost);
+	$self->_config($key, $new_boost);
 }
 
 sub lei_add_external {
diff --git a/lib/PublicInbox/LeiInit.pm b/lib/PublicInbox/LeiInit.pm
index 6558ac0a..27ce8169 100644
--- a/lib/PublicInbox/LeiInit.pm
+++ b/lib/PublicInbox/LeiInit.pm
@@ -23,7 +23,7 @@ sub lei_init {
 
 		# some folks like symlinks and bind mounts :P
 		if (@dir && "@cur[1,0]" eq "@dir[1,0]") {
-			$self->lei_config('leistore.dir', $dir);
+			$self->_config('leistore.dir', $dir);
 			$self->_lei_store(1)->done;
 			return $self->qerr("$exists (as $cur)");
 		}
@@ -31,7 +31,7 @@ sub lei_init {
 E: leistore.dir=$cur already initialized and it is not $dir
 
 	}
-	$self->lei_config('leistore.dir', $dir);
+	$self->_config('leistore.dir', $dir);
 	$self->_lei_store(1)->done;
 	$exists //= "# leistore.dir=$dir newly initialized";
 	$self->qerr($exists);
diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm
index 754f8294..89f5c359 100644
--- a/lib/PublicInbox/LeiSavedSearch.pm
+++ b/lib/PublicInbox/LeiSavedSearch.pm
@@ -29,14 +29,6 @@ sub BOOL_FIELDS () {
 	qw(external local remote import-remote import-before threads)
 }
 
-sub cfg_dump ($$) {
-	my ($lei, $f) = @_;
-	my $ret = eval { PublicInbox::Config->git_config_dump($f, $lei->{2}) };
-	return $ret if !$@;
-	$lei->err($@);
-	undef;
-}
-
 sub lss_dir_for ($$;$) {
 	my ($lei, $dstref, $on_fs) = @_;
 	my $pfx;
@@ -64,7 +56,7 @@ sub lss_dir_for ($$;$) {
 		for my $g ("$pfx-*", '*') {
 			my @maybe = glob("$lss_dir$g/lei.saved-search");
 			for my $f (@maybe) {
-				$c = cfg_dump($lei, $f) // next;
+				$c = $lei->cfg_dump($f) // next;
 				$o = $c->{'lei.q.output'} // next;
 				$o =~ s!$LOCAL_PFX!! or next;
 				@st = stat($o) or next;
@@ -88,7 +80,7 @@ sub list {
 		print $fh "\tpath = ", cquote_val($p), "\n";
 	}
 	close $fh or die "close $f: $!";
-	my $cfg = cfg_dump($lei, $f);
+	my $cfg = $lei->cfg_dump($f);
 	unlink($f);
 	my $out = $cfg ? $cfg->get_all('lei.q.output') : [];
 	map {;
@@ -113,7 +105,7 @@ sub up { # updating existing saved search via "lei up"
 	output2lssdir($self, $lei, \$dir, \$f) or
 		return $lei->fail("--save was not used with $dst cwd=".
 					$lei->rel2abs('.'));
-	$self->{-cfg} = cfg_dump($lei, $f) // return $lei->fail;
+	$self->{-cfg} = $lei->cfg_dump($f) // return $lei->fail;
 	$self->{-ovf} = "$dir/over.sqlite3";
 	$self->{'-f'} = $f;
 	$self->{lock_path} = "$self->{-f}.flock";
@@ -276,7 +268,7 @@ sub output2lssdir {
 	my $dir = lss_dir_for($lei, \$dst, 1);
 	my $f = "$dir/lei.saved-search";
 	if (-f $f && -r _) {
-		$self->{-cfg} = cfg_dump($lei, $f) // return;
+		$self->{-cfg} = $lei->cfg_dump($f) // return;
 		$$dir_ref = $dir;
 		$$fn_ref = $f;
 		return 1;
diff --git a/t/lei.t b/t/lei.t
index c8f47576..53fc43fb 100644
--- a/t/lei.t
+++ b/t/lei.t
@@ -100,6 +100,9 @@ my $test_config = sub {
 	is($lei_out, "tr00\n", "-c string value passed as-is");
 	lei_ok(qw(-c imap.debug=a -c imap.debug=b config --get-all imap.debug));
 	is($lei_out, "a\nb\n", '-c and --get-all work together');
+
+	lei_ok([qw(config -e)], { VISUAL => 'cat' });
+	is($lei_out, "[a]\n\tb = c\n", '--edit works');
 };
 
 my $test_completion = sub {