From 1af9ca1cd669c2d2d82e314951873ce74dffdc60 Mon Sep 17 00:00:00 2001
From: Amolith <amolith@secluded.site>
Date: Mon, 30 Mar 2026 16:34:08 -0600
Subject: [PATCH] Fix pre-existing review findings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address issues found by CodeRabbit in pre-existing code:

- Fix FormatSnapshotLine doc comment to note that both paths and
tags are omitted when empty, not just tags
- Fix misleading "resolveFixtureTOML-style" comment in snapshot
tests — the fixtures are JSON, not TOML
- Remove redundant tt := tt loop variable captures in exec_test.go
and list_snapshots_test.go (unnecessary since Go 1.22)
- Reject whitespace-only input in notEmpty validator by trimming
before checking; update the test expectation to match
---
 internal/form/form.go                  | 5 +++--
 internal/form/form_test.go             | 2 +-
 internal/restic/exec_test.go           | 2 --
 internal/restic/list_snapshots_test.go | 1 -
 internal/restic/snapshots.go           | 3 ++-
 internal/restic/snapshots_test.go      | 4 ++--
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/internal/form/form.go b/internal/form/form.go
index a2a60e9a926f2b6e9b45f0074a2ae086c176f084..407448a3da2ad6435cea4304b6efb97219ad4be6 100644
--- a/internal/form/form.go
+++ b/internal/form/form.go
@@ -89,10 +89,11 @@ func BackupPaths() ([]string, error) {
 	return splitFields(raw), nil
 }
 
-// notEmpty returns a validation function that rejects blank input.
+// notEmpty returns a validation function that rejects blank or
+// whitespace-only input.
 func notEmpty(fieldName string) func(string) error {
 	return func(s string) error {
-		if len(s) == 0 {
+		if strings.TrimSpace(s) == "" {
 			return fmt.Errorf("%s is required", fieldName)
 		}
 		return nil
diff --git a/internal/form/form_test.go b/internal/form/form_test.go
index e6a54c266ad816928059b353d9ec5e4cef1b13b5..7147a9b88d5a0b70cb5191f5bda10cd0d3fd48b8 100644
--- a/internal/form/form_test.go
+++ b/internal/form/form_test.go
@@ -16,7 +16,7 @@ func TestNotEmpty(t *testing.T) {
 	}{
 		{name: "empty string", input: "", wantErr: true},
 		{name: "non-empty string", input: "hello", wantErr: false},
-		{name: "whitespace only", input: "  ", wantErr: false},
+		{name: "whitespace only", input: "  ", wantErr: true},
 	}
 
 	for _, tt := range tests {
diff --git a/internal/restic/exec_test.go b/internal/restic/exec_test.go
index 7952de755b30959e8270442eb31245f02543f777..6761338f3a2f8c5716085d45b3866b0d6c4345dd 100644
--- a/internal/restic/exec_test.go
+++ b/internal/restic/exec_test.go
@@ -44,7 +44,6 @@ func TestBuildArgv(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
@@ -234,7 +233,6 @@ func TestResolveEnvironCommands(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
diff --git a/internal/restic/list_snapshots_test.go b/internal/restic/list_snapshots_test.go
index d33de4e338eeea82f4fdd8f3e94fb0145402a528..9dad4ebcdbd0c1ee795c3f35c17fc94ce74baef8 100644
--- a/internal/restic/list_snapshots_test.go
+++ b/internal/restic/list_snapshots_test.go
@@ -134,7 +134,6 @@ func TestBuildSnapshotCmd(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
diff --git a/internal/restic/snapshots.go b/internal/restic/snapshots.go
index 926846623eb43301bcf3d9003542b98091fbc2d6..039952c1466651cec46f37afa35b0306546091eb 100644
--- a/internal/restic/snapshots.go
+++ b/internal/restic/snapshots.go
@@ -33,7 +33,8 @@ func ParseSnapshots(data []byte) ([]Snapshot, error) {
 //
 //	<short_id>  <date> <time>  <hostname>  <paths>  [<tags>]
 //
-// Tags are omitted when empty. Hostname shows "(unknown)" when blank.
+// Paths and tags are omitted when empty. Hostname shows "(unknown)"
+// when blank.
 func FormatSnapshotLine(s Snapshot) string {
 	var b strings.Builder
 
diff --git a/internal/restic/snapshots_test.go b/internal/restic/snapshots_test.go
index ca42669089e75e2dc3b8dc1955771f13c348431c..b6597ba15818bd6856fdc35d3bf6d52abefcc184 100644
--- a/internal/restic/snapshots_test.go
+++ b/internal/restic/snapshots_test.go
@@ -5,8 +5,8 @@ import (
 	"time"
 )
 
-// resolveFixtureTOML-style: embed test JSON as string constants so tests
-// are self-contained and don't depend on external files.
+// Embed test JSON as string constants so tests are self-contained and
+// don't depend on external files.
 
 const snapshotFixtureNormal = `[
   {