From e7aa75735d16757ed533f5ffa9f7cfbe121c3d55 Mon Sep 17 00:00:00 2001 From: MDietermann Date: Mon, 13 Apr 2026 22:00:49 +0200 Subject: [PATCH] refactor(tui): use strconv.Atoi in parseIntClamp, drop spurious error return --- internal/tui/wizard/wizard.go | 29 +++++++++++++++++------------ internal/tui/wizard/wizard_test.go | 18 ++++++++++-------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/internal/tui/wizard/wizard.go b/internal/tui/wizard/wizard.go index 59a1674..a6242fa 100644 --- a/internal/tui/wizard/wizard.go +++ b/internal/tui/wizard/wizard.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strconv" "strings" "github.com/charmbracelet/huh" @@ -88,12 +89,13 @@ func Run(ctx context.Context, reg *module.Registry, initial tui.WizardState) (tu return s, false, err } + // Redundant-but-safe: the DescriptionFunc closure on the plan-review + // screen already kept s.Modules in sync. The MultiSelect group is + // frozen by the time we reach here, so this is idempotent. Kept for + // the case where the form is exited via a path that bypasses the + // plan-review screen in future revisions. s.Modules = syncSelected(s.Modules, selectedModules) - // Parse parallelism: clamp to 1 on any parse failure, since the wizard - // uses a free-form Input; a stricter schema can come later. - if n, perr := parseIntClamp(parallelismStr, 1); perr == nil { - s.Parallelism = n - } + s.Parallelism = parseIntClamp(parallelismStr, 1) s.Confirmed = proceed return s, proceed, nil } @@ -149,15 +151,18 @@ func renderPlan(ctx context.Context, reg *module.Registry, selectedIDs []string) return b.String() } -// parseIntClamp parses s as int. Empty or invalid input returns fallback. -func parseIntClamp(s string, fallback int) (int, error) { +// parseIntClamp parses s as a positive int. Empty, non-numeric (including +// partial matches like "3abc"), negative, zero, or overflowing input all +// return fallback. strconv.Atoi (not fmt.Sscanf) is used so partial matches +// are rejected and overflow surfaces as an error. +func parseIntClamp(s string, fallback int) int { s = strings.TrimSpace(s) if s == "" { - return fallback, nil + return fallback } - var n int - if _, err := fmt.Sscanf(s, "%d", &n); err != nil || n < 1 { - return fallback, nil + n, err := strconv.Atoi(s) + if err != nil || n < 1 { + return fallback } - return n, nil + return n } diff --git a/internal/tui/wizard/wizard_test.go b/internal/tui/wizard/wizard_test.go index 94999fe..f2a886f 100644 --- a/internal/tui/wizard/wizard_test.go +++ b/internal/tui/wizard/wizard_test.go @@ -45,16 +45,18 @@ func TestRenderPlanEmpty(t *testing.T) { func TestParseIntClamp(t *testing.T) { cases := map[string]int{ - "": 1, - " ": 1, - "abc": 1, - "0": 1, - "-5": 1, - "3": 3, - "12": 12, + "": 1, + " ": 1, + "abc": 1, + "3abc": 1, // partial match is rejected (strconv.Atoi, not Sscanf) + "0": 1, + "-5": 1, + "3": 3, + "12": 12, + "99999999999999999999": 1, // overflow → fallback } for in, want := range cases { - got, _ := parseIntClamp(in, 1) + got := parseIntClamp(in, 1) if got != want { t.Errorf("parseIntClamp(%q) = %d, want %d", in, got, want) }