Skip to content

Commit 23978ad

Browse files
claudeclaude
committed
M7: extract pure CCFE::Layout geometry, de-dup do_form/resize_form
The M6 value-column geometry (right-aligned value that expands on a wide screen; a long label wraps onto its own line(s) when it would otherwise collide with the value) and the page-advance arithmetic were written out twice: once in do_form's initial layout, and again byte-for-byte in resize_form's reflow. That maths is pure -- screen columns/rows in, columns/rows out, no curses and no globals. Lift it into CCFE::Layout as field_geometry() and page_advance(); both sites now call them. The new_field/move_field/free_field calls, the wrap trace and the field-set packing stay in ccfe.pl. This removes the duplicated arithmetic (the value column and the page break depend on the width, so resize is a horizontal *and* vertical reflow -- exactly what do_form first computes), so the two can no longer drift apart. Unit-tested with hand-computed expectations in t/17-layout.t (27 cases); the resize/layout/multipage tty tests (t/08, t/10, t/11) and the full suite (now 302 tests) stay green -- the on-screen proof the geometry is byte-identical. All four CI checks pass locally. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 666d811 commit 23978ad

4 files changed

Lines changed: 284 additions & 54 deletions

File tree

‎ROADMAP.md‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,17 @@ the conformance tests.
192192
verb, prompting for `confirm`, honouring `log`/`wait_key`, spawning commands)
193193
stays in `ccfe.pl` as it draws lists and runs processes. Unit-tested in
194194
`t/16-action.t` (23 cases); the menu/form tty tests and full suite (275 tests)
195-
stay green. Next: the pure `Layout` helpers, and finally the `$ctx` threading.
195+
stay green.
196+
- 🔄 **`CCFE::Layout`** (done): the M6 value-column geometry (right-aligned
197+
value that expands on a wide screen; long labels wrap onto their own line(s)
198+
when they would collide with the value) and the page-advance arithmetic were
199+
written out *twice* -- in `do_form`'s initial layout and again, byte for byte,
200+
in `resize_form`'s reflow. That maths is pure (numbers in, numbers out), so it
201+
is now `field_geometry()` / `page_advance()` in a module both sites share; the
202+
`new_field`/`move_field` calls and tracing stay in `ccfe.pl`. Unit-tested with
203+
hand-computed expectations in `t/17-layout.t` (27 cases); the resize/layout/
204+
multipage tty tests and full suite (302 tests) stay green. Next: the `$ctx`
205+
threading to retire the remaining globals.
196206

197207
## M8 — Non-functional close-out audit _(final gate)_
198208
Five dimensions: **test coverage, code quality, performance, security,

‎src/ccfe.pl‎

Lines changed: 67 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
use CCFE::FormFile (); # pure .form parser (see load_form)
5151
use CCFE::Config (); # pure .conf section tokenizer (see load_config)
5252
use CCFE::Action (); # pure action-string parser (see do_menu/do_form)
53+
use CCFE::Layout (); # pure form value-column/page geometry (see do_form)
5354
use FindBin (); # to locate the program at runtime (see the path block below)
5455

5556
# Optional display-width support. In a UTF-8 locale a label/title can occupy
@@ -3021,8 +3022,7 @@ sub do_form {
30213022
$lflags_x +
30223023
$lflags_size +
30233024
$form{fields}[$i]{htab} * $HTAB_COLS;
3024-
my $lw = disp_width($label); # label width in columns
3025-
my $dots_x = $label_x + $lw;
3025+
my $lw = disp_width($label); # label width in columns
30263026
$val = '';
30273027
$val = $default if defined($default);
30283028
$val = $field_vals{$id} if defined( $field_vals{$id} );
@@ -3037,41 +3037,45 @@ sub do_form {
30373037
# label on a narrow terminal -- the label wraps onto its own
30383038
# full-width line(s) and the value drops to the row after the
30393039
# last label line, so it is never pushed off-screen or truncated.
3040+
# The geometry is pure (CCFE::Layout); resize_form reuses it.
30403041
my $auto = ( $LAYOUT == $NORMAL and $FIELD_VALUE_POS == -1 );
3041-
my $val_x = $FIELD_VALUE_POS;
3042-
if ( $val_x == -1 ) {
3043-
$val_x = $COLS - $len - 1 - $rflags_size;
3044-
$val_x = $label_x + 1 if $val_x < $label_x + 1;
3045-
}
3046-
3047-
# Wrap the label when it would otherwise collide with the value.
3048-
my $label_w = $lw;
3049-
my $wrap_rows = 0;
3050-
if ( $auto
3051-
and $label_x + $lw + $FIELD_VALUE_GAP > $val_x )
3052-
{
3053-
$label_w = $COLS - $label_x - $rflags_size - 1;
3054-
$label_w = 1 if $label_w < 1;
3055-
$wrap_rows = int( ( $lw + $label_w - 1 ) / $label_w );
3056-
$wrap_rows = 1 if $wrap_rows < 1;
3057-
$dots_x = $label_x; # value row: dots run from the margin
3058-
trace( "do_form: wrapped label \"$id\" over $wrap_rows line(s)",
3059-
$LOG_NORMAL );
3060-
}
3042+
my $geom = CCFE::Layout::field_geometry(
3043+
{
3044+
cols => $COLS,
3045+
len => $len,
3046+
label_x => $label_x,
3047+
label_w => $lw,
3048+
rflags_size => $rflags_size,
3049+
value_pos => $FIELD_VALUE_POS,
3050+
gap => $FIELD_VALUE_GAP,
3051+
auto => $auto,
3052+
}
3053+
);
3054+
my $val_x = $geom->{val_x};
3055+
my $label_w = $geom->{label_w};
3056+
my $wrap_rows = $geom->{wrap_rows};
3057+
my $dots_x = $geom->{dots_x};
3058+
my $lvald_x = $geom->{lvald_x};
3059+
my $rvald_x = $geom->{rvald_x};
3060+
my $rflags_x = $geom->{rflags_x};
3061+
trace(
3062+
"do_form: wrapped label \"$id\" over $wrap_rows line(s)",
3063+
$LOG_NORMAL
3064+
) if $wrap_rows;
30613065
$form{fields}[$i]{wrap_rows} = $wrap_rows;
30623066

3063-
my $lvald_x = $val_x - 1;
3064-
my $rvald_x = $val_x + $len;
3065-
my $rflags_x = $COLS - $rflags_size;
3066-
30673067
# Advance to this field's top row, breaking to a new page if the
30683068
# whole (possibly multi-row) block would not fit on this one.
3069-
my $block_h = $wrap_rows ? $wrap_rows + 1 : 1;
3070-
$y += $form{fields}[$i]{vtab};
3071-
$y = 0
3072-
if $y >= $mwinr
3073-
or ( $y > 0 and $y + $block_h > $mwinr );
3074-
my $vr = $y + $wrap_rows; # row of the value and its markers
3069+
my $pg = CCFE::Layout::page_advance(
3070+
{
3071+
y => $y,
3072+
vtab => $form{fields}[$i]{vtab},
3073+
wrap_rows => $wrap_rows,
3074+
mwinr => $mwinr,
3075+
}
3076+
);
3077+
$y = $pg->{y};
3078+
my $vr = $pg->{vr}; # row of the value and its markers
30753079

30763080
$field =
30773081
$wrap_rows
@@ -3341,31 +3345,41 @@ sub do_form {
33413345
my $lw = disp_width($label); # label width in columns
33423346
my ( $val_x, $dots_x, $lvald_x, $rvald_x, $rflags_x, $label_w );
33433347
if ($reflow) {
3344-
$val_x = $COLS - $len - 1 - $rflags_size;
3345-
$val_x = $label_x + 1 if $val_x < $label_x + 1;
3346-
$wr = 0;
3347-
$label_w = $lw;
3348-
if ( $label_x + $lw + $FIELD_VALUE_GAP > $val_x ) {
3349-
$label_w = $COLS - $label_x - $rflags_size - 1;
3350-
$label_w = 1 if $label_w < 1;
3351-
$wr = int( ( $lw + $label_w - 1 ) / $label_w );
3352-
$wr = 1 if $wr < 1;
3353-
}
3348+
# Same pure geometry do_form first laid the field out with.
3349+
my $geom = CCFE::Layout::field_geometry(
3350+
{
3351+
cols => $COLS,
3352+
len => $len,
3353+
label_x => $label_x,
3354+
label_w => $lw,
3355+
rflags_size => $rflags_size,
3356+
value_pos => $FIELD_VALUE_POS,
3357+
gap => $FIELD_VALUE_GAP,
3358+
auto => 1,
3359+
}
3360+
);
3361+
$val_x = $geom->{val_x};
3362+
$label_w = $geom->{label_w};
3363+
$wr = $geom->{wrap_rows};
3364+
$dots_x = $geom->{dots_x};
3365+
$lvald_x = $geom->{lvald_x};
3366+
$rvald_x = $geom->{rvald_x};
3367+
$rflags_x = $geom->{rflags_x};
33543368
$f->{wrap_rows} = $wr;
3355-
$dots_x = $wr ? $label_x : $label_x + $lw;
3356-
$lvald_x = $val_x - 1;
3357-
$rvald_x = $val_x + $len;
3358-
$rflags_x = $COLS - $rflags_size;
33593369
}
33603370

3361-
my $bh = $wr ? $wr + 1 : 1;
3362-
$yy += ( $f->{vtab} || 0 );
3363-
$yy = 0
3364-
if $yy >= $mwinr
3365-
or ( $yy > 0 and $yy + $bh > $mwinr );
3366-
my $page_start = ( $yy == 0 ) ? 1 : 0;
3371+
my $pg = CCFE::Layout::page_advance(
3372+
{
3373+
y => $yy,
3374+
vtab => $f->{vtab} || 0,
3375+
wrap_rows => $wr,
3376+
mwinr => $mwinr,
3377+
}
3378+
);
3379+
$yy = $pg->{y};
3380+
my $page_start = $pg->{page_start} ? 1 : 0;
33673381
$npages++ if $page_start;
3368-
my $vr = $yy + $wr;
3382+
my $vr = $pg->{vr};
33693383

33703384
if ($reflow) {
33713385
# Recreate the label (its height/width follow the wrap) ...

‎src/lib/CCFE/Layout.pm‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package CCFE::Layout;
2+
3+
# Pure form-geometry helpers (ROADMAP M7, REFACTOR.md §3).
4+
#
5+
# The value-column geometry and the page-advance arithmetic were worked out in
6+
# M6 (right-aligned "classic SMIT" value column that expands on a wide screen,
7+
# with the label wrapping onto its own line(s) when a long label on a narrow
8+
# terminal would otherwise collide with the value). That maths was written
9+
# out twice -- once when do_form() first lays the form out, and again, byte for
10+
# byte, when resize_form() reflows it for a new $LINES/$COLS. It is pure
11+
# (numbers in, numbers out: no curses, no globals), so it lives here and the
12+
# two call sites share it; the actual new_field()/move_field() calls and the
13+
# tracing stay in ccfe.pl.
14+
#
15+
# All inputs/outputs are in screen columns/rows.
16+
17+
use v5.36;
18+
19+
our $VERSION = '2.1';
20+
21+
# field_geometry(\%in) -> \%out : the value column, the (possibly wrapped)
22+
# label box and the four marker/delimiter columns for one logical field.
23+
#
24+
# in: cols current screen width ($COLS)
25+
# len the value field's width
26+
# label_x the label's left column
27+
# label_w the label's natural display width (disp_width($label))
28+
# rflags_size the right flag margin ($FIELD_RMARGIN)
29+
# value_pos configured value column, or -1 for "auto/right-align"
30+
# gap min columns to keep between label and value before wrap
31+
# auto true when the value auto-right-aligns (NORMAL layout and
32+
# value_pos == -1) -- only then can the label wrap
33+
#
34+
# out: val_x the value field's left column
35+
# wrap_rows label height-1 (0 = label shares the value's row)
36+
# label_w the label box width (full width when wrapped)
37+
# dots_x where the dot leader / label-on-value-row starts
38+
# lvald_x left value-delimiter column
39+
# rvald_x right value-delimiter column
40+
# rflags_x right flag-marker column
41+
sub field_geometry ($in) {
42+
my $cols = $in->{cols};
43+
my $len = $in->{len};
44+
my $label_x = $in->{label_x};
45+
my $lw = $in->{label_w};
46+
my $rflags = $in->{rflags_size};
47+
48+
my $val_x = $in->{value_pos};
49+
if ( $val_x == -1 ) {
50+
$val_x = $cols - $len - 1 - $rflags;
51+
$val_x = $label_x + 1 if $val_x < $label_x + 1;
52+
}
53+
54+
my $label_w = $lw;
55+
my $wrap_rows = 0;
56+
if ( $in->{auto} and $label_x + $lw + $in->{gap} > $val_x ) {
57+
$label_w = $cols - $label_x - $rflags - 1;
58+
$label_w = 1 if $label_w < 1;
59+
$wrap_rows = int( ( $lw + $label_w - 1 ) / $label_w );
60+
$wrap_rows = 1 if $wrap_rows < 1;
61+
}
62+
63+
return {
64+
val_x => $val_x,
65+
wrap_rows => $wrap_rows,
66+
label_w => $label_w,
67+
dots_x => $wrap_rows ? $label_x : $label_x + $lw,
68+
lvald_x => $val_x - 1,
69+
rvald_x => $val_x + $len,
70+
rflags_x => $cols - $rflags,
71+
};
72+
}
73+
74+
# page_advance(\%in) -> \%out : place one field's block on the current page,
75+
# breaking to a new page when the whole (label + value) block will not fit.
76+
#
77+
# in: y the running top row
78+
# vtab this field's extra leading rows
79+
# wrap_rows from field_geometry (block is wrap_rows+1 rows, else 1)
80+
# mwinr the form sub-window's row count
81+
#
82+
# out: y this field's top row (0 after a page break)
83+
# vr the value/marker row (y + wrap_rows)
84+
# page_start true when this field opens a new page (y == 0)
85+
sub page_advance ($in) {
86+
my $mwinr = $in->{mwinr};
87+
my $wrap_rows = $in->{wrap_rows};
88+
my $block_h = $wrap_rows ? $wrap_rows + 1 : 1;
89+
90+
my $y = $in->{y} + $in->{vtab};
91+
$y = 0 if $y >= $mwinr or ( $y > 0 and $y + $block_h > $mwinr );
92+
93+
return {
94+
y => $y,
95+
vr => $y + $wrap_rows,
96+
page_start => ( $y == 0 ),
97+
};
98+
}
99+
100+
1;

‎src/t/17-layout.t‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
#!/usr/bin/perl
2+
#
3+
# Unit tests for the pure CCFE::Layout form-geometry helpers (ROADMAP M7).
4+
#
5+
# The value-column geometry and the page-advance arithmetic (worked out in M6)
6+
# were written out twice in ccfe.pl -- once in do_form's initial layout and
7+
# again, byte for byte, in resize_form's reflow. That maths is pure, so it now
8+
# lives in CCFE::Layout and the two sites share it. These drive the helpers
9+
# directly with hand-computed expectations; t/10-resize.t and t/11-layout.t
10+
# remain the on-screen integration check.
11+
#
12+
use strict;
13+
use warnings;
14+
use FindBin qw($Bin);
15+
use lib "$Bin/../lib";
16+
use Test::More;
17+
18+
require_ok('CCFE::Layout') or BAIL_OUT('cannot load CCFE::Layout');
19+
20+
# ---- field_geometry: right-aligned value, short label (no wrap) ---------
21+
my $g = CCFE::Layout::field_geometry(
22+
{
23+
cols => 80, len => 20, label_x => 2, label_w => 10,
24+
rflags_size => 2, value_pos => -1, gap => 4, auto => 1,
25+
}
26+
);
27+
is( $g->{val_x}, 57, 'wide screen: value right-aligned (80-20-1-2)' );
28+
is( $g->{wrap_rows}, 0, ' short label does not wrap' );
29+
is( $g->{label_w}, 10, ' label keeps its natural width' );
30+
is( $g->{dots_x}, 12, ' dots start after the label (label_x+lw)' );
31+
is( $g->{lvald_x}, 56, ' left value delimiter is val_x-1' );
32+
is( $g->{rvald_x}, 77, ' right value delimiter is val_x+len' );
33+
is( $g->{rflags_x}, 78, ' right flag column is cols-rflags' );
34+
35+
# ---- field_geometry: long label wraps onto its own lines ----------------
36+
$g = CCFE::Layout::field_geometry(
37+
{
38+
cols => 40, len => 20, label_x => 2, label_w => 70,
39+
rflags_size => 2, value_pos => -1, gap => 4, auto => 1,
40+
}
41+
);
42+
is( $g->{val_x}, 17, 'narrow screen: value still right-aligned (40-20-1-2)' );
43+
is( $g->{label_w}, 35, ' wrapped label box is full width (cols-label_x-rflags-1)' );
44+
is( $g->{wrap_rows}, 2, ' 70-col label over a 35-col box wraps to 2 rows' );
45+
is( $g->{dots_x}, 2, ' wrapped: dots run from the label margin' );
46+
47+
# ---- field_geometry: value clamped to just past the label ---------------
48+
$g = CCFE::Layout::field_geometry(
49+
{
50+
cols => 30, len => 25, label_x => 2, label_w => 5,
51+
rflags_size => 2, value_pos => -1, gap => 4, auto => 1,
52+
}
53+
);
54+
is( $g->{val_x}, 3, 'value never sits left of label_x+1 (clamped up)' );
55+
56+
# ---- field_geometry: explicit value_pos, no auto-wrap -------------------
57+
$g = CCFE::Layout::field_geometry(
58+
{
59+
cols => 80, len => 20, label_x => 2, label_w => 10,
60+
rflags_size => 2, value_pos => 40, gap => 4, auto => 0,
61+
}
62+
);
63+
is( $g->{val_x}, 40, 'explicit value_pos is honoured verbatim' );
64+
is( $g->{wrap_rows}, 0, ' no wrap when not auto' );
65+
66+
# ---- field_geometry: a long label does NOT wrap unless auto -------------
67+
$g = CCFE::Layout::field_geometry(
68+
{
69+
cols => 80, len => 20, label_x => 2, label_w => 60,
70+
rflags_size => 2, value_pos => -1, gap => 4, auto => 0,
71+
}
72+
);
73+
is( $g->{wrap_rows}, 0, 'auto=0 suppresses wrapping even for a long label' );
74+
is( $g->{dots_x}, 62, ' dots stay after the (unwrapped) label' );
75+
76+
# ---- page_advance: first field opens page 1 -----------------------------
77+
my $p = CCFE::Layout::page_advance(
78+
{ y => 0, vtab => 0, wrap_rows => 0, mwinr => 20 } );
79+
is( $p->{y}, 0, 'first field sits at row 0' );
80+
is( $p->{vr}, 0, ' value row is 0' );
81+
ok( $p->{page_start}, ' and it starts a new page' );
82+
83+
# ---- page_advance: a field that fits stays on the page ------------------
84+
$p = CCFE::Layout::page_advance(
85+
{ y => 5, vtab => 0, wrap_rows => 0, mwinr => 20 } );
86+
is( $p->{y}, 5, 'a fitting field keeps its row' );
87+
ok( !$p->{page_start}, ' and does not start a page' );
88+
89+
# ---- page_advance: a block that would overflow breaks to a new page ------
90+
$p = CCFE::Layout::page_advance(
91+
{ y => 19, vtab => 0, wrap_rows => 2, mwinr => 20 } );
92+
is( $p->{y}, 0, 'a 3-row block at row 19 of 20 breaks to a new page' );
93+
ok( $p->{page_start}, ' new page started' );
94+
95+
# ---- page_advance: vtab pushes the field down ---------------------------
96+
$p = CCFE::Layout::page_advance(
97+
{ y => 3, vtab => 2, wrap_rows => 1, mwinr => 20 } );
98+
is( $p->{y}, 5, 'vtab adds leading rows (3+2)' );
99+
is( $p->{vr}, 6, ' value row is y + wrap_rows' );
100+
101+
# ---- page_advance: y at/over the window height wraps to a new page -------
102+
$p = CCFE::Layout::page_advance(
103+
{ y => 20, vtab => 0, wrap_rows => 0, mwinr => 20 } );
104+
is( $p->{y}, 0, 'y == mwinr forces a page break' );
105+
106+
done_testing();

0 commit comments

Comments
 (0)