Add a rules screen that finds and removes dead rules

Nothing showed whether a rule was still earning its place. The new screen, on 5, lists every rule in file order with the number of transactions it claims, marking those that claim none. The count comes from Engine.Usage, which counts by first match, so a rule shadowed by an earlier one reports zero even though its glob matches. That is the case worth catching: such a rule looks correct in isolation and can never fire. d removes the selected rule and p removes every unused one, each behind a y/n confirmation since this rewrites a hand-maintained file. config.DeleteRules edits rules.toml textually rather than re-serialising the parsed rules, so comments and layout survive; a comment directly above a rule goes with it, while one separated by a blank line is left as a heading. The result is re-parsed before it replaces the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-09 02:02:01 +02:00
parent aeca203855
commit c2eba53fd4
8 changed files with 831 additions and 9 deletions
@@ -95,13 +95,135 @@ func AppendRule(root string, r Rule) error {
 	b.WriteString("\n")
 	b.WriteString(formatRule(r))

-	tmp, err := os.CreateTemp(root, ".rules-*.toml")
+	return writeFileAtomic(root, path, b.String())
+}
+
+// DeleteRules removes the rules at the given positions (0-based, as loaded by
+// LoadRules) from rules.toml.
+//
+// The file is edited textually rather than re-serialised from the parsed
+// rules, so comments, ordering and formatting the user put there by hand
+// survive. A comment block sitting directly above a deleted rule goes with it,
+// since it documents that rule; a comment separated by a blank line is treated
+// as a section heading and left alone.
+func DeleteRules(root string, positions []int) (int, error) {
+	if len(positions) == 0 {
+		return 0, nil
+	}
+	doomed := map[int]bool{}
+	for _, p := range positions {
+		doomed[p] = true
+	}
+
+	path := filepath.Join(root, RulesFile)
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		return 0, fmt.Errorf("read %s: %w", path, err)
+	}
+	lines := strings.Split(string(raw), "\n")
+
+	// Where each [[rule]] block begins.
+	var starts []int
+	for i, line := range lines {
+		if strings.TrimSpace(line) == "[[rule]]" {
+			starts = append(starts, i)
+		}
+	}
+	for _, p := range positions {
+		if p < 0 || p >= len(starts) {
+			return 0, fmt.Errorf("rule %d is out of range; %s holds %d rules", p+1, path, len(starts))
+		}
+	}
+
+	// A rule owns the run of comment lines directly above it, with no blank
+	// line in between. Anything further up is a heading for what follows.
+	prefix := func(k int) int {
+		i := starts[k]
+		for i > 0 && strings.HasPrefix(strings.TrimSpace(lines[i-1]), "#") {
+			i--
+		}
+		return i
+	}
+
+	drop := map[int]bool{}
+	for k := range starts {
+		if !doomed[k] {
+			continue
+		}
+		// The block runs up to the next rule's comment prefix, so a comment
+		// introducing the following rule is not swept up with this one.
+		end := len(lines)
+		if k+1 < len(starts) {
+			end = prefix(k + 1)
+		}
+		for i := prefix(k); i < end; i++ {
+			drop[i] = true
+		}
+		// Blank lines are the gap between rules, not part of either; leaving
+		// them avoids gluing the neighbours together.
+		for i := end - 1; i >= starts[k] && strings.TrimSpace(lines[i]) == ""; i-- {
+			delete(drop, i)
+		}
+	}
+
+	kept := make([]string, 0, len(lines))
+	for i, line := range lines {
+		if !drop[i] {
+			kept = append(kept, line)
+		}
+	}
+	out := collapseBlankRuns(kept)
+
+	// Never write something that will not load again.
+	var check Rules
+	if _, err := toml.Decode(out, &check); err != nil {
+		return 0, fmt.Errorf("deleting from %s would produce invalid TOML: %w", path, err)
+	}
+	if want := len(starts) - len(doomed); len(check.Rule) != want {
+		return 0, fmt.Errorf("deleting from %s would leave %d rules, expected %d",
+			path, len(check.Rule), want)
+	}
+
+	if err := writeFileAtomic(root, path, out); err != nil {
+		return 0, err
+	}
+	return len(doomed), nil
+}
+
+// collapseBlankRuns squeezes the runs of blank lines that deletion leaves
+// behind down to one.
+func collapseBlankRuns(lines []string) string {
+	out := make([]string, 0, len(lines))
+	blank := false
+	for _, line := range lines {
+		if strings.TrimSpace(line) == "" {
+			if blank {
+				continue
+			}
+			blank = true
+		} else {
+			blank = false
+		}
+		out = append(out, line)
+	}
+	// Drop leading blank lines outright.
+	for len(out) > 0 && strings.TrimSpace(out[0]) == "" {
+		out = out[1:]
+	}
+	text := strings.Join(out, "\n")
+	return strings.TrimRight(text, "\n") + "\n"
+}
+
+// writeFileAtomic replaces path via a temporary file in the same directory, so
+// a failure part-way cannot truncate the user's config.
+func writeFileAtomic(dir, path, content string) error {
+	tmp, err := os.CreateTemp(dir, ".rules-*.toml")
 	if err != nil {
 		return fmt.Errorf("write %s: %w", path, err)
 	}
 	defer os.Remove(tmp.Name())

-	if _, err := tmp.WriteString(b.String()); err != nil {
+	if _, err := tmp.WriteString(content); err != nil {
 		tmp.Close()
 		return fmt.Errorf("write %s: %w", path, err)
 	}
@@ -3,6 +3,7 @@ package config
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )

@@ -195,6 +196,164 @@ func TestAppendRuleQuotesValues(t *testing.T) {
 	}
 }

+const rulesWithComments = `# Rules for my accounts.
+# Order matters: the first match wins.
+
+# Weekly shop.
+[[rule]]
+match = "*LIDL*"
+tag   = "groceries"
+
+[[rule]]
+match = "*PAYROLL*"
+tag   = "salary"
+
+# Moving money to myself.
+[[rule]]
+match    = "*TO SAVINGS*"
+transfer = true
+tag      = "transfer"
+`
+
+// Deleting must edit the file textually: a user's comments and layout are not
+// recoverable from the parsed rules.
+func TestDeleteRulesKeepsCommentsAndOrder(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, RulesFile)
+	if err := os.WriteFile(path, []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	n, err := DeleteRules(root, []int{1}) // the payroll rule, which has no comment
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n != 1 {
+		t.Errorf("deleted %d rules, want 1", n)
+	}
+
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded.Rule) != 2 {
+		t.Fatalf("got %d rules, want 2: %+v", len(loaded.Rule), loaded.Rule)
+	}
+	if loaded.Rule[0].Match != "*LIDL*" || loaded.Rule[1].Match != "*TO SAVINGS*" {
+		t.Errorf("surviving rules = %+v, want the order preserved", loaded.Rule)
+	}
+
+	body, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	text := string(body)
+	for _, want := range []string{
+		"# Rules for my accounts.",
+		"# Order matters: the first match wins.",
+		"# Weekly shop.",
+		"# Moving money to myself.",
+	} {
+		if !strings.Contains(text, want) {
+			t.Errorf("comment %q was lost:\n%s", want, text)
+		}
+	}
+	if strings.Contains(text, "PAYROLL") {
+		t.Errorf("the deleted rule is still there:\n%s", text)
+	}
+}
+
+// A comment directly above a rule documents it and goes with it; a heading
+// separated by a blank line does not.
+func TestDeleteRulesTakesAttachedComment(t *testing.T) {
+	root := t.TempDir()
+	if err := os.WriteFile(filepath.Join(root, RulesFile), []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := DeleteRules(root, []int{0}); err != nil { // the commented LIDL rule
+		t.Fatal(err)
+	}
+	body, _ := os.ReadFile(filepath.Join(root, RulesFile))
+	text := string(body)
+
+	if strings.Contains(text, "# Weekly shop.") {
+		t.Errorf("the rule's own comment should have gone with it:\n%s", text)
+	}
+	if !strings.Contains(text, "# Rules for my accounts.") {
+		t.Errorf("the file heading must survive:\n%s", text)
+	}
+}
+
+func TestDeleteRulesMultiple(t *testing.T) {
+	root := t.TempDir()
+	if err := os.WriteFile(filepath.Join(root, RulesFile), []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	n, err := DeleteRules(root, []int{0, 2})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n != 2 {
+		t.Errorf("deleted %d, want 2", n)
+	}
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded.Rule) != 1 || loaded.Rule[0].Match != "*PAYROLL*" {
+		t.Errorf("remaining = %+v, want only the payroll rule", loaded.Rule)
+	}
+}
+
+func TestDeleteRulesAll(t *testing.T) {
+	root := t.TempDir()
+	if err := os.WriteFile(filepath.Join(root, RulesFile), []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := DeleteRules(root, []int{0, 1, 2}); err != nil {
+		t.Fatal(err)
+	}
+	loaded, err := LoadRules(root)
+	if err != nil {
+		t.Fatalf("emptying the file must leave it loadable: %v", err)
+	}
+	if len(loaded.Rule) != 0 {
+		t.Errorf("got %+v, want no rules", loaded.Rule)
+	}
+}
+
+func TestDeleteRulesOutOfRange(t *testing.T) {
+	root := t.TempDir()
+	if err := os.WriteFile(filepath.Join(root, RulesFile), []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := DeleteRules(root, []int{7}); err == nil {
+		t.Error("expected an error for an out-of-range position")
+	}
+	loaded, _ := LoadRules(root)
+	if len(loaded.Rule) != 3 {
+		t.Error("a rejected delete must leave the file untouched")
+	}
+}
+
+// Deleting nothing is a no-op, not a rewrite.
+func TestDeleteRulesEmptySelection(t *testing.T) {
+	root := t.TempDir()
+	path := filepath.Join(root, RulesFile)
+	if err := os.WriteFile(path, []byte(rulesWithComments), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if n, err := DeleteRules(root, nil); err != nil || n != 0 {
+		t.Fatalf("n=%d err=%v, want 0 and no error", n, err)
+	}
+	body, _ := os.ReadFile(path)
+	if string(body) != rulesWithComments {
+		t.Error("the file was rewritten despite deleting nothing")
+	}
+}
+
 func TestUserConfigPathFollowsXDG(t *testing.T) {
 	t.Setenv("XDG_CONFIG_HOME", "/custom/config")
 	path, err := UserConfigPath()