From 9d0f17840479508de4aaf76fe6c150e94a9f79c3 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 19 Sep 2021 12:50:33 +0000 Subject: [PATCH] lei config --edit: use controlling terminal 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 +# License: AGPL-3.0+ +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(<{-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(<{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, <{-f} is unparseable -EOM +sub cfg_verify { + my ($self, $cfg) = @_; my $new_out = $cfg->{'lei.q.output'} // ''; - return do_edit($lss, $lei, <{-f} has multiple values of lei.q.output + return $self->cfg_do_edit(<{-f} has multiple values of lei.q.output EOM - return do_edit($lss, $lei, <{-f} needs lei.q.output + return $self->cfg_do_edit(<{-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, <cfg_do_edit(<cfg_do_edit(<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 { -- 2.44.0