Skip to content

Commit 7f8371f

Browse files
claudeclaude
committed
TD-3: share action-opts handling between do_menu and do_form
The confirm/log/wait_key option loop was duplicated verbatim in do_menu and do_form (the only difference was the confirm dialog's title). Extract it into apply_action_opts(\@opts, $win, $title) -> ($wait_key, $aborted, $es); both call sites collapse from ~28 lines to ~5. Single source of truth for the action options. Guarded by the new t/28-action-confirm.t (TD-2-adjacent coverage), which drives the previously-untested confirm path end to end: declining (No) aborts the action, accepting (Yes) runs it. Behaviour preserved -- the menu/form/restricted action tests stay green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 03fc096 commit 7f8371f

2 files changed

Lines changed: 107 additions & 57 deletions

File tree

‎src/ccfe.pl‎

Lines changed: 37 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -2098,6 +2098,32 @@ sub load_config {
20982098
return $res;
20992099
}
21002100

2101+
# Apply an action's options (confirm / log / wait_key), shared by do_menu and
2102+
# do_form (TD-3 de-dup). Sets the global $LOG_REQUESTED; `confirm` pops a
2103+
# Yes/No list titled $title. Returns ($wait_key, $aborted, $es): $aborted is
2104+
# true when the user declined the confirmation, and $es is the confirmation
2105+
# list's status (undef when there was no `confirm` option) so the caller can
2106+
# honour an exit from that dialog.
2107+
sub apply_action_opts {
2108+
my ( $opts, $win, $title ) = @_;
2109+
my $wait_key = $NO;
2110+
my $aborted = $NO;
2111+
my $es;
2112+
$LOG_REQUESTED = $NO;
2113+
for my $opt ( @{$opts} ) {
2114+
if ( $opt eq 'confirm' ) {
2115+
my $val;
2116+
( $es, $val ) =
2117+
do_list( $win, $title, 'single-val', \@CONFIRM_ITEMS, undef );
2118+
$aborted = $YES if $val ne $BFIELD_YES;
2119+
}
2120+
elsif ( $opt eq 'log' ) { $LOG_REQUESTED = $YES }
2121+
elsif ( $opt eq 'wait_key' ) { $wait_key = $YES }
2122+
else { trace("unknown action option \"$opt\"") }
2123+
}
2124+
return ( $wait_key, $aborted, $es );
2125+
}
2126+
21012127
sub do_menu {
21022128
my ( $menuname, $title ) = @_;
21032129

@@ -2293,35 +2319,12 @@ sub do_menu {
22932319
$args = $act->{args};
22942320
@actopts = @{ $act->{opts} };
22952321

2296-
$wait_key = $NO;
2297-
$LOG_REQUESTED = $NO;
2298-
foreach $opt (@actopts) {
2299-
SWITCH: {
2300-
$_ = $opt;
2301-
if (/^confirm$/) {
2302-
my $title = $menu{items}[$ci]{descr};
2303-
my $val;
2304-
( $es, $val ) =
2305-
do_list( $win, $title, 'single-val',
2306-
\@CONFIRM_ITEMS, undef );
2307-
if ( $val ne $BFIELD_YES ) {
2308-
$action = 'ABORTED';
2309-
}
2310-
last SWITCH;
2311-
}
2312-
elsif (/^log$/) {
2313-
$LOG_REQUESTED = $YES;
2314-
last SWITCH;
2315-
}
2316-
elsif (/^wait_key$/) {
2317-
$wait_key = $YES;
2318-
last SWITCH;
2319-
}
2320-
else {
2321-
trace("unknown action option \"$_\"");
2322-
}
2323-
}
2324-
}
2322+
my ( $aborted, $opt_es );
2323+
( $wait_key, $aborted, $opt_es ) =
2324+
apply_action_opts( \@actopts, $win,
2325+
$menu{items}[$ci]{descr} );
2326+
$es = $opt_es if defined $opt_es;
2327+
$action = 'ABORTED' if $aborted;
23252328

23262329
if ( $action eq 'menu' ) {
23272330
( $es, undef, undef ) =
@@ -3871,34 +3874,11 @@ sub do_form {
38713874
$args = $act->{args};
38723875
@actopts = @{ $act->{opts} };
38733876

3874-
$wait_key = $NO;
3875-
$LOG_REQUESTED = $NO;
3876-
foreach $opt (@actopts) {
3877-
SWITCH: {
3878-
$_ = $opt;
3879-
if (/^confirm$/) {
3880-
my $val;
3881-
( $es, $val ) =
3882-
do_list( $win, $CONFIRM_TITLE, 'single-val',
3883-
\@CONFIRM_ITEMS, undef );
3884-
if ( $val ne $BFIELD_YES ) {
3885-
$action = 'ABORTED';
3886-
}
3887-
last SWITCH;
3888-
}
3889-
elsif (/^log$/) {
3890-
$LOG_REQUESTED = $YES;
3891-
last SWITCH;
3892-
}
3893-
elsif (/^wait_key$/) {
3894-
$wait_key = $YES;
3895-
last SWITCH;
3896-
}
3897-
else {
3898-
trace("unknown action option \"$_\"");
3899-
}
3900-
}
3901-
}
3877+
my ( $aborted, $opt_es );
3878+
( $wait_key, $aborted, $opt_es ) =
3879+
apply_action_opts( \@actopts, $win, $CONFIRM_TITLE );
3880+
$es = $opt_es if defined $opt_es;
3881+
$action = 'ABORTED' if $aborted;
39023882
$save_persistent->();
39033883
if ( $action eq 'run' ) {
39043884
$prepare_action->( \$args );

‎src/t/28-action-confirm.t‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/usr/bin/perl
2+
#
3+
# Action `confirm` option (TD-2 coverage; guards the TD-3 opts de-dup).
4+
#
5+
# An action with the `confirm` option pops a Yes/No list before running; No
6+
# aborts it. This path (do_menu/do_form -> the confirm do_list) was untested.
7+
# Driving it both fixes the gap and pins the behaviour before the confirm/log/
8+
# wait_key opts handling is shared between do_menu and do_form.
9+
#
10+
use strict;
11+
use warnings;
12+
use FindBin qw($Bin);
13+
use lib "$Bin/lib";
14+
use File::Temp qw(tempdir);
15+
use Test::More;
16+
17+
my $src = "$Bin/..";
18+
19+
eval { require CCFE::Test::Pty; 1 } or plan skip_all => "pty helper: $@";
20+
plan skip_all => 'no Linux pseudo-terminal' unless CCFE::Test::Pty->available;
21+
plan skip_all => 'Curses not installed' unless eval { require Curses; 1 };
22+
plan skip_all => 'no installer' unless -f "$src/install.sh";
23+
24+
my $prefix = tempdir( CLEANUP => 1 );
25+
my $log = `cd "$src" && sh install.sh -b -p "$prefix" 2>&1`;
26+
plan skip_all => "install failed: $log" unless $? == 0 && -x "$prefix/bin/ccfe";
27+
28+
my $objs = "$prefix/share/ccfe/objects/ccfe";
29+
open( my $mf, '>', "$objs/cmenu.menu" ) or plan skip_all => "menu: $!";
30+
print {$mf} <<'MENU';
31+
title { Confirm test }
32+
item {
33+
id = T
34+
descr = do it
35+
action = run(confirm): printf 'CONFIRMED_RAN\n'
36+
}
37+
MENU
38+
close($mf);
39+
40+
plan tests => 3;
41+
42+
# --- decline: No aborts the action --------------------------------------
43+
my $p = CCFE::Test::Pty->spawn( 80, 24, "$prefix/bin/ccfe", 'cmenu' );
44+
$p->pump(1.2);
45+
$p->send("\r"); # activate -> confirm Yes/No list pops
46+
$p->pump(0.8);
47+
like( $p->screen, qr/Confirm and continue|Abort and return/,
48+
'confirm opens a Yes/No chooser' );
49+
# default selection is No -> Enter declines
50+
$p->send("\r");
51+
$p->pump(0.8);
52+
unlike( $p->screen, qr/CONFIRMED_RAN/,
53+
'declining the confirmation does not run the action' );
54+
$p->send("\e");
55+
$p->pump(0.3);
56+
57+
# --- accept: choosing Yes runs it ---------------------------------------
58+
my $p2 = CCFE::Test::Pty->spawn( 80, 24, "$prefix/bin/ccfe", 'cmenu' );
59+
$p2->pump(1.2);
60+
$p2->send("\r"); # activate -> confirm list
61+
$p2->pump(0.8);
62+
$p2->send("\eOB"); # down-arrow (application mode) to the YES item
63+
$p2->pump(0.3);
64+
$p2->send("\r"); # accept
65+
$p2->pump(1.0);
66+
like( $p2->screen, qr/CONFIRMED_RAN/,
67+
'confirming with Yes runs the action' );
68+
$p2->send(" ");
69+
$p2->pump(0.2);
70+
$p2->send("\e");

0 commit comments

Comments
 (0)