diff --git a/CLAUDE.md b/CLAUDE.md index b7a6d13..9ff09b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,18 @@ rule builder cannot shadow a rule the user wrote by hand. `viewRules`, or typing `q` would quit and `i` would start an import. Any new full-screen input needs the same treatment. +**A rule's usage count is how many transactions it wins, not how many its glob +could match** — `Engine.Usage` counts by `MatchIndex`, so a rule shadowed by an +earlier one correctly reports zero. That is what makes the rules screen able to +find dead rules at all. + +**`config.DeleteRules` edits rules.toml textually, never by re-serialising the +parsed rules**, because comments and formatting are not recoverable from +`[]Rule`. A rule owns the comment lines directly above it; a comment separated +by a blank line is a heading for what follows and stays. Both writers go +through `writeFileAtomic`, and deletion re-parses the result before replacing +the file. + ## Adding a bank parser Implement `parser.Parser` and call `parser.Register` from an `init`. Nothing diff --git a/README.md b/README.md index 9458b62..8d53098 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ imported — creating an `account.toml` is not enough on its own. Run | Key | Action | | --- | --- | -| `1` `2` `3` `4` / `tab` | accounts · transactions · report · rule builder | +| `1` `2` `3` `4` `5` / `tab` | accounts · transactions · report · rule builder · rules | | `enter` | open the selected account (accounts view) | | `t` | set the tag on the selected transaction | | `x` | toggle transfer on the selected transaction | @@ -106,6 +106,34 @@ narrows the preview to that account. Rules are appended, so anything already in `rules.toml` keeps precedence — the preview accounts for that automatically, since it only ever lists transactions no existing rule has already tagged. +### Rules (`5`) + +Every rule in file order, with the number of transactions it actually claims. +Rules that claim none are marked `✗`. + +``` +money · rules · 5 rules · 2 match nothing + +# Pattern Account Tag T Txns +1 *LIDL* (all) groceries 3 +2 ✗ *LIDL SOFIA* (all) shadowed 0 +3 ✗ *OLD BANK NAME* (all) dead 0 +4 *ZARA* (all) clothes 1 +5 *КАУФЛАНД* checking groceries 1 +``` + +The count is how many transactions the rule *wins*, not how many its glob could +match. Rule 2 above matches `LIDL SOFIA 4412` perfectly well, but rule 1 claims +it first, so rule 2 is dead weight — which is the point of the screen. A rule +can therefore reach zero either by matching nothing or by being shadowed, and +both are worth deleting. + +`d` deletes the selected rule, `p` deletes every rule marked `✗` at once, and +both ask for a `y` first. Deleting edits `rules.toml` textually, so your +comments, ordering and formatting survive; a comment sitting directly above a +deleted rule goes with it, while one separated by a blank line is treated as a +section heading and left alone. + ## rules.toml Rules are evaluated in file order and the **first match wins**, so put transfer diff --git a/internal/config/config.go b/internal/config/config.go index 6166bbf..eebdf8c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index be58689..0cb9343 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -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() diff --git a/internal/rules/rules.go b/internal/rules/rules.go index 7764225..9f0d292 100644 --- a/internal/rules/rules.go +++ b/internal/rules/rules.go @@ -23,10 +23,26 @@ func New(r *config.Rules) *Engine { return &Engine{rules: r.Rule} } +// Rules returns the ordered rules, as loaded from rules.toml. +func (e *Engine) Rules() []config.Rule { return e.rules } + // Match returns the first rule matching a transaction on the given account, or -// nil if none does. Every pattern a rule sets must match: a rule with both -// match and counterparty is an "and", not an "or". +// nil if none does. func (e *Engine) Match(accountSlug string, t model.Transaction) *config.Rule { + if i := e.MatchIndex(accountSlug, t); i >= 0 { + return &e.rules[i] + } + return nil +} + +// MatchIndex returns the position of the first rule matching a transaction, or +// -1 if none does. Every pattern a rule sets must match: a rule with both +// match and counterparty is an "and", not an "or". +// +// The position matters as well as the rule: because the first match wins, a +// rule that is fully shadowed by an earlier one never applies to anything, and +// only the index reveals that. +func (e *Engine) MatchIndex(accountSlug string, t model.Transaction) int { var ( description = model.NormalizeDescription(t.Description) counterparty = model.NormalizeDescription(t.Counterparty) @@ -46,9 +62,22 @@ func (e *Engine) Match(accountSlug string, t model.Transaction) *config.Rule { if r.Type != "" && !glob.Match(r.Type, kind) { continue } - return r + return i } - return nil + return -1 +} + +// Usage counts how many transactions each rule actually claims. A rule with a +// count of zero is dead: either nothing matches it, or an earlier rule takes +// everything it would have caught. +func (e *Engine) Usage(txns []model.Transaction) []int { + counts := make([]int, len(e.rules)) + for _, t := range txns { + if i := e.MatchIndex(t.AccountSlug, t); i >= 0 { + counts[i]++ + } + } + return counts } // ApplyTxn returns the tag and transfer flag for a transaction. An unmatched diff --git a/internal/rules/rules_test.go b/internal/rules/rules_test.go index 19bcd4e..d7fdadd 100644 --- a/internal/rules/rules_test.go +++ b/internal/rules/rules_test.go @@ -139,6 +139,52 @@ func TestFirstMatchWins(t *testing.T) { } } +// A rule can be dead two ways: nothing matches it, or an earlier rule already +// claimed everything it would have caught. Usage must report both as zero. +func TestUsageCountsFirstMatchOnly(t *testing.T) { + engine := New(&config.Rules{Rule: []config.Rule{ + {Match: "*LIDL*", Tag: "groceries"}, // claims both LIDL rows + {Match: "*LIDL SOFIA*", Tag: "shadowed"}, // fully shadowed by the above + {Match: "*NOTHING MATCHES ME*", Tag: "no"}, // matches nothing at all + {Match: "*PAYROLL*", Tag: "salary"}, // claims one row + }}) + + txns := []model.Transaction{ + {AccountSlug: "checking", Description: "LIDL SOFIA 4412"}, + {AccountSlug: "checking", Description: "LIDL VARNA 9911"}, + {AccountSlug: "checking", Description: "ACME PAYROLL"}, + {AccountSlug: "checking", Description: "UNMATCHED SHOP"}, + } + + usage := engine.Usage(txns) + want := []int{2, 0, 0, 1} + if len(usage) != len(want) { + t.Fatalf("usage has %d entries, want %d", len(usage), len(want)) + } + for i := range want { + if usage[i] != want[i] { + t.Errorf("rule %d used by %d transactions, want %d", i+1, usage[i], want[i]) + } + } +} + +func TestMatchIndex(t *testing.T) { + engine := New(&config.Rules{Rule: []config.Rule{ + {Match: "*LIDL EXPRESS*", Tag: "snacks"}, + {Match: "*LIDL*", Tag: "groceries"}, + }}) + + if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL EXPRESS 1"}); got != 0 { + t.Errorf("index = %d, want 0", got) + } + if got := engine.MatchIndex("checking", model.Transaction{Description: "LIDL 1"}); got != 1 { + t.Errorf("index = %d, want 1", got) + } + if got := engine.MatchIndex("checking", model.Transaction{Description: "OTHER"}); got != -1 { + t.Errorf("index = %d, want -1 for no match", got) + } +} + func TestAccountScopedRule(t *testing.T) { engine := New(&config.Rules{Rule: []config.Rule{ {Match: "*TRANSFER*", Tag: "transfer", Transfer: true, Account: "savings"}, diff --git a/internal/tui/tui.go b/internal/tui/tui.go index aafb801..0e6962a 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -29,8 +29,20 @@ const ( viewTxns viewReport viewRules + viewRuleList - viewCount = 4 + viewCount = 5 +) + +// confirmation is a pending destructive action awaiting a y/n answer. +// Deleting rewrites a file the user maintains by hand, so it is never done on +// a single keypress. +type confirmation int + +const ( + confirmNone confirmation = iota + confirmDeleteRule + confirmPruneRules ) // input is the modal state: the transaction list is read-only until the user @@ -76,6 +88,12 @@ type Model struct { untagged []descGroup ruleMatches int // untagged descriptions the current glob matches + // Rule list: every rule with the number of transactions it actually + // claims, so dead ones can be found and removed. + ruleListTable table.Model + ruleUsage []int + confirm confirmation + filter store.Filter onlyUntagged bool @@ -174,6 +192,15 @@ func New(root string, db *store.DB, accounts []*config.Account, engine *rules.En {Title: "Untagged description", Width: 44}, {Title: "N", Width: 4}, }), + ruleListTable: newTable([]table.Column{ + {Title: "#", Width: 3}, + {Title: " ", Width: 1}, + {Title: "Pattern", Width: 34}, + {Title: "Account", Width: 12}, + {Title: "Tag", Width: 14}, + {Title: "T", Width: 1}, + {Title: "Txns", Width: 6}, + }), reportTable: newTable([]table.Column{ {Title: "Tag", Width: 20}, {Title: "Cur", Width: 4}, @@ -435,6 +462,190 @@ func (m *Model) knownAccount(slug string) bool { return false } +// reloadRuleList counts, for every rule, how many transactions it actually +// claims. A rule can match nothing because no description fits it, or because +// an earlier rule already took everything it would have caught; both show up +// here as a zero. +func (m *Model) reloadRuleList() error { + txns, err := m.db.Transactions(store.Filter{}) + if err != nil { + return err + } + rs := m.engine.Rules() + m.ruleUsage = m.engine.Usage(txns) + + rows := make([]table.Row, 0, len(rs)) + for i, r := range rs { + marker := " " + if m.ruleUsage[i] == 0 { + marker = "✗" + } + transfer := "" + if r.Transfer { + transfer = "T" + } + account := r.Account + if account == "" { + account = "(all)" + } + rows = append(rows, table.Row{ + fmt.Sprintf("%d", i+1), marker, rulePattern(r), account, r.Tag, transfer, + fmt.Sprintf("%d", m.ruleUsage[i]), + }) + } + + cursor := m.ruleListTable.Cursor() + m.ruleListTable.SetRows(rows) + if cursor >= len(rows) { + cursor = len(rows) - 1 + } + if cursor < 0 { + cursor = 0 + } + m.ruleListTable.SetCursor(cursor) + return nil +} + +// rulePattern renders whichever patterns a rule sets, labelled so a +// counterparty or type rule is not mistaken for a description one. +func rulePattern(r config.Rule) string { + var parts []string + if r.Match != "" { + parts = append(parts, r.Match) + } + if r.Counterparty != "" { + parts = append(parts, "counterparty:"+r.Counterparty) + } + if r.Type != "" { + parts = append(parts, "type:"+r.Type) + } + return strings.Join(parts, " + ") +} + +// unusedRules lists the positions of every rule claiming no transactions. +func (m *Model) unusedRules() []int { + var out []int + for i, n := range m.ruleUsage { + if n == 0 { + out = append(out, i) + } + } + return out +} + +// deleteRules removes rules from rules.toml, then reloads and retags so the +// counts on screen reflect the new file. +func (m *Model) deleteRules(positions []int) error { + n, err := config.DeleteRules(m.root, positions) + if err != nil { + return err + } + + loaded, err := config.LoadRules(m.root) + if err != nil { + return fmt.Errorf("rules deleted, but re-reading rules.toml failed: %w", err) + } + m.engine = rules.New(loaded) + + retagged, err := m.engine.Retag(m.db) + if err != nil { + return err + } + m.status = fmt.Sprintf("deleted %d rule(s), %d transactions retagged", n, retagged) + + if err := m.reload(); err != nil { + return err + } + return m.reloadRuleList() +} + +// openRuleList switches to the rule list, remembering where to return to. +func (m *Model) openRuleList() { + if m.view != viewRuleList { + m.ruleReturn = m.view + } + m.view = viewRuleList + m.confirm = confirmNone + if err := m.reloadRuleList(); err != nil { + m.err = err + } +} + +// updateRuleList drives the rule list, including the delete confirmations. +func (m *Model) updateRuleList(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + if m.confirm != confirmNone { + pending := m.confirm + m.confirm = confirmNone + if msg.String() != "y" { + m.status = "cancelled" + return m, nil + } + m.err = nil + switch pending { + case confirmDeleteRule: + if i := m.ruleListTable.Cursor(); i >= 0 && i < len(m.ruleUsage) { + if err := m.deleteRules([]int{i}); err != nil { + m.err = err + } + } + case confirmPruneRules: + if err := m.deleteRules(m.unusedRules()); err != nil { + m.err = err + } + } + return m, nil + } + + switch msg.String() { + case "q", "ctrl+c": + return m, tea.Quit + case "esc": + m.view = m.ruleReturn + return m, nil + case "1": + m.view = viewAccounts + return m, nil + case "2": + m.view = viewTxns + return m, nil + case "3": + m.view = viewReport + return m, nil + case "4": + return m, m.openRuleBuilder() + + case "d": + i := m.ruleListTable.Cursor() + rs := m.engine.Rules() + if i < 0 || i >= len(rs) { + return m, nil + } + m.confirm = confirmDeleteRule + m.status = fmt.Sprintf("delete rule %d (%s → %s), used by %d transactions? y/n", + i+1, rulePattern(rs[i]), rs[i].Tag, m.ruleUsage[i]) + return m, nil + + case "p": + unused := m.unusedRules() + if len(unused) == 0 { + m.status = "no unused rules to prune" + return m, nil + } + m.confirm = confirmPruneRules + m.status = fmt.Sprintf("delete all %d rules that match nothing? y/n", len(unused)) + return m, nil + + case "r": + m.err = m.reloadRuleList() + m.status = "counts refreshed" + return m, nil + } + + var cmd tea.Cmd + m.ruleListTable, cmd = m.ruleListTable.Update(msg) + return m, cmd +} + // openRuleBuilder switches to the rule builder, remembering where to return to // and seeding the account field from whatever account is being browsed. func (m *Model) openRuleBuilder() tea.Cmd { @@ -525,6 +736,9 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.view == viewRules { return m.updateRules(msg) } + if m.view == viewRuleList { + return m.updateRuleList(msg) + } return m.updateNormal(msg) } return m, nil @@ -620,6 +834,7 @@ func (m *Model) resize() { m.txnTable.SetHeight(h) m.reportTable.SetHeight(h) m.ruleTable.SetHeight(h) + m.ruleListTable.SetHeight(h) // The preview list gets whatever the form does not use. if m.width > 0 { @@ -711,10 +926,16 @@ func (m *Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m, nil case "4": return m, m.openRuleBuilder() + case "5": + m.openRuleList() + return m, nil case "tab": m.view = (m.view + 1) % viewCount - if m.view == viewRules { + switch m.view { + case viewRules: return m, m.openRuleBuilder() + case viewRuleList: + m.openRuleList() } return m, nil @@ -871,6 +1092,8 @@ func (m *Model) View() string { b.WriteString(m.reportTable.View()) case viewRules: b.WriteString(m.rulesView()) + case viewRuleList: + b.WriteString(m.ruleListTable.View()) } } b.WriteString("\n") @@ -971,6 +1194,12 @@ func (m *Model) emptyMessage() string { return "" } return "Nothing to report yet.\n\nPress i to import some statements first." + + case viewRuleList: + if len(m.ruleListTable.Rows()) > 0 { + return "" + } + return "No rules yet.\n\nPress 4 to build one, or write rules.toml by hand." } return "" } @@ -992,6 +1221,12 @@ func (m *Model) title() string { return "money · accounts" case viewRules: return "money · rule builder · writes to rules.toml" + case viewRuleList: + unused := len(m.unusedRules()) + if unused == 0 { + return fmt.Sprintf("money · rules · %d rules, all in use", len(m.engine.Rules())) + } + return fmt.Sprintf("money · rules · %d rules · %d match nothing", len(m.engine.Rules()), unused) case viewReport: return "money · report · " + scope + " · transfers excluded" default: @@ -1020,6 +1255,11 @@ func (m *Model) help() string { return "enter open · 2 transactions · 3 report · 4 rules · i import · r retag · q quit" case viewRules: return "tab/↑↓ field · pgup/pgdn scroll list · enter save rule · esc back · ctrl+c quit" + case viewRuleList: + if m.confirm != confirmNone { + return "y confirm · any other key cancels" + } + return "d delete rule · p prune all unused · r refresh counts · 4 rule builder · esc back · q quit" case viewReport: return "1 accounts · 2 transactions · 4 rules · u untagged · a all accounts · q quit" default: diff --git a/internal/tui/tui_test.go b/internal/tui/tui_test.go index 05a0e52..08eff12 100644 --- a/internal/tui/tui_test.go +++ b/internal/tui/tui_test.go @@ -654,6 +654,192 @@ func TestRuleBuilderTabCyclesFieldsAndEscLeaves(t *testing.T) { } } +// newRuleListModel seeds a data root whose rules.toml holds one useful rule, +// one shadowed rule and one that matches nothing. +func newRuleListModel(t *testing.T) (*Model, string) { + t.Helper() + m, _, root := newRuleModel(t) + + rulesTOML := `# My rules. + +# The weekly shop. +[[rule]] +match = "*LIDL*" +tag = "groceries" + +[[rule]] +match = "*LIDL SOFIA*" +tag = "shadowed" + +[[rule]] +match = "*NEVER MATCHES ANYTHING*" +tag = "dead" + +[[rule]] +match = "*ZARA*" +tag = "clothes" +` + if err := os.WriteFile(filepath.Join(root, config.RulesFile), []byte(rulesTOML), 0o644); err != nil { + t.Fatal(err) + } + loaded, err := config.LoadRules(root) + if err != nil { + t.Fatal(err) + } + m.engine = rules.New(loaded) + if _, err := m.engine.Retag(m.db); err != nil { + t.Fatal(err) + } + key(t, m, "5") + return m, root +} + +func TestRuleListCountsUsage(t *testing.T) { + m, _ := newRuleListModel(t) + + if m.view != viewRuleList { + t.Fatal("expected 5 to open the rule list") + } + + // LIDL SOFIA x2 plus LIDL VARNA; the shadowed and dead rules claim nothing. + want := []int{3, 0, 0, 0} + if len(m.ruleUsage) != len(want) { + t.Fatalf("usage = %v, want %d entries", m.ruleUsage, len(want)) + } + for i := range want { + if m.ruleUsage[i] != want[i] { + t.Errorf("rule %d used by %d, want %d", i+1, m.ruleUsage[i], want[i]) + } + } + + // Unused rules are marked so they can be picked out at a glance. + rows := m.ruleListTable.Rows() + if rows[0][1] != " " { + t.Errorf("the used rule is marked %q, want no marker", rows[0][1]) + } + for _, i := range []int{1, 2, 3} { + if rows[i][1] != "✗" { + t.Errorf("rule %d marker = %q, want ✗", i+1, rows[i][1]) + } + } + + if !strings.Contains(m.View(), "3 match nothing") { + t.Errorf("expected the title to count the dead rules:\n%s", m.View()) + } +} + +// Deleting rewrites a hand-maintained file, so it takes a confirmation. +func TestRuleListDeleteNeedsConfirmation(t *testing.T) { + m, root := newRuleListModel(t) + + m.ruleListTable.SetCursor(2) // the dead rule + key(t, m, "d") + if m.confirm != confirmDeleteRule { + t.Fatal("expected d to ask for confirmation") + } + if !strings.Contains(m.status, "delete rule 3") { + t.Errorf("status = %q, want the rule identified", m.status) + } + + key(t, m, "n") // anything but y cancels + if m.confirm != confirmNone { + t.Error("expected the confirmation to clear") + } + loaded, _ := config.LoadRules(root) + if len(loaded.Rule) != 4 { + t.Errorf("cancelling deleted something: %d rules left", len(loaded.Rule)) + } +} + +func TestRuleListDeleteSelected(t *testing.T) { + m, root := newRuleListModel(t) + + m.ruleListTable.SetCursor(1) // the shadowed rule + key(t, m, "d") + key(t, m, "y") + + if m.err != nil { + t.Fatalf("delete failed: %v", m.err) + } + loaded, err := config.LoadRules(root) + if err != nil { + t.Fatal(err) + } + if len(loaded.Rule) != 3 { + t.Fatalf("got %d rules, want 3: %+v", len(loaded.Rule), loaded.Rule) + } + for _, r := range loaded.Rule { + if r.Tag == "shadowed" { + t.Error("the selected rule is still in rules.toml") + } + } + // The on-screen counts are rebuilt from the new file. + if len(m.ruleUsage) != 3 { + t.Errorf("usage still has %d entries, want 3", len(m.ruleUsage)) + } +} + +func TestRuleListPruneUnused(t *testing.T) { + m, root := newRuleListModel(t) + + key(t, m, "p") + if m.confirm != confirmPruneRules { + t.Fatal("expected p to ask for confirmation") + } + if !strings.Contains(m.status, "all 3 rules") { + t.Errorf("status = %q, want the count of dead rules", m.status) + } + key(t, m, "y") + + if m.err != nil { + t.Fatalf("prune failed: %v", m.err) + } + loaded, err := config.LoadRules(root) + if err != nil { + t.Fatal(err) + } + if len(loaded.Rule) != 1 || loaded.Rule[0].Tag != "groceries" { + t.Fatalf("remaining rules = %+v, want only the groceries rule", loaded.Rule) + } + + // The rule that was doing work still tags what it did before. + txns, err := m.db.Transactions(store.Filter{}) + if err != nil { + t.Fatal(err) + } + tagged := 0 + for _, txn := range txns { + if txn.Tag() == "groceries" { + tagged++ + } + } + if tagged != 3 { + t.Errorf("%d transactions tagged after pruning, want 3", tagged) + } + if !strings.Contains(m.View(), "all in use") { + t.Errorf("expected the title to report no dead rules left:\n%s", m.View()) + } +} + +func TestRuleListPruneWithNothingToDo(t *testing.T) { + m, root := newRuleListModel(t) + key(t, m, "p") + key(t, m, "y") + + // Second prune: nothing left to remove. + key(t, m, "p") + if m.confirm != confirmNone { + t.Error("expected no confirmation when there is nothing to prune") + } + if !strings.Contains(m.status, "no unused rules") { + t.Errorf("status = %q, want a note that there is nothing to prune", m.status) + } + loaded, _ := config.LoadRules(root) + if len(loaded.Rule) != 1 { + t.Errorf("got %d rules, want the 1 survivor untouched", len(loaded.Rule)) + } +} + // newEmptyModel builds a model over an index with nothing in it. func newEmptyModel(t *testing.T, accounts []*config.Account) *Model { t.Helper()