diff --git a/CHANGELOG.md b/CHANGELOG.md index b0d0357..3dca861 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 under budget, red over it — while still showing the true, unclamped percentage (#80). +### Fixed + +- Removed the duplicate "Total" data row that appeared directly above the TOTAL row + when the report is grouped as a single total (#137). +- Budget burn-down view now sizes its list-name column to the available width + instead of a fixed 24 columns, and drops the "(remaining …)" figure when that + column would otherwise fall below a readable minimum; percentage labels now + round down consistently with the displayed progress bar (#136). +- Text truncation and column width measurement now use display columns (accounting + for emoji and CJK character width) instead of Unicode rune count, preventing text + overflow in list and task names (#135). +- Fixed the report table rendering wider than the terminal when Hours or Billed + totals needed more than 8 characters (e.g. large hour totals); the Hours and + Billed columns are now measured from content instead of reserved at a fixed + width, so with realistic content the table fits within the terminal width. + Below a readable minimum (around 48 columns) the Item column floor takes + precedence and the table may overflow — the same accepted trade-off that + applies to all columns (#138). +- Relative date presets (like "last 7 days") now lock to the time the range was + loaded, preventing automatic re-labeling when rebuilding after midnight (#28). +- Filters screen now scrolls vertically to accommodate large numbers of lists, tags + and statuses (#28). +- Task and entry tags are now parsed correctly whether ClickUp returns them as tag + objects or as plain strings — the two documented shapes (#28). + ## [1.9.0] - 2026-07-28 ### Added diff --git a/README.it.md b/README.it.md index 9ce51d5..2d3b595 100644 --- a/README.it.md +++ b/README.it.md @@ -232,7 +232,9 @@ sessione. I filtri si compongono con la selezione membri e con il periodo attivo restringono solo ciò che è già stato caricato. Quando il periodo cambia, le selezioni dei filtri si adattano automaticamente alle nuove voci: ogni valore selezionato che non compare più viene scartato, così il report non resta mai bloccato vuoto per un -filtro ormai obsoleto. Comandi disponibili: +filtro ormai obsoleto. La schermata scorre verticalmente quando ci sono molte liste, +tag o status, così puoi navigare a qualsiasi valore anche su terminali con poche righe. +Comandi disponibili: - `Tab` / `Shift+Tab`: cambia sezione - `↑` / `↓` (anche `k` / `j`): naviga all'interno della sezione diff --git a/README.md b/README.md index 583750b..f3c37c2 100644 --- a/README.md +++ b/README.md @@ -221,7 +221,8 @@ rest of the session. Filters compose with the team member selection and the acti date range — they only narrow what is already loaded. When the date range changes, filter selections automatically adjust to the new entries: any selected value that no longer occurs is dropped, so the report never gets stuck empty because of a stale -filter. Available commands: +filter. The screen scrolls vertically when there are many lists, tags or statuses, +so you can navigate to any value even on short terminals. Available commands: - `Tab` / `Shift+Tab`: switch section - `↑` / `↓` (also `k` / `j`): move within the section diff --git a/docs/superpowers/plans/2026-07-30-tranche-d-layout-correctness.md b/docs/superpowers/plans/2026-07-30-tranche-d-layout-correctness.md new file mode 100644 index 0000000..83c54c6 --- /dev/null +++ b/docs/superpowers/plans/2026-07-30-tranche-d-layout-correctness.md @@ -0,0 +1,1877 @@ +# Tranche D — layout & correttezza: piano di implementazione + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Far diventare vere le invarianti di layout che i commenti già +dichiarano, chiudendo #137, #136, #135, #141, #138 (punto 1) e le caselle aperte +della #28. + +**Architecture:** Una primitiva sola (`ansi.Truncate`) sotto due helper nuovi in +`internal/tui/width.go`; da lì scendono la migrazione dei call site, il fix di +`clampWidth`, la larghezza misurata delle colonne numeriche e la view budget +width-aware. Poi due interventi indipendenti sulla #28: il range pinnato con +trasporto nel messaggio, e lo scroll dei Filters su una finestra condivisa col +palette. + +**Tech Stack:** Go 1.26, bubbletea v1.3.10, bubbles v1.0.0, lipgloss v1.1.0, +`charmbracelet/x/ansi` v0.11.6 (già dipendenza diretta). + +**Spec:** `docs/superpowers/specs/2026-07-30-tranche-d-layout-correctness-design.md` + +## Global Constraints + +- `internal/report` e `internal/duration` restano **puri**: nessun I/O, nessun + import di `internal/config`, `internal/clickup`, `internal/tui`. +- Nessuna funzione di stile in produzione chiama `lipgloss.NewStyle()`: gli + stili vengono dal renderer iniettato del `theme`. +- Mai chiamare l'API ClickUp reale. Il comportamento di rete si esercita solo + con `httptest` e il base URL del client puntato lì (segui il pattern dei test + già presenti in `internal/clickup`). +- Golden rigenerati con `go test ./internal/tui -update`, **mai** a mano. +- Gate prima di ogni commit, tutti e cinque puliti: `gofmt -l .`, + `go vet ./...`, `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, + `go build ./...`, `go test ./... -race`. +- Tutto ciò che vive nel repo è in **inglese**: codice, identificatori, + commenti, stringhe UI, nomi e messaggi dei test, messaggi di commit. +- Conventional Commits. **MAI** `Co-Authored-By`. +- **Un test scritto contro un bug specifico non è attendibile finché non lo si è + visto fallire contro quel bug.** Ogni task che chiude un difetto allega il + transcript del RED. Unica eccezione, dichiarata nella spec: il Task 3 (#141), + behaviorally inert e invisibile alla suite per costruzione. +- **Un RED va visto, non dedotto.** Se un test nuovo non compila perché nomina + una funzione che ancora non esiste, l'intero package non compila e nessun + numero comportamentale appare: `FAIL [build failed]` non è un RED. Dove serve, + gli step qui sotto **separano** i test comportamentali (che girano contro il + codice attuale) da quelli che nominano simboli nuovi. +- **Le larghezze si misurano rendendo, non calcolando.** Sommare a mano le + colonne che si crede la tabella userà ha già prodotto due numeri sbagliati in + questa tranche: `reportAmountWidth` riprende spazio ad Amount come ultima + risorsa, e `lipgloss/table` dimensiona ogni colonna anche sull'header. Misura + con `lipgloss.Width(strings.Split(reportTable(...), "\n")[0])`. +- Fixture obbligatorie: ogni test di larghezza nasce con **rune larghe** o con + la **congiunzione** che il difetto richiede. Un test a label corte e cifre + normali passa contro metà dei bug di questa tranche. + +## Helper esistenti (verificati — usali con queste firme esatte) + +- `newTestModel() Model` — `log_test.go:31`. **Nessun parametro.** +- `newTestModelOnReport() Model` — `log_test.go:36`. +- `testTheme(dark bool) theme` — `theme_test.go:15`. +- `goldenEntries()`, `goldenReport()`, `goldenModel()` — `golden_test.go`. + `goldenReport()` è `GroupByList`; `goldenModel()` fissa `loc = time.UTC`. +- `golden(t, name, out)` — `golden_test.go`. +- Il campo del Model è **`m.filtersScreen`** (`app.go:179`), non `m.filters`. +- `newFilters(entries []report.TimeEntry, lists, tags, statuses map[string]bool, billable *bool) filtersModel` + — `filters.go:70`. **Le opzioni delle sezioni Lists/Tags/Statuses derivano + dalle `entries`**, non dalle mappe: le mappe sono solo lo stato *selected*. + Con `entries == nil` tutte le sezioni tranne Billable hanno zero opzioni. + +## Dipendenze fra i task + +`1 → 2 → {3, 4, 5, 6}`, e `7 → 9`. Il Task 8 è indipendente. + +Il Task 9 **dipende dal Task 7**: estrae `rebuildReport` dagli stessi tre +blocchi di rebuild (`report.go:83`, `app.go:719`, `rates.go:855`) in cui il Task +7 sostituisce `currentRange()` con `activeRange()` e in cui l'handler passa a +`msg.start`/`msg.end`. Eseguito prima, il Task 9 farebbe puntare la lista di +siti del Task 7 a righe che non esistono più. + +Task 4 prima di Task 5: entrambi toccano `report_table.go`, e il Task 4 crea +golden nuovi che il Task 5 poi rigenera. + +--- + +### Task 1: `width.go` — le due primitive + +**Files:** +- Create: `internal/tui/width.go` +- Create: `internal/tui/width_test.go` + +**Interfaces:** +- Produces: `truncateWidth(s string, cols int) string`, `cell(s string, cols int) string`. + Ogni task successivo usa questi due e nient'altro per far stare una stringa + in una colonna. + +- [ ] **Step 1: Write the failing test** + +In `internal/tui/width_test.go`: + +```go +package tui + +import ( + "fmt" + "testing" + + "github.com/charmbracelet/lipgloss" +) + +// Display width and rune count agree only for ASCII, which is why every fixture +// here that matters is not ASCII. +func TestTruncateWidth(t *testing.T) { + tests := []struct { + name string + s string + cols int + want string + }{ + {"empty", "", 5, ""}, + {"fits", "hello", 10, "hello"}, + {"exact", "hello", 5, "hello"}, + {"cuts ascii", "hello world", 5, "hell…"}, + {"zero cols", "hello", 0, ""}, + {"negative cols", "hello", -3, ""}, + {"one col fits", "x", 1, "x"}, + {"one col cuts", "xy", 1, "…"}, + // A wide cluster is dropped whole rather than split, so the result can + // land NARROWER than cols. That is what makes the pad in cell() load-bearing. + {"wide runes cut on the boundary", "日本語のリスト", 2, "…"}, + {"wide runes cut inside", "日本語のリスト", 5, "日本…"}, + {"emoji", "🚀🚀🚀🚀", 3, "🚀…"}, + {"mixed", "ab🚀cd", 5, "ab🚀…"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := truncateWidth(tc.s, tc.cols) + if got != tc.want { + t.Errorf("truncateWidth(%q, %d) = %q, want %q", tc.s, tc.cols, got, tc.want) + } + if w := lipgloss.Width(got); tc.cols > 0 && w > tc.cols { + t.Errorf("truncateWidth(%q, %d) rendered %d columns, want at most %d", tc.s, tc.cols, w, tc.cols) + } + }) + } +} + +// cell must produce EXACTLY cols columns, which is the half that fmt's "%-Ns" +// gets wrong: fmt pads by rune count, so a 7-rune 14-column string padded to +// "%-10s" comes out 17 columns wide. +func TestCellIsExactlyColsWide(t *testing.T) { + for _, s := range []string{"", "x", "hello", "hello world", "日本語のリスト", "🚀🚀🚀🚀", "ab🚀cd"} { + for _, cols := range []int{1, 2, 5, 10, 24} { + got := cell(s, cols) + if w := lipgloss.Width(got); w != cols { + t.Errorf("cell(%q, %d) rendered %d columns, want exactly %d (%q)", s, cols, w, cols, got) + } + } + } +} + +func TestCellNonPositiveCols(t *testing.T) { + for _, cols := range []int{0, -1, -12} { + if got := cell("日本語", cols); got != "" { + t.Errorf("cell(%q, %d) = %q, want %q", "日本語", cols, got, "") + } + } +} + +// The migration in Task 2 must be a no-op for ASCII content at cols >= 2: that +// is what keeps every existing golden green, so the wide-rune tests are the only +// thing that distinguishes before from after. cols == 1 is the one documented +// divergence — the old rune cut returned "…" even when the input fit. +func TestCellMatchesTheOldPaddedCutForASCII(t *testing.T) { + for _, s := range []string{"", "x", "Website", "Website redesign", "Mobile app"} { + for _, cols := range []int{2, 5, 20, 24, 40} { + want := fmt.Sprintf("%-*s", cols, oldRuneCutForTest(s, cols)) + if got := cell(s, cols); got != want { + t.Errorf("cell(%q, %d) = %q, want %q (the pre-migration rendering)", s, cols, got, want) + } + } + } +} + +// oldRuneCutForTest is the rune-based cut this tranche deletes, kept in the test +// file only, as the reference the ASCII equivalence is measured against. +func oldRuneCutForTest(s string, n int) string { + if s == "" { + return "" + } + if n <= 1 { + return "…" + } + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[:n-1]) + "…" +} + +// The report footer cuts styled strings, so the cut must survive escapes. +// TestMain pins termenv.Ascii for this package, so a theme style renders without +// escapes: the fixture writes them by hand instead. +func TestTruncateWidthIsANSIAware(t *testing.T) { + const styled = "\x1b[1mabcdef\x1b[0m" + got := truncateWidth(styled, 3) + if want := "\x1b[1mab…\x1b[0m"; got != want { + t.Errorf("truncateWidth(%q, 3) = %q, want %q", styled, got, want) + } + if w := lipgloss.Width(got); w != 3 { + t.Errorf("truncated styled string rendered %d columns, want 3", w) + } +} + +// A style left OPEN by the caller is not closed by the cut: the escape survives +// and bleeds past it. Nothing in production feeds an unterminated style today +// (help.View closes its own), and this test exists so that a caller who starts +// doing it finds out here rather than on a user's terminal. +func TestTruncateWidthDoesNotCloseAnOpenStyle(t *testing.T) { + got := truncateWidth("\x1b[31mabcdef", 3) + if want := "\x1b[31mab…"; got != want { + t.Errorf("truncateWidth of an unterminated style = %q, want %q", got, want) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `go test ./internal/tui -run 'TestTruncateWidth|TestCell' -v` +Expected: `FAIL [build failed]` — `undefined: truncateWidth`, `undefined: cell`. +Qui il build-failure **è** il RED corretto: le funzioni non esistono ancora e non +c'è alcun difetto da esercitare. + +- [ ] **Step 3: Write the implementation** + +`internal/tui/width.go`: + +```go +package tui + +import ( + "strings" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" +) + +// This file is the only sanctioned way to fit a string into a column. +// +// Every layout in this package is measured in DISPLAY COLUMNS, but Go counts +// runes in two places that used to be relied on: a rune-sliced cut, and fmt's +// "%-Ns" padding (fmt's width is a rune count, so a 7-rune 14-column string +// padded to 10 comes out 17 columns wide). The two agree only for ASCII, and +// ClickUp list and task names are exactly where emoji and CJK live. +// +// Both helpers take a SINGLE LINE. ansi.Truncate lets a "\n" through, so a +// multi-line input would produce a "cell" whose padding applies to the last line +// only; no call site passes one. + +// truncateWidth cuts s to at most cols display columns, the ellipsis included. +// cols <= 0 returns "". +func truncateWidth(s string, cols int) string { + if cols <= 0 { + return "" + } + return ansi.Truncate(s, cols, "…") +} + +// cell renders s into exactly cols display columns: truncated when too wide, +// space-padded when too narrow. It replaces the truncate(s, N) + "%-Ns" pair, +// where both halves counted runes. +// +// The pad is not cosmetic. ansi.Truncate drops a wide cluster whole rather than +// splitting it, so a cut can land NARROWER than cols ("日本語のリスト" cut to 2 +// gives "…", one column). Without the measured pad the column misaligns anyway, +// just in the other direction. +func cell(s string, cols int) string { + s = truncateWidth(s, cols) + if pad := cols - lipgloss.Width(s); pad > 0 { + s += strings.Repeat(" ", pad) + } + return s +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/tui -run 'TestTruncateWidth|TestCell' -v` +Expected: PASS, tutti i sottotest. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/width.go internal/tui/width_test.go +git commit -m "feat(tui): add display-width truncate and cell helpers (#135)" +``` + +--- + +### Task 2: migrare i call site, eliminare `truncate` e `shaveToWidth` + +**Files:** +- Modify: `internal/tui/report.go` (elimina `truncate`, righe 270-282; aggiorna il commento a riga 269 che cita `shaveToWidth`) +- Modify: `internal/tui/report_table.go` (elimina `shaveToWidth`, righe 224-259; migra i 2 usi a 209-210; aggiorna i commenti a 142 e 195 che citano `truncate`) +- Modify: `internal/tui/palette.go` (**3 usi di `shaveToWidth`**: 214, 219, 253) +- Modify: `internal/tui/budget.go:75` +- Modify: `internal/tui/rates_view.go:53, 57, 76, 82, 92, 93, 143, 147, 149` +- Modify: `internal/tui/entries.go:583, 593, 643, 648` +- Modify: `internal/tui/log.go:540` +- Test: `internal/tui/width_test.go` (aggiunte) + +**Interfaces:** +- Consumes: `truncateWidth`, `cell` (Task 1). +- Produces: `truncate` e `shaveToWidth` **non esistono più**. + +**Attenzione, è il punto in cui questo task si rompe se letto in fretta:** +`shaveToWidth` ha **5 chiamanti**, non 2. Tre sono in `palette.go` e non hanno +niente a che vedere con la tabella report. Cancellare la funzione senza migrarli +non compila. Verifica prima di iniziare: + +```bash +grep -rn "shaveToWidth\|truncate(" internal/tui/*.go | grep -v _test +``` + +**Regola meccanica:** coppia `truncate(s,N)` + verbo `%-Ns` → `cell(s, N)` con +`%s`; sito senza pad → `truncateWidth(s, N)`. `shaveToWidth(s, w)` → +`truncateWidth(s, w)` in tutti e cinque i siti: la shave-loop faceva a mano, un +rune per giro, ciò che `ansi.Truncate` fa in un passo. + +- [ ] **Step 1: See the defect fail, before touching any call site** + +Il test sull'helper non è il RED di questo task: `cell` è già corretta dal Task +1. Il RED è sulla **coppia vecchia**. Aggiungi questo test temporaneo, eseguilo, +allega il transcript, poi cancellalo: + +```go +func TestTemporaryProofTheOldPairMisaligns(t *testing.T) { + const wide = "日本語のリストの名前がとても長い場合" + old := fmt.Sprintf("%-24s", oldRuneCutForTest(wide, 24)) + t.Logf("the old cut+pad rendered %d columns, want 24", lipgloss.Width(old)) + if lipgloss.Width(old) == 24 { + t.Fatal("the old pair did NOT misalign — the fixture is wrong, fix the fixture") + } +} +``` + +Run: `go test ./internal/tui -run TestTemporaryProofTheOldPairMisaligns -v` +Expected: PASS con un numero **diverso da 24** nel log (misurato: 42). Allega il +log, poi rimuovi il test. + +- [ ] **Step 2: Add the permanent regression test** + +```go +// The rates list, the entries browser and the budget screen all pair a cut with +// a fixed-width column. With a wide-rune list name the row used to render wider +// than its own column, which is what pushed those screens past the terminal +// edge. The fixture is deliberately CJK: an ASCII name passes against the bug. +func TestFixedWidthRowsHoldTheirColumnWithWideRunes(t *testing.T) { + const wide = "日本語のリストの名前がとても長い場合" + if lipgloss.Width(wide) == len([]rune(wide)) { + t.Fatalf("fixture is not wide: %d runes, %d columns", len([]rune(wide)), lipgloss.Width(wide)) + } + for _, cols := range []int{20, 24, 30} { + if w := lipgloss.Width(cell(wide, cols)); w != cols { + t.Errorf("cell(wide, %d) rendered %d columns, want %d", cols, w, cols) + } + } +} +``` + +- [ ] **Step 3: Migrate the sites** + +`internal/tui/budget.go:74-76` — swap meccanico (il Task 6 riscriverà la riga): + +```go + rows.WriteString(fmt.Sprintf("%s %s %.2f / %.2f %s (remaining %.2f)\n", + cell(l.ListName, 24), renderBudgetBar(th, l.PercentUsed), l.Billed, l.Budget, l.Currency, l.Remaining)) +``` + +`internal/tui/rates_view.go`: + +```go +// :52-53 + line := fmt.Sprintf("%s %10.2f %-5s %10s %s", + cell(r.name, 24), rate, rt.effectiveCurrency(r.listID), moneyOrDash(bud, hasBud), tag) +// :57 — a sentence, not a column + note := fmt.Sprintf("Effective for %s: %.2f %s", truncateWidth(sel.name, 24), rt.rateFor(sel.listID), rt.effectiveCurrency(sel.listID)) +// :76 + line := fmt.Sprintf("%s %10.2f %s", cell(fmt.Sprintf("%s (%d)", mr.name, mr.id), 30), rate, tag) +// :82 — a sentence + note = fmt.Sprintf("%s is overridden on %d list(s) by a (list,member) rate.", truncateWidth(sel.name, 24), n) +// :91-94 + line := fmt.Sprintf("%s %s %10.2f %.2f (%s)", + cell(rt.listName(o.listID), 20), + cell(fmt.Sprintf("%s (%d)", rt.memberName(o.member), o.member), 22), + o.rate, below, src) +// :143 — no padding, the row is the whole line + b += billingRow(th, i == rt.draft.idx, truncateWidth(r.name, 40)) +// :147 — a sentence + b := th.Help.Render(fmt.Sprintf("New override on %s — choose the member:", truncateWidth(rt.listName(rt.draft.listID), 24))) + "\n" +// :149 — no padding + b += billingRow(th, i == rt.draft.idx, truncateWidth(fmt.Sprintf("%s (%d)", mr.name, mr.id), 40)) +``` + +Le **tre** righe di header letterali di questo file — `rates_view.go:42` +(`%-24s`), `:66` (`%-30s`), `:87` (`%-20s %-22s`) — restano con i verbi `%-Ns`, +e lo stesso vale per il `%-5s` sulla valuta a riga 52. Aggiungi a ciascuna delle +tre righe di header un commento di una riga che dica perché: sono letterali +ASCII (e codici ISO), dove rune e colonne coincidono per costruzione, e restano +allineate con le righe dati perché `cell(...,N)` produce esattamente N colonne. + +`internal/tui/entries.go`: + +```go +// :583 — a sentence + "Delete " + th.Accent.Render(truncateWidth(e.TaskName, 40)) + " (" + +// :593 — a title + b := th.Title.Render("Tags") + " " + th.Accent.Render(truncateWidth(tagPickerTaskName(es), 40)) + "\n\n" +// :642-643 + line := fmt.Sprintf("%s %s %6s %s %s", + when, cell(e.TaskName, 24), duration.FormatHours(e.Duration), bill, owner) +// :648 — appended, no column + line += " " + truncateWidth(tagBadges(e.EntryTags), 20) +``` + +`internal/tui/log.go:540`: + +```go + line := truncateWidth(tk.Name, 40) +``` + +`internal/tui/palette.go` — i tre siti, sostituzione diretta: + +```go +// :214 + q := truncateWidth("> "+p.query, innerW) +// :219 + msg := truncateWidth("no matching action", innerW) +// :253 + label := truncateWidth(it.a.label, max(labelW, 1)) +``` + +`internal/tui/report_table.go:209-210` — sostituzione diretta, poi elimina +`shaveToWidth` (224-259): + +```go + rows[i][0] = truncateWidth(rows[i][0], itemW) + rows[i][3] = truncateWidth(rows[i][3], amountW) +``` + +Elimina `truncate` (`report.go:270-282`) e aggiorna i tre commenti che nominano +funzioni che non esistono più: `report.go:269` (cita `shaveToWidth`), +`report_table.go:142` («truncate's own guard (n <= 1) is what makes that safe» — +la guardia nuova è `cols <= 0 → ""`, che è **diversa**, quindi il commento va +riscritto, non solo rinominato) e `report_table.go:195` («letting truncate cut +it»). + +- [ ] **Step 4: Run the full suite and regenerate nothing** + +Run: `go test ./internal/tui -race` +Expected: **PASS senza rigenerare alcun golden**, compresi i golden `palette_*`. +Se un golden si muove, non rigenerarlo: significa che un sito è stato migrato con +la larghezza sbagliata, o che un `%-Ns` è rimasto accoppiato a un `cell`. Riporta +nel report ogni golden che si è mosso e perché. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add -A internal/tui +git commit -m "refactor(tui): truncate and pad by display width, not rune count (#135)" +``` + +--- + +### Task 3: `clampWidth` senza il renderer di default (#141) + +**Files:** +- Modify: `internal/tui/footer.go:56-63` +- Test: `internal/tui/footer_test.go` + +**Interfaces:** +- Consumes: `truncateWidth` (Task 1). + +**Nota di processo:** è l'unico task della tranche **senza** red-green, e la spec +lo dichiara. Il difetto è behaviorally inert: `MaxWidth` e `ansi.Truncate` +tagliano in modo indipendente dal profilo colore, e `TestMain` fissa +`termenv.Ascii`, quindi nessun test comportamentale può fallire contro di esso. +La prova è il grep più il test anti-bleed. Non inventare un red-green: se ti +sembra di averne trovato uno, stai testando altro. + +- [ ] **Step 1: Write the tests, and run them against the CURRENT code first** + +```go +// clampWidth must not build a style on lipgloss's default renderer: that is the +// one thing the injected-renderer discipline exists to prevent, and this file's +// own opening comment says so about help.New(). +// +// This is invisible to the suite by construction: TestMain pins the default +// renderer to termenv.Ascii, so a style built on it renders identically to one +// built on the injected renderer, and no golden or assertion can tell them +// apart. Do not add a test that claims otherwise — pin the truncation instead, +// and keep the grep in the checklist. +func TestClampWidthNeverExceedsTheTerminal(t *testing.T) { + th := testTheme(true) + long := "↑/↓ move · enter run · esc close · ctrl+c force quit · ? help · g group · e export" + for _, width := range []int{20, 40, 66, 80} { + got := clampWidth(th, long, width) + if w := lipgloss.Width(got); w > width { + t.Errorf("clampWidth(width=%d) rendered %d columns", width, w) + } + if !strings.HasSuffix(got, "…") { + t.Errorf("clampWidth(width=%d) = %q, want a visible ellipsis", width, got) + } + } +} + +// The footer's input comes from help.View, which closes its own styles. If a +// caller ever passes an unterminated one, ansi.Truncate will NOT close it and +// the style bleeds past the cut — this pins the boundary of what clampWidth +// promises, so the discovery happens here and not on a user's terminal. +func TestClampWidthDoesNotCloseAnOpenStyle(t *testing.T) { + th := testTheme(true) + got := clampWidth(th, "\x1b[31m"+strings.Repeat("x", 40), 10) + if !strings.Contains(got, "\x1b[31m") { + t.Fatalf("expected the caller's escape to survive, got %q", got) + } + if strings.Count(got, "\x1b[0m") != 0 { + t.Errorf("clampWidth closed a style it did not open: %q", got) + } +} +``` + +Run: `go test ./internal/tui -run TestClampWidth -v` +Expected: il primo PASS (il comportamento non cambia con questo task). Il secondo +documenta cosa fa `MaxWidth` **oggi** con uno stile aperto. **Riporta l'esito di +entrambi nel report prima di cambiare il codice**, così la differenza +prima/dopo è registrata invece di essere assunta. + +- [ ] **Step 2: Replace the implementation** + +```go + // ansi.Truncate cuts ANSI-aware without a renderer: MaxWidth would do the + // same cut, but only by building a style on lipgloss's DEFAULT renderer — + // the one this file's opening comment refuses help.New() over. The ellipsis + // is added separately, styled from the theme, so the cut is visible rather + // than looking like a footer that simply ends. + // + // This is invisible to the tests by construction: TestMain pins the default + // renderer to termenv.Ascii, so both spellings render identically and no + // golden can tell them apart. The guard is the grep, not the suite. + return ansi.Truncate(s, width-1, "") + th.Help.Render("…") +``` + +- [ ] **Step 3: Run the tests, then the grep that is this task's real proof** + +```bash +go test ./internal/tui -run TestClampWidth -v +go test ./internal/tui -race +grep -rn "lipgloss.NewStyle()" internal/ --include="*.go" | grep -v "_test.go" +``` +Expected: test PASS, golden fermi; il grep restituisce **solo** il commento a +`report_table.go:163`, nessuna occorrenza di codice. Allega l'output del grep: è +la chiusura della seconda casella della #141. + +- [ ] **Step 4: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/footer.go internal/tui/footer_test.go +git commit -m "refactor(tui): cut the footer with ansi.Truncate, not a default-renderer style (#141)" +``` + +--- + +### Task 4: la riga `Total` duplicata (#137) + +**Files:** +- Modify: `internal/tui/report_table.go` (`reportRows`, righe 36-73) +- Test: `internal/tui/report_table_test.go` +- Test: `internal/tui/golden_test.go` (**due golden nuovi**) +- Test: `internal/tui/testdata/report_total.golden`, `report_total_multicurrency.golden` (nuovi) + +**Interfaces:** +- Produces: `reportRows` non emette più la riga bucket sotto `GroupByTotal` con + un bucket unico; `firstTotal` in quel caso è 0. + +**Nota sui golden:** **nessun golden esistente si muove.** Tutti i golden del +report girano su `goldenReport()`, che è `GroupByList`, e in `testdata/` non +esiste alcuna riga bucket `Total` (verificalo: `grep -rln "^│ Total" internal/tui/testdata/` +non trova niente). I due golden di questa forma vanno **creati**. + +- [ ] **Step 1: Write the failing test** + +```go +// Under GroupByTotal — the grouping the report loads with — the single bucket IS +// the totals row: same hours, same billed, and the same amounts, because one +// bucket collects every entry. The table used to show "Total" directly above +// "TOTAL", differing only in case and color (#137). +func TestReportRowsSuppressesTheBucketUnderTotalGrouping(t *testing.T) { + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 15.5, BilledHours: 12.5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 15.5, BilledHours: 12.5, Amount: 625}}, + TotalHours: 15.5, BilledHours: 12.5, TotalAmount: 625, + } + rows, firstTotal := reportRows(r) + if len(rows) != 1 { + t.Fatalf("got %d rows, want 1 (the TOTAL row alone): %v", len(rows), rows) + } + if firstTotal != 0 { + t.Errorf("firstTotal = %d, want 0 (the TOTAL row is the first row)", firstTotal) + } + if rows[0][0] != "TOTAL" { + t.Errorf("rows[0][0] = %q, want %q", rows[0][0], "TOTAL") + } +} + +// Multi-currency keeps its shape: TOTAL with an empty Amount, then one subtotal +// row per currency. The suppressed bucket carried "A + B", which is the same +// information those subtotal rows carry. +func TestReportRowsSuppressesTheBucketUnderTotalGroupingMultiCurrency(t *testing.T) { + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 18, BilledHours: 17, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 337.5}, {Currency: "USD", Amount: 512.5}}}}, + CurrencySubtotals: []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 7.75, BilledHours: 6.75, Amount: 337.5}, + {Currency: "USD", Hours: 10.25, BilledHours: 10.25, Amount: 512.5}, + }, + TotalHours: 18, BilledHours: 17, + } + rows, firstTotal := reportRows(r) + if len(rows) != 3 { + t.Fatalf("got %d rows, want 3 (TOTAL + two subtotals): %v", len(rows), rows) + } + if firstTotal != 0 { + t.Errorf("firstTotal = %d, want 0", firstTotal) + } + if rows[0][3] != "" { + t.Errorf("TOTAL Amount = %q, want empty (the subtotals carry the figures)", rows[0][3]) + } +} + +// Every other grouping keeps its bucket rows: the suppression is about the +// degenerate one-bucket case of GroupByTotal, not about totals in general. +func TestReportRowsKeepsBucketsUnderOtherGroupings(t *testing.T) { + r := report.Report{ + GroupBy: report.GroupByList, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Website", Hours: 12.5, BilledHours: 12.5}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 12.5, BilledHours: 12.5, Amount: 625}}, + TotalHours: 12.5, BilledHours: 12.5, TotalAmount: 625, + } + rows, firstTotal := reportRows(r) + if len(rows) != 2 || firstTotal != 1 { + t.Fatalf("got %d rows firstTotal=%d, want 2 rows firstTotal=1: %v", len(rows), firstTotal, rows) + } +} +``` + +- [ ] **Step 2: Run to verify the first two fail** + +Run: `go test ./internal/tui -run TestReportRows -v` +Expected: i primi due FAIL con `got 2 rows, want 1` e `got 4 rows, want 3`; il +terzo PASS. Tutti e tre compilano contro il codice attuale, quindi questo è un +RED comportamentale vero. **Allega il transcript.** + +- [ ] **Step 3: Implement** + +In `reportRows`, sostituisci il ciclo sui bucket (righe 38-45) con: + +```go + // Under GroupByTotal the single bucket IS the totals row: it collects every + // entry, so its hours, billed hours and Amounts equal the totals and the + // currency subtotals exactly — not approximately, so there is not even the + // PerDay rounding caveat that applies at finer groupings. Emitting both put + // "Total" directly above "TOTAL", differing only in case and color (#137). + if !(r.GroupBy == report.GroupByTotal && len(r.Buckets) == 1) { + for _, b := range r.Buckets { + rows = append(rows, []string{ + b.Label, + fmt.Sprintf("%.2f", b.Hours), + fmt.Sprintf("%.2f", b.BilledHours), + formatAmounts(b.Amounts, r.DefaultCurrency), + }) + } + } +``` + +`firstTotal := len(rows)` resta com'è: diventa 0 da solo, e con `firstTotal == 0` +lo `StyleFunc` colora già la riga 0 come riga totali. `len(Buckets) == 0` non +arriva qui: `report.go:208` intercetta con «No hours to show.». + +- [ ] **Step 4: Add the two new goldens** + +In `golden_test.go`, accanto agli altri, seguendo il pattern del file: + +```go +// The Total grouping is what the report loads with, so it is the first table a +// user (and the README GIF) ever sees. Two goldens: one currency, and the +// multi-currency shape where TOTAL's Amount is empty and the subtotals carry +// the figures (#137). +func TestGoldenReportTotalGrouping(t *testing.T) { ... } +func TestGoldenReportTotalGroupingMultiCurrency(t *testing.T) { ... } +``` + +Costruisci le due `report.Report` con le stesse fixture dello Step 1 e rendi con +`reportTable(testTheme(true), r, 80)`. Poi: + +```bash +go test ./internal/tui -race # i due golden nuovi FAIL: il file non esiste +go test ./internal/tui -update +git status --short internal/tui/testdata/ +``` +Expected: **due file nuovi**, nessun file esistente modificato. Se un golden +esistente si muove, fermati e riporta: significa che la soppressione ha colpito +una forma che non doveva. **Guarda i due golden nuovi con i tuoi occhi** e +riportali nel report. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): stop repeating the total row under the Total grouping (#137)" +``` + +--- + +### Task 5: le colonne numeriche misurate (#138, punto 1) + +**Files:** +- Modify: `internal/tui/report_table.go` (`reportItemWidth`, `reportAmountWidth`, + elimina `reportNumWidth` e aggiorna il commento del blocco `const`, righe 11-23) +- Test: `internal/tui/report_table_test.go` + +**Interfaces:** +- Produces: `reportNumWidths(rows [][]string) (hours, billed int)`. + `reportItemWidth` e `reportAmountWidth` mantengono la firma attuale. + +**I due fatti che decidono l'implementazione:** + +1. In `lipgloss/table` **l'header determina la larghezza di colonna**. Misurato: + celle `"12.50"` con header `"H"` → 7 colonne, con header `"Billed"` → 8. + Quindi la misura **deve** includere gli header, come `reportItemWidth` già fa + per Item e Amount (righe 102-103). Misurando solo i dati il budget assume + 5+5=10 mentre il renderer usa 5+6=11, e la tabella sfora di 1. +2. **La tabella non può stare in 40 colonne** con cifre a 10 caratteri: il minimo + post-fix è `chrome(10) + floor Item + 10 + 10 + header Amount(6)` = 48 con + label lunghe, 43 con label corte. Un test che asserisce `≤ 40` fallisce + contro il codice corretto. + +Sweep misurato del codice **attuale** (larghezza resa, `!` = sfora): + +| width | label lunga + cifre larghe | label corta + cifre larghe | +|---|---|---| +| 40 | 48 ! | 44 ! | +| 44 | 48 ! | 47 ! | +| 48 | 52 ! | 47 ✓ | +| 60 | 64 ! | 47 ✓ | +| 80 | 84 ! | 47 ✓ | +| 100 | 86 ! | 47 ✓ | + +Da qui le larghezze dei test: **60, 80, 100** per la label lunga, e **44** per la +label corta — l'unica finestra in cui quella direzione è rossa prima (47 in 44) e +verde dopo (43 in 44). + +- [ ] **Step 1: Write the two behavioral tests ONLY** + +Non scrivere ancora il test su `reportNumWidths`: nominerebbe un simbolo +inesistente, il package non compilerebbe e i due RED qui sotto non si vedrebbero. + +```go +// reportNumWidth reserved 8 columns for each of Hours and Billed but nothing +// enforced it: a wider value pushed the table past the terminal edge by exactly +// 2 x (width - 8). The overflow needs BOTH factors — a long label (so Item is +// budget-bound rather than capped by its own content) and numbers wider than 8. +// A test missing either one passes against the bug: measured, a long label with +// ordinary hours renders 55 columns at width 60. +// +// 40 and 44 are deliberately absent: the fix cannot fit this table in fewer than +// 48 columns (chrome 10 + Item floor 12 + 10 + 10 + Amount header 6), so +// asserting there would fail against correct code. +func TestReportTableNeverExceedsWidthWithLongLabelAndWideNumbers(t *testing.T) { + th := testTheme(true) + const longLabel = "Website redesign and content migration backlog" + rep := reportWithHours(longLabel, 1234567.5) + for _, width := range []int{60, 80, 100} { + got := lipgloss.Width(strings.Split(reportTable(th, rep, width), "\n")[0]) + if got > width { + t.Errorf("width %d: table rendered %d columns", width, got) + } + } +} + +// The other direction: short labels leave Item capped by its own content, but +// wide numbers still push the table past a narrow terminal. 44 is the width +// where the current code renders 47 (overflowing by 3) and the fix renders 43. +func TestReportTableNeverExceedsWidthWithShortLabelAndWideNumbers(t *testing.T) { + th := testTheme(true) + rep := reportWithHours("Website", 1234567.5) + if got := lipgloss.Width(strings.Split(reportTable(th, rep, 44), "\n")[0]); got > 44 { + t.Errorf("table rendered %d columns in a 44-column terminal", got) + } +} + +// reportWithHours builds a one-bucket report whose Hours and Billed carry the +// given value, so a test can dial the numeric width independently of the label. +func reportWithHours(label string, hours float64) report.Report { + return report.Report{ + GroupBy: report.GroupByList, DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: label, Hours: hours, BilledHours: hours, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: hours, BilledHours: hours, Amount: 625}}, + TotalHours: hours, BilledHours: hours, TotalAmount: 625, + } +} +``` + +- [ ] **Step 2: Run to see the RED, with the numbers from the sweep** + +Run: `go test ./internal/tui -run TestReportTableNeverExceedsWidthWith -v` +Expected: il primo FAIL tre volte — **64** a width 60, **84** a 80, **86** a 100; +il secondo FAIL una volta — **47** in un terminale da 44. Se i numeri non sono +questi, fermati: la fixture non è quella misurata dalla spec e il test non sta +esercitando il difetto che credi. **Allega il transcript.** + +- [ ] **Step 3: Implement, and add the header test alongside it** + +```go +// reportNumWidths measures the Hours and Billed columns from the rows, HEADERS +// INCLUDED. The headers are not decoration in this arithmetic: lipgloss/table +// sizes a column from every row it is given, the header row among them, so a +// "Billed" header holds 6 columns even when every value is 5. reportNumWidth +// used to reserve a flat 8 for each and nothing enforced it — a wider value +// simply pushed the table past the terminal edge by 2 x (width - 8) (#138). +func reportNumWidths(rows [][]string) (hours, billed int) { + hours = lipgloss.Width(reportHeaders[1]) + billed = lipgloss.Width(reportHeaders[2]) + for _, row := range rows { + hours = max(hours, lipgloss.Width(row[1])) + billed = max(billed, lipgloss.Width(row[2])) + } + return hours, billed +} +``` + +In `reportItemWidth` e `reportAmountWidth`, sostituisci `2*reportNumWidth` con +`hours + billed` presi da `reportNumWidths(rows)`. Elimina la costante +`reportNumWidth` e riscrivi la parte del commento del blocco `const` che la +descrive. + +E il test che inchioda la parte header: + +```go +// Measuring the data only would make the budget assume 10 where the renderer +// uses 11, and the table would overflow by 1 — the same bug in a smaller size. +func TestReportNumWidthsIncludeTheHeaders(t *testing.T) { + rows := [][]string{{"Website", "1.00", "1.00", "5.00 EUR"}} + hours, billed := reportNumWidths(rows) + if hours != lipgloss.Width(reportHeaders[1]) || billed != lipgloss.Width(reportHeaders[2]) { + t.Errorf("reportNumWidths = (%d, %d), want the header widths (%d, %d)", + hours, billed, lipgloss.Width(reportHeaders[1]), lipgloss.Width(reportHeaders[2])) + } +} +``` + +- [ ] **Step 4: Run, then regenerate the goldens** + +```bash +go test ./internal/tui -run 'TestReportTable|TestReportNumWidths|TestReportItemWidth|TestReportAmountWidth' -v +go test ./internal/tui -race +go test ./internal/tui -update +git diff internal/tui/testdata/ +``` +Expected: dove Item è vincolato dal budget (label lunghe: `report`, +`report_narrow`) Item guadagna **5** colonne — non 6: 16 riservate → 11 misurate +(`"Hours"`=5 + `"Billed"`=6). Dove le label sono corte l'output è identico. +**Guarda il diff dei golden con i tuoi occhi**: se un golden guadagna 6 colonne +invece di 5, la misura non include gli header. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): measure the report's numeric columns instead of reserving them (#138)" +``` + +--- + +### Task 6: il budget in 80 colonne (#136) + +**Files:** +- Modify: `internal/tui/budget.go` +- Modify: `internal/tui/golden_test.go:198, 203` — `TestGoldenBudget` e + `TestGoldenBudgetEmpty` chiamano `view(testTheme(true))` e vanno aggiornati +- Modify: il chiamante di `budgetModel.view` in `internal/tui` (cercalo: + `grep -rn "budgetScreen.view" internal/tui`) +- Test: `internal/tui/budget_test.go` +- Test: `internal/tui/testdata/budget.golden`, `budget_empty.golden` + +**Interfaces:** +- Consumes: `cell` (Task 1). +- Produces: `budgetModel.view(th theme, width int) string`; + `budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemaining bool)`; + `budgetFigures(l report.BudgetLine, withRemaining bool) string`. + +**Larghezza dei golden, fissata qui e non lasciata all'implementatore:** +entrambi i golden rendono a **width 80** — la larghezza della issue #136. Con +`view(th, 0)` il golden non cambierebbe affatto (il fallback tiene il layout +naturale), che è l'opposto di quello che questo task deve dimostrare. + +**La soglia di degrado, misurata:** la riga richiede almeno +`4 + 1 + budgetBarWidth + 1 + pctW + 2 + figuresW(senza remaining) + 12` +colonne, cioè **63** con le cifre del golden e **65** con importi a quattro cifre +e percentuale sopra 100. Sotto quella soglia sfora, per design. Quindi: +- fixture a una riga (cifre del golden): sta in 63, 65, 80, 100; **sfora a 60**; +- fixture a due righe (con 1040.00 e 104%): sta in 65, 80, 100; **sfora a 60 e 63**. + +I test qui sotto asseriscono `≤ width` solo alle larghezze in cui la fixture ci +sta. Non è indulgenza: è la differenza fra un test che verifica il fix e un test +che pretende l'impossibile. + +- [ ] **Step 1: Run the bar test against the CURRENT code, in isolation** + +`renderBudgetBar` ha già la firma giusta, quindi il difetto #136.2 si può vedere +fallire **prima** di toccare la firma di `view`. Metti questo test in un file a +sé (`internal/tui/budget_bar_test.go`) così non venga trascinato giù dagli errori +di compilazione dei test dello Step 2: + +```go +// The label rounded while the fill truncated, so from 99.5% up the label read +// "100%" over 19 of 20 blocks. Flooring the LABEL is the fix, not rounding the +// fill: a rounded fill would show a full bar from 97.5%, and a full bar means +// the budget is spent. The label stays unclamped above 100 — that is why this +// screen does not use bubbles/progress. +func TestBudgetBarLabelAgreesWithTheFill(t *testing.T) { + th := testTheme(true) + for _, pct := range []float64{0, 50, 97.6, 99.4, 99.5, 99.6, 99.9, 100, 104.7} { + bar := renderBudgetBar(th, pct) + full := strings.Count(bar, string(gaugeFull)) + wantFull := int(min(max(pct, 0), 100) / 100 * budgetBarWidth) + if full != wantFull { + t.Errorf("pct=%.1f: %d full blocks, want %d (%q)", pct, full, wantFull, bar) + } + // The label must state the floored percentage, so it can never claim a + // milestone the bar has not reached. + wantLabel := fmt.Sprintf("%.0f%%", math.Floor(pct)) + if !strings.HasSuffix(bar, " "+wantLabel) { + t.Errorf("pct=%.1f: %q does not end in %q", pct, bar, wantLabel) + } + } +} +``` + +Run: `go test ./internal/tui -run TestBudgetBarLabelAgreesWithTheFill -v` +Expected: **FAIL** su 99,5 / 99,6 / 99,9 (label `100%` invece di `99%`) e su +104,7 (label `105%` invece di `104%`). **Allega il transcript**: è il RED di +#136.2. + +- [ ] **Step 2: Prove the overflow against the current code, then write the rest** + +Sonda temporanea (la firma di `view` è ancora quella vecchia): eseguila, allega +l'output, cancellala. + +```go +func TestTemporaryProofBudgetOverflows80(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + }) + for _, line := range strings.Split(bm.view(th), "\n") { + t.Logf("%3d cols %q", lipgloss.Width(line), line) + } +} +``` +Expected nel log: una riga da **94** colonne. + +Poi i test definitivi, in `budget_test.go`: + +```go +// The budget line is unbounded: it grows with the figures. Measured before the +// fix, testdata/budget.golden is 94 columns (90 of content + 4 of th.Box), and +// 80 columns is a bare terminal and a split tmux pane (#136). +// +// The widths are the ones this fixture can actually fit: the row needs +// box(4) + 1 + gauge(20) + 1 + pct + 2 + figures + name-floor(12), which is 65 +// for these two lines. Asserting at 60 would demand the impossible — below the +// threshold the row overflows by design, and the spec says so. +func TestBudgetViewNeverExceedsWidth(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + {ListName: "Mobile app", Billed: 1040, Budget: 1000, Currency: "EUR", Remaining: -40, PercentUsed: 104}, + }) + for _, width := range []int{65, 80, 100, 120} { + for _, line := range strings.Split(bm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line rendered %d columns: %q", width, w, line) + } + } + } +} + +// Small figures let the row fit a narrow terminal, which is what proves the +// layout is measured rather than reserved: same code, more room, wider name. +func TestBudgetViewFitsNarrowTerminalsWithSmallFigures(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 12.5, Budget: 20, Currency: "EUR", Remaining: 7.5, PercentUsed: 62.5}, + }) + for _, width := range []int{60, 80} { + for _, line := range strings.Split(bm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line rendered %d columns: %q", width, w, line) + } + } + } +} + +// A wide-rune list name must not push the row past its column: the name column +// is the one that absorbs the slack, so it is also the one that used to +// misalign. An ASCII fixture passes against the bug. +func TestBudgetViewHoldsItsWidthWithWideRunes(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: strings.Repeat("🚀", 24), Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + }) + for _, line := range strings.Split(bm.view(th, 80), "\n") { + if w := lipgloss.Width(line); w > 80 { + t.Errorf("line rendered %d columns: %q", w, line) + } + } +} + +// "(remaining X)" is the most redundant field on the row (it is budget minus +// billed), so it is what gives way when the name column would fall below the +// floor. One pass: compute with it, and if the name is too narrow, recompute +// without it and never put it back. Measured for this fixture at width 80: with +// remaining the name would get 10 columns, under the floor of 12, so remaining +// goes and the name gets 29. +func TestBudgetLayoutDropsRemainingBeforeStarvingTheName(t *testing.T) { + lines := []report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + } + nameW, showRemaining := budgetLayout(lines, 80) + if showRemaining { + t.Errorf("at width 80 remaining should have been dropped; name column is %d", nameW) + } + if nameW != 29 { + t.Errorf("nameW = %d, want 29", nameW) + } + // A roomy terminal keeps remaining AND a wider name. + wideName, wideRem := budgetLayout(lines, 100) + if !wideRem { + t.Errorf("at width 100 remaining should fit; name column is %d", wideName) + } +} + +// Before the first WindowSizeMsg the width is 0 and nothing is sized against it: +// the screen keeps its natural layout, the same fallback reportItemWidth has. +func TestBudgetLayoutFallsBackBeforeTheFirstWindowSize(t *testing.T) { + nameW, showRemaining := budgetLayout([]report.BudgetLine{{ListName: "Website"}}, 0) + if nameW != budgetNameWidth || !showRemaining { + t.Errorf("budgetLayout(width=0) = (%d, %v), want (%d, true)", nameW, showRemaining, budgetNameWidth) + } +} +``` + +Run: `go test ./internal/tui -run TestBudget -v` +Expected: `FAIL [build failed]` sulla firma di `view` e su `budgetLayout` — +atteso, sono simboli nuovi. Il RED comportamentale di questo task è quello dello +Step 1 più il log della sonda. + +- [ ] **Step 3: Implement** + +```go +const ( + // boxChrome is what th.Box spends on itself: a rounded border plus one + // column of padding on each side. Measured, not assumed: + // th.Box.Render("12345") renders 9 columns. + boxChrome = 4 + // budgetNameWidth is the natural list-name column, used before the first + // WindowSizeMsg — the same "nothing is sized yet" fallback reportItemWidth has. + budgetNameWidth = 24 + // budgetMinNameWidth is where a list name stops being a name, mirroring + // reportMinItemWidth's floor on the report table. + budgetMinNameWidth = 12 +) + +// budgetFigures renders the money part of a row. remaining is the most redundant +// field on the line — it is Budget minus Billed — so it is what gives way on a +// narrow terminal. +func budgetFigures(l report.BudgetLine, withRemaining bool) string { + if withRemaining { + return fmt.Sprintf("%.2f / %.2f %s (remaining %.2f)", l.Billed, l.Budget, l.Currency, l.Remaining) + } + return fmt.Sprintf("%.2f / %.2f %s", l.Billed, l.Budget, l.Currency) +} + +// budgetLayout resolves the list-name column and whether "(remaining …)" fits. +// +// Everything except the name is MEASURED from the real rows, because none of it +// is fixed-width: the figures grow with the amounts and the percentage grows past +// 100. Reserving a constant for them is exactly how this screen came to render 94 +// columns into an 80-column terminal. +// +// One pass, in this order — the circularity (figures depend on the drop, the drop +// depends on the name, the name depends on the figures) is resolved here and not +// left to the caller: +// +// 1. compute with remaining; if the name column clears the floor, done; +// 2. otherwise recompute without remaining, and never put it back; +// 3. if the name is still under the floor, the floor wins and the row +// overflows. That happens below boxChrome + 1 + budgetBarWidth + 1 + pctW + +// 2 + figures + budgetMinNameWidth — around 63 to 65 columns with realistic +// amounts. Below it no split saves the row, and pretending otherwise would +// mean a name column too narrow to read. Accepted and explicit, the same +// trade-off the report table makes below its own floor. +func budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemaining bool) { + if width <= 0 { // before the first WindowSizeMsg nothing is sized against it + return budgetNameWidth, true + } + pctW := 0 + for _, l := range lines { + pctW = max(pctW, lipgloss.Width(fmt.Sprintf("%.0f%%", math.Floor(l.PercentUsed)))) + } + widestFigures := func(withRemaining bool) int { + w := 0 + for _, l := range lines { + w = max(w, lipgloss.Width(budgetFigures(l, withRemaining))) + } + return w + } + fixed := 1 + budgetBarWidth + 1 + pctW + 2 + usable := width - boxChrome + + if nameW = usable - fixed - widestFigures(true); nameW >= budgetMinNameWidth { + return nameW, true + } + return max(usable-fixed-widestFigures(false), budgetMinNameWidth), false +} +``` + +`view` diventa: + +```go +func (bm budgetModel) view(th theme, width int) string { + title := th.Title.Render("Budget burn-down") + if len(bm.lines) == 0 { + return title + "\n\n" + th.Box.Render("No budgets configured.") + } + nameW, showRemaining := budgetLayout(bm.lines, width) + var rows strings.Builder + for _, l := range bm.lines { + rows.WriteString(fmt.Sprintf("%s %s %s\n", + cell(l.ListName, nameW), renderBudgetBar(th, l.PercentUsed), budgetFigures(l, showRemaining))) + } + body := th.Box.Render(strings.TrimRight(rows.String(), "\n")) + return title + "\n\n" + body +} +``` + +E in `renderBudgetBar`, l'ultima riga: + +```go + // The fill truncates (int()), so the label FLOORS to match. Rounding the + // label instead is what made 99.5% read "100%" over 19 of 20 blocks; and + // rounding the FILL to match a rounded label would be worse — it would show + // a full bar from 97.5%, and a full bar means the budget is spent. Above 100 + // the label stays unclamped: that asymmetry is the whole point of this + // function (see the doc comment). + return fmt.Sprintf("%s %.0f%%", bar, math.Floor(percentUsed)) +``` + +Aggiorna i chiamanti: quello di produzione, e `golden_test.go:198,203`, che +diventano `newBudget(lines).view(testTheme(true), 80)` e +`newBudget(nil).view(testTheme(true), 80)`. + +- [ ] **Step 4: Run, then regenerate the goldens** + +```bash +go test ./internal/tui -run TestBudget -v +go test ./internal/tui -race +go test ./internal/tui -update +git diff internal/tui/testdata/budget.golden +``` +Expected: test PASS; `budget.golden` scende da 94 a 80 colonne e perde +`(remaining …)`, con il nome a 29 colonne. `budget_empty.golden` non cambia (il +ramo vuoto ignora la larghezza). **Guardali** e riporta la forma nuova. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): fit the budget screen in 80 columns and make its label agree with its bar (#136)" +``` + +--- + +### Task 7: il range pinnato (#28) + +**Files:** +- Modify: `internal/tui/app.go` (`entriesMsg` a riga 54, `loadEntriesCmd` a 380-392, + handler `entriesMsg` a ~699-727, campi del `Model` a ~125-150, nuovo `activeRange`) +- Modify: `internal/tui/entries.go` (`entriesReloadedMsg` a 23-26, `reloadForBrowser` + a ~563, `updateEntryCmd` a ~436) +- Modify: `internal/tui/demo.go:180` (`demoEntriesCmd` produce `entriesMsg`) +- Modify: `internal/tui/report.go:67, 81, 127, 184`, `internal/tui/rates.go:854` +- Test: `internal/tui/range_pin_test.go` (nuovo) + +**Interfaces:** +- Produces: `Model.loadedStart`, `Model.loadedEnd`, + `Model.activeRange() (start, end time.Time)`. `entriesMsg` e + `entriesReloadedMsg` portano `start, end time.Time`. + +**Il punto che fa la differenza:** il pin non si ricalcola, si **trasporta**. +Ripinnare chiamando `currentRange()` nell'handler ricalcola il range *dopo* il +load: un load alle 23:59:59 su R1 il cui messaggio arriva alle 00:00:01 +pinnerebbe R2 ≠ R1, e il pin nascerebbe disallineato dalle entry che descrive — +cioè proprio il drift che esiste per eliminare. + +**Il fuso orario è parte della fixture, non un dettaglio.** `newTestModel()` non +imposta la timezone, quindi `m.loc == time.Local` e `RangeForPreset` risolve la +mezzanotte **locale**. Su una macchina a UTC+2 i due istanti attorno alla +mezzanotte UTC cadono nello stesso giorno locale e il range non si muove: il test +passerebbe in CI (UTC) e fallirebbe sulla macchina di chi sviluppa. Ogni test di +questo task fissa `m.loc = time.UTC`, come fa `goldenModel()`. + +- [ ] **Step 1: Write the failing test** + +```go +// A relative preset re-resolved time.Now() on every rebuild, so regrouping after +// midnight relabeled the report with a range the loaded entries never covered. +// The range is pinned by the load and carried in the message, so a rebuild +// cannot drift away from its own data (#28). +// +// m.loc is pinned to UTC on purpose: with time.Local the two instants below fall +// on the same local day east of Greenwich, the fixture stops exercising the +// drift, and the test would pass in CI while failing on a developer's machine. +func TestRebuildKeepsTheRangeTheEntriesWereLoadedFor(t *testing.T) { + before := time.Date(2026, time.July, 24, 23, 59, 59, 0, time.UTC) + after := time.Date(2026, time.July, 25, 0, 0, 1, 0, time.UTC) + now := before + m := newTestModel() + m.loc = time.UTC + m.preset = report.PresetLast7d + m.now = func() time.Time { return now } + + start, end := m.currentRange() + mm, _ := m.Update(entriesMsg{entries: goldenEntries(), start: start, end: end}) + m = mm.(Model) + if !m.report.Start.Equal(start) { + t.Fatalf("report.Start = %v, want the loaded range start %v", m.report.Start, start) + } + + now = after // midnight passes; last_7d would now resolve one day later + if newStart, _ := m.currentRange(); newStart.Equal(start) { + t.Fatal("the fixture does not exercise the drift: currentRange did not move across midnight") + } + if !m.applyReport() { + t.Fatal("applyReport returned false") + } + if !m.report.Start.Equal(start) { + t.Errorf("after midnight report.Start = %v, want the pinned %v", m.report.Start, start) + } +} + +// Home's label describes the range the NEXT load will fetch, so it must stay +// fresh: month, week mode and preset all change on Home WITHOUT a reload, and a +// pinned label would freeze while the user navigates. +func TestHomeLabelFollowsTheSelectionNotThePin(t *testing.T) { + m := newTestModel() + m.loc = time.UTC + m.now = func() time.Time { return time.Date(2026, time.July, 24, 12, 0, 0, 0, time.UTC) } + start, end := m.currentRange() + mm, _ := m.Update(entriesMsg{entries: goldenEntries(), start: start, end: end}) + m = mm.(Model) + pinned := m.rangeLabel() + + m.month-- // Home's PrevMonth, which triggers no reload + if got := m.rangeLabel(); got == pinned { + t.Errorf("rangeLabel stayed %q after changing month: the label must describe the next load", got) + } +} + +// Nothing is pinned before the first load, and the fallback is reachable in the +// live flow: the rates screen opens from Home and rebuilds the report there. +func TestActiveRangeFallsBackBeforeAnyLoad(t *testing.T) { + m := newTestModel() + m.loc = time.UTC + wantStart, wantEnd := m.currentRange() + gotStart, gotEnd := m.activeRange() + if !gotStart.Equal(wantStart) || !gotEnd.Equal(wantEnd) { + t.Errorf("activeRange() = (%v, %v), want currentRange() (%v, %v)", gotStart, gotEnd, wantStart, wantEnd) + } +} + +// A browser reload is a load: it must refresh the pin, not leave a stale one. +func TestBrowserReloadRepinsTheRange(t *testing.T) { + m := newTestModel() + m.loc = time.UTC + old := time.Date(2026, time.June, 1, 0, 0, 0, 0, time.UTC) + m.loadedStart, m.loadedEnd = old, old.AddDate(0, 1, 0) + start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + end := start.AddDate(0, 1, 0) + mm, _ := m.Update(entriesReloadedMsg{entries: goldenEntries(), status: "ok", start: start, end: end}) + m = mm.(Model) + if got, _ := m.activeRange(); !got.Equal(start) { + t.Errorf("activeRange() = %v after a browser reload, want the reloaded %v", got, start) + } +} +``` + +- [ ] **Step 2: Stage the RED so it is observable** + +I test non compilano finché `entriesMsg` non ha i campi e `activeRange` non +esiste — atteso. Per **vedere** il difetto: + +1. aggiungi **solo** i campi `start, end` ai due messaggi, i campi + `loadedStart/loadedEnd` al Model, e `activeRange()` (che a questo punto + ritorna sempre il fallback, perché niente pinna ancora); +2. fai passare `start`/`end` dai due cmd ai messaggi; +3. **non** toccare ancora i sei siti di rebuild né gli handler; +4. Run: `go test ./internal/tui -run TestRebuildKeepsTheRange -v` + +Expected: FAIL con `after midnight report.Start = 2026-07-19 …, want the pinned +2026-07-18 …` (un giorno di scarto). **Allega il transcript.** Se passa, il pin +non è la causa e la fixture non esercita il drift: correggi la fixture. + +- [ ] **Step 3: Implement** + +`app.go:54`: + +```go + // entriesMsg carries the range the load resolved, not just the entries: the + // handler pins that pair, so a rebuild after midnight cannot relabel the + // report with a range these entries never covered (#28). Re-deriving the + // range in the handler would defeat the point — a load at 23:59:59 whose + // message lands at 00:00:01 would pin the wrong day. + entriesMsg struct { + entries []report.TimeEntry + start, end time.Time + } +``` + +`entries.go:23-26`, uguale con il commento accorciato. Produttori: +`loadEntriesCmd` (`app.go:390`), `demoEntriesCmd` (`demo.go:180`) e +`reloadForBrowser` (`entries.go:564`) hanno già `start`/`end` in scope. + +`Model`, accanto agli altri campi di periodo: + +```go + // loadedStart/loadedEnd are the range the currently loaded entries were + // fetched for, pinned from the pair the load itself resolved (#28). Zero + // means nothing has been loaded yet. + loadedStart, loadedEnd time.Time +``` + +```go +// activeRange is the range the LOADED entries actually cover: the pinned pair +// when a load has happened, else what the next load would use. +// +// The split matters because a relative preset moves under you. Every rebuild +// over already-loaded entries goes through here, so it describes its own data. +// Every surface that describes the NEXT load — Home's label, and the loads +// themselves — uses currentRange() instead: Home changes month, week mode and +// preset WITHOUT reloading, so a pinned label would freeze while the user +// navigates. +func (m Model) activeRange() (start, end time.Time) { + if !m.loadedStart.IsZero() { + return m.loadedStart, m.loadedEnd + } + return m.currentRange() +} +``` + +Nei due handler, **prima** di ricostruire: +`m.loadedStart, m.loadedEnd = msg.start, msg.end`, e usa `msg.start, msg.end` +per la `report.Build` immediata dell'handler `entriesMsg`. + +Sostituisci `m.currentRange()` con `m.activeRange()` **solo** in questi sei +punti: `entries.go:436` (`updateEntryCmd` — «moved outside the current range» +deve riferirsi al range delle entry a schermo), `report.go:67` (`dailySeries`), +`report.go:81`, `report.go:127`, `report.go:184`, `rates.go:854`. + +Lascia `currentRange()` a `home.go:99`, `app.go:287`, `entries.go:564`. Aggiungi +a `rangeLabel` un commento di una riga che dica perché è fresco. + +- [ ] **Step 4: Run** + +Run: `go test ./internal/tui -race` +Expected: PASS. Nessun golden si muove: il pin non cambia il rendering, solo +quale range viene usato. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "fix(tui): pin the resolved range to the loaded entries (#28)" +``` + +--- + +### Task 8: lo scroll dei Filters (#28) + +**Files:** +- Modify: `internal/tui/palette.go` (`scrollPalette` a 139-151) +- Modify: `internal/tui/filters.go` (`filtersModel`, `updateFilters`, `view`) +- Modify: `internal/tui/golden_test.go:219` — `TestGoldenFilters` chiama + `view(testTheme(true))` e va aggiornato +- Modify: il chiamante di produzione di `filtersModel.view` +- Test: `internal/tui/filters_test.go` +- Test: `internal/tui/testdata/filters.golden`, più un golden nuovo per la + vista finestrata + +**Interfaces:** +- Produces: `scrollWindow(idx, top, rows int) int`; `filtersModel.top`; + `filtersCursorRow(fs filtersModel) int`; + `filtersVisualRows(fs filtersModel, th theme) []string`; + `filtersRows(height int) int`; `filtersModel.view(th theme, rows int) string`. + +**Il contratto, fissato qui perché due letture producono due UI diverse:** +`view(th, rows)` emette **al più `rows` righe totali**, titolo e riga vuota +compresi; la finestra sulle righe di contenuto è quindi `rows - 2`. +`filtersRows(height)` sottrae solo la riga vuota e il footer che `View` aggiunge +sempre — **non** il titolo, che è dentro il budget — con un floor di 6 perché la +schermata si restringe ma non svanisce. `filtersVisualRows` produce **solo le +righe di contenuto**, senza titolo. + +**La proiezione, decisa nella spec:** la finestra è sulle **righe visuali +dell'intera vista** (header di sezione inclusi), non sulle opzioni della sola +sezione attiva. L'overflow di cui si lamenta la #28 è quello della vista intera, +e una finestra per-sezione lascerebbe fuori schermo gli header delle sezioni +successive. L'indice del cursore è la riga visuale su cui sta `(sec, row)`. + +**Le fixture: le opzioni vengono dalle `entries`.** `newFilters` costruisce +`options` da `entries` (`filters.go:74-84`); le mappe che riceve sono solo lo +stato *selected*. Passare 40 chiavi come `lists` non crea 40 opzioni: crea +**zero** opzioni, un cursore che non si muove, e un test che passa contro +qualunque implementazione, compresa una che non scrolla. Costruisci le entry: + +```go +// filtersFixture builds entries whose list names produce n Lists options, plus +// one tag and one status, so the Filters screen has something to scroll. +func filtersFixture(n int) []report.TimeEntry { + entries := make([]report.TimeEntry, 0, n) + for i := range n { + entries = append(entries, report.TimeEntry{ + ListName: fmt.Sprintf("List %02d", i), + Tags: []string{"backend"}, + Status: "done", + }) + } + return entries +} +``` + +- [ ] **Step 1: Write the failing test** + +```go +// The Filters screen had no window at all: many lists or tags simply ran off the +// bottom of a short terminal, with no way to reach them (#28). +func TestFiltersViewNeverExceedsItsRowBudget(t *testing.T) { + th := testTheme(true) + fs := newFilters(filtersFixture(40), nil, nil, nil, nil) + for _, rows := range []int{6, 10, 20} { + if got := strings.Count(fs.view(th, rows), "\n") + 1; got > rows { + t.Errorf("row budget %d: view rendered %d lines", rows, got) + } + } +} + +// The window must follow the cursor. Asserting on the rendered cursor is not an +// option: under termenv.Ascii the active row is byte-identical to the others +// (th.Accent renders nothing), and "▸" marks the section header, which scrolls +// out. So assert on the window itself, which is what the feature actually is. +func TestFiltersWindowFollowsTheCursor(t *testing.T) { + m := newTestModel() + m.filtersScreen = newFilters(filtersFixture(40), nil, nil, nil, nil) + m.screen = screenFilters + m.height = 12 + for range 30 { // walk down well past the first window + mm, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m = mm.(Model) + } + fs := m.filtersScreen + if fs.row == 0 && fs.sec == 0 { + t.Fatal("the cursor never moved: the fixture has no options to walk") + } + row, rows := filtersCursorRow(fs), filtersRows(m.height)-2 + if row < fs.top || row >= fs.top+rows { + t.Errorf("cursor at visual row %d is outside the window [%d, %d)", row, fs.top, fs.top+rows) + } +} + +// scrollWindow is the palette's own scrolling, extracted so both screens share +// one idiom. Same behavior, now with a name. +func TestScrollWindow(t *testing.T) { + tests := []struct { + name string + idx, top, rows, want int + }{ + {"inside stays", 3, 2, 5, 2}, + {"above pulls up", 1, 4, 5, 1}, + {"below pushes down", 9, 2, 5, 5}, + {"first row", 0, 0, 5, 0}, + {"zero rows", 7, 3, 0, 0}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := scrollWindow(tc.idx, tc.top, tc.rows); got != tc.want { + t.Errorf("scrollWindow(%d, %d, %d) = %d, want %d", tc.idx, tc.top, tc.rows, got, tc.want) + } + }) + } +} + +// The cursor's visual row must account for section headers and the "(none)" +// placeholder an empty section still renders, or the window scrolls to the wrong +// place. With two Lists options, Lists row 1 is visual row 2 (header + first +// option); the Tags section that follows starts after header + 2 options. +func TestFiltersCursorRowCountsHeadersAndOptions(t *testing.T) { + fs := newFilters(filtersFixture(2), nil, nil, nil, nil) + fs.sec, fs.row = 0, 1 + if got := filtersCursorRow(fs); got != 2 { + t.Errorf("filtersCursorRow at Lists row 1 = %d, want 2", got) + } + fs.sec, fs.row = 1, 0 + if got := filtersCursorRow(fs); got != 4 { + t.Errorf("filtersCursorRow at Tags row 0 = %d, want 4", got) + } +} +``` + +Prima di eseguire, **verifica i due numeri 2 e 4 contro la struttura reale**: +`filtersFixture(2)` produce 2 opzioni in Lists (`List 00`, `List 01`), 1 in Tags +(`backend`), 1 in Statuses (`done`), 3 in Billable. Se `newFilters` ordina o +raggruppa diversamente da come credi, aggiusta i numeri attesi al comportamento +reale — non il comportamento ai numeri. + +- [ ] **Step 2: Stage the RED** + +Aggiungi **solo** la firma `view(th theme, rows int)` che ignora `rows`, più +`filtersRows` e `filtersCursorRow`, senza finestrare niente. Poi: + +Run: `go test ./internal/tui -run TestFiltersViewNeverExceedsItsRowBudget -v` +Expected: FAIL su tutti e tre i budget, con un numero di righe pari alla vista +intera (con 40 liste: 2 header+opzioni per quattro sezioni — conta quante escono +e riportalo). **Allega il transcript.** + +- [ ] **Step 3: Implement** + +In `palette.go`, estrai: + +```go +// scrollWindow moves a visible window of `rows` rows so idx stays inside it, and +// returns the new top. Shared by the palette and the Filters screen, so there is +// one scrolling idiom in this package rather than two (#28). +func scrollWindow(idx, top, rows int) int { + if rows <= 0 { + return 0 + } + if idx < top { + return idx + } + if idx >= top+rows { + return idx - rows + 1 + } + return top +} +``` + +`scrollPalette` diventa `p.top = scrollWindow(p.idx, p.top, rows); return p`. +Sposta la nota sulla #28 dal commento di `scrollPalette` a quello di +`scrollWindow`, aggiornandola: adesso i Filters ce l'hanno. + +In `filters.go`, ristruttura `view` in due passi — costruire tutte le righe +visuali, poi finestrarle. È anche ciò che rende la proiezione testabile: + +```go +// filtersVisualRows renders the screen's CONTENT as one flat slice of visual +// rows: a section header, then its options (or a single "(none)"), for each +// section. The title is not part of the slice. The window in view() is taken +// over THIS slice, so the cursor's index and the rendering can never disagree +// about what row something is on. +func filtersVisualRows(fs filtersModel, th theme) []string + +// filtersCursorRow is the visual row (sec, row) sits on — the index +// filtersVisualRows puts it at. Headers and the "(none)" placeholder count. +func filtersCursorRow(fs filtersModel) int { + n := 0 + for si, sec := range fs.sections { + if si == fs.sec { + return n + 1 + fs.row // this section's header, then row + } + n += 1 + max(1, len(sec.options)) // header + options, or the "(none)" row + } + return n +} + +// filtersRows is how many lines view() may emit in total, title included. The +// subtraction accounts for the blank line and the footer that View always +// appends; the floor says the screen shrinks but never vanishes. +func filtersRows(height int) int { + return max(6, height-2) +} +``` + +`filtersModel` prende `top int`; `updateFilters` chiude ogni movimento del +cursore con +`fs.top = scrollWindow(filtersCursorRow(fs), fs.top, filtersRows(m.height)-2)`, +e scrive indietro `m.filtersScreen = fs` **prima** del return, come vuole il +pattern del pacchetto. + +- [ ] **Step 4: Run and add a golden** + +```bash +go test ./internal/tui -run 'TestFilters|TestScrollWindow' -v +go test ./internal/tui -race +go test ./internal/tui -update +``` +Aggiorna `golden_test.go:219` in `newFilters(goldenEntries(), nil, nil, nil, nil).view(testTheme(true), 24)` +— 24 righe, abbastanza per la vista intera delle fixture golden, così +`filters.golden` non cambia. Poi aggiungi un golden nuovo per la vista +finestrata: `filtersFixture(40)` reso con `rows = 12`. **Guardalo**: deve +mostrare il taglio in fondo e il cursore dentro. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui +git commit -m "feat(tui): scroll the Filters screen on the palette's shared window (#28)" +``` + +--- + +### Task 9: il resto della #28 — `task_tags`, micro-nit, densità demo + +> **Dipende dal Task 7**: estrae `rebuildReport` dagli stessi blocchi che il +> Task 7 modifica. Non eseguirlo prima. + +**Files:** +- Modify: `internal/clickup/timeentries.go` (`rawEntry.TaskTags`, `rawEntry.Tags`) +- Test: `internal/clickup/timeentries_test.go` +- Modify: `internal/report/filter.go` (`Filter`) +- Modify: `internal/tui/report.go` (`applyReport` → `rebuildReport`), + `internal/tui/app.go`, `internal/tui/rates.go` +- Test: `internal/tui/demo_test.go` + +**Interfaces:** +- Produces: `clickup.tagName` con `UnmarshalJSON`; + `Model.rebuildReport(groupBy string) bool`. + +- [ ] **Step 1: Write the failing test** + +`internal/clickup/timeentries_test.go` — segui il pattern httptest già presente +nel file per costruire il client e puntarne il base URL: + +```go +// ClickUp's documented example returns task_tags as objects +// ({name, tag_fg, tag_bg, creator}), but the reference's generated schema for +// the date-range endpoint types it as an array of strings. A bare string used to +// fail the WHOLE decode — "cannot unmarshal string into Go struct field +// rawEntry.data.task_tags" — losing every entry, not just the tags (#28). +func TestTimeEntriesAcceptsBothTagShapes(t *testing.T) { + tests := []struct { + name string + tags string + want []string + }{ + {"objects, as documented", `[{"name":"urgent"},{"name":"frontend"}]`, []string{"urgent", "frontend"}}, + {"bare strings, as the generated schema types it", `["urgent","frontend"]`, []string{"urgent", "frontend"}}, + {"mixed", `[{"name":"urgent"},"frontend"]`, []string{"urgent", "frontend"}}, + {"empty", `[]`, nil}, + {"null", `null`, nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // ... httptest server returning one entry whose task_tags is tc.tags, + // following this file's existing fixture shape ... + if !slices.Equal(got[0].Tags, tc.want) { + t.Errorf("Tags = %v, want %v", got[0].Tags, tc.want) + } + }) + } +} +``` + +`slices.Equal` tratta `nil` e `[]string{}` come uguali, quindi i casi +`empty`/`null` passano qualunque delle due cose produca `toTimeEntry`: non è una +differenza da inseguire. + +`internal/tui/demo_test.go`: + +```go +// Demo mode used to be near-empty for the non-month presets because the fixtures +// lived on fixed month days; #4 made them day offsets wrapped modulo the range +// span. This pins that, so the presets stay demoable (#28). It is a +// regression-guard on a box that is ALREADY closed: it must pass on the first +// run, and a failure means #4's wrapping regressed. +func TestDemoEntriesPopulateEveryPreset(t *testing.T) { + now := time.Date(2026, time.July, 24, 12, 0, 0, 0, time.UTC) + for _, preset := range []string{report.PresetThisMonth, report.PresetLast7d, report.PresetLast30d, report.PresetThisWeek} { + t.Run(preset, func(t *testing.T) { + start, end := report.RangeForPreset(preset, 2026, time.July, now, time.UTC) + entries := demoEntries(start, end) + if len(entries) != 8 { + t.Errorf("got %d entries, want all 8 fixtures inside the range", len(entries)) + } + for _, e := range entries { + if e.Start.Before(start) || !e.Start.Before(end) { + t.Errorf("entry %s at %v falls outside [%v, %v)", e.ID, e.Start, start, end) + } + } + }) + } +} +``` + +- [ ] **Step 2: Run to verify the tag test fails** + +Run: `go test ./internal/clickup -run TestTimeEntriesAcceptsBothTagShapes -v` +Expected: `bare strings` e `mixed` FAIL con +`json: cannot unmarshal string into Go struct field`. **Allega il transcript.** + +Run: `go test ./internal/tui -run TestDemoEntriesPopulateEveryPreset -v` +Expected: PASS al primo colpo. Dichiaralo nel report come regression-guard, non +come fix. + +- [ ] **Step 3: Implement** + +`internal/clickup/timeentries.go`: + +```go +// tagName is a tag as ClickUp returns it on a time entry. The documented example +// is an object ({name, tag_fg, tag_bg, creator}), but the API reference's +// generated schema for the date-range endpoint types task_tags as an array of +// strings. A bare string used to fail the WHOLE TimeEntries decode, not just the +// tags, so both shapes are accepted and the assumption is gone rather than +// annotated (#28). A live confirmation against a real workspace is tracked in +// #129. +type tagName struct{ Name string } + +func (t *tagName) UnmarshalJSON(b []byte) error { + var s string + if err := json.Unmarshal(b, &s); err == nil { + t.Name = s + return nil + } + var o struct { + Name string `json:"name"` + } + if err := json.Unmarshal(b, &o); err != nil { + return fmt.Errorf("tagName: unhandled value: %s", b) + } + t.Name = o.Name + return nil +} +``` + +`rawEntry.TaskTags` e `rawEntry.Tags` diventano `[]tagName`; i due cicli in +`toTimeEntry` restano identici. + +`internal/report/filter.go` — `Filter` calcola i tre conteggi una volta e deriva +l'emptiness da quelli (`nL+nT+nS == 0 && c.Billable == nil`) invece di chiamare +`Empty()` e poi ricontare. **`Empty()` resta esattamente com'è**: è API esportata, +usata da `internal/cli/report.go:139` e `internal/tui/app.go:369`, e memoizzare +conteggi sulla struct la esporrebbe a hazard di copia. Il comportamento non +cambia: se un test esistente si muove, hai cambiato semantica — torna indietro. + +`internal/tui` — estrai il blocco triplicato: + +```go +// rebuildReport rebuilds m.report and m.rep for the given grouping, over the +// visible entries and the pinned range. It returns false when the timezone or +// the pricing rules failed to parse, in which case the caller must not overwrite +// the error screen those helpers already routed to. It is the single rebuild +// used by applyReport, by the entries handler and by the grouping and rates +// changes, which each carried their own copy. +func (m *Model) rebuildReport(groupBy string) bool +``` + +Chiamanti: `report.go:83`, `app.go:719`, `rates.go:855`, e `applyReport` stesso, +che resta come wrapper che risolve il grouping di default. I tre chiamanti sono +metodi a ricevitore **valore**: `m.rebuildReport(g)` su un ricevitore valore +compila (la copia locale è indirizzabile) e muta quella copia, che è esattamente +ciò che serve dato che poi la ritornano. + +**Attenzione all'handler `entriesMsg`**: dopo il Task 7 usa `msg.start`/`msg.end` +e non `activeRange()`. Se lo fai passare per `rebuildReport`, il pin va scritto +**prima** della chiamata, così `activeRange()` dentro `rebuildReport` legge già +la coppia nuova. Verifica con `TestRebuildKeepsTheRangeTheEntriesWereLoadedFor`. + +- [ ] **Step 4: Run** + +```bash +go test ./internal/clickup ./internal/report ./internal/tui -race +``` +Expected: PASS, nessun golden mosso. + +- [ ] **Step 5: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/ +git commit -m "fix(clickup): accept both documented shapes of task_tags (#28)" +``` + +--- + +### Task 10: documentazione e chiusura + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: `README.md`, `README.it.md` (solo se mostrano la riga budget) + +- [ ] **Step 1: CHANGELOG** + +Sotto `## [Unreleased]`, in inglese, una riga per issue chiusa. + +- [ ] **Step 2: Verifica che i README non mentano** + +```bash +grep -n "remaining\|budget\|burn-down" README.md README.it.md | head -20 +``` +La tabella dei tasti non cambia (nessun tasto nuovo). Se un README mostra un +esempio della riga budget, aggiornalo alla forma a 80 colonne. + +- [ ] **Step 3: Full gate + commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add CHANGELOG.md README.md README.it.md +git commit -m "docs: changelog for v1.9 tranche D" +``` + +--- + +## Self-review del piano + +**Copertura della spec.** §4.1 → Task 1. §4.2 → Task 2. §4.3 → Task 3. §4.4 → +Task 4. §4.5 → Task 6. §4.6 → Task 5. §4.7 → Task 7 (range) e Task 8 (scroll). +§4.8 → Task 9. §6 è vincolo globale. §5 (fuori scope) non genera task. + +**Ordine e dipendenze.** `1 → 2 → {3,4,5,6}`; `4` prima di `5`; `7 → 9`; `8` +indipendente. La dipendenza `7 → 9` è reale e va rispettata: il Task 9 riscrive +le stesse righe in cui il Task 7 mette `activeRange()` e `msg.start`. + +**Firme.** Gli helper esistenti sono elencati in cima con la firma verificata nel +repo — in particolare `newTestModel()` **senza parametri** e il campo +`m.filtersScreen`. `newFilters` deriva le opzioni dalle **entries**, ed è per +questo che il Task 8 porta la sua `filtersFixture`. Nessun nome definito in un +task è usato con un'ortografia diversa in un altro. + +**RED osservabili.** Task 2, 5, 6, 7 e 8 separano i test comportamentali (che +girano contro il codice attuale) da quelli che nominano simboli nuovi, perché un +`FAIL [build failed]` non è un RED. Task 1 è l'eccezione dichiarata: là il +build-failure è tutto ciò che può esistere, non c'è difetto da esercitare. + +**Larghezze.** Ogni larghezza asserita nei test viene dallo sweep misurato del +Task 5 e dalla soglia misurata del Task 6. Nessun test pretende una larghezza +sotto il minimo del codice corretto: era l'errore che rendeva impossibili tre +test della versione precedente di questo piano. + +**Placeholder.** Nessuno nel codice da scrivere. Tre punti rimandano +deliberatamente alla verifica dell'implementatore contro il repo, e lo dicono: il +pattern httptest di `internal/clickup` (Task 9 Step 1), i numeri 2 e 4 di +`filtersCursorRow` (Task 8 Step 1), e il conteggio esatto delle righe nel RED del +Task 8 Step 2. Sono verifiche, non decisioni lasciate aperte. diff --git a/docs/superpowers/specs/2026-07-30-tranche-d-layout-correctness-design.md b/docs/superpowers/specs/2026-07-30-tranche-d-layout-correctness-design.md new file mode 100644 index 0000000..55d4b6d --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-tranche-d-layout-correctness-design.md @@ -0,0 +1,379 @@ +# Tranche D — layout & correttezza (design) + +> v1.9, tranche D. Chiude **6 issue**: #137, #136, #135, #141, #138 (punto 1) e +> le caselle aperte della #28. Le tranche A/B/C hanno già portato theme registry +> con renderer iniettato, footer/help, keymap + nav stack, overlay compositor, +> command palette, tabella report `lipgloss/table`, gauge e sparkline. + +## 1. Obiettivo + +Le schermate misurano i layout in **colonne di display**; tre helper diversi +tagliano e riempiono in **rune**. Le due cose coincidono solo in ASCII, e i nomi +di lista e task di ClickUp sono esattamente il posto dove vivono emoji e CJK. +Questa tranche unifica il taglio su una primitiva sola, chiude tre difetti di +layout misurati e smarca il debito residuo della #28. + +Non è una tranche di feature: nessuna schermata nuova, nessun tasto nuovo, +nessuna chiamata API nuova. Il valore è che invarianti già dichiarate nei +commenti diventino vere. + +## 2. Evidenza misurata + +Ogni numero qui sotto è stato prodotto eseguendo codice, e **ogni numero porta +la fixture che lo genera** — un numero senza fixture non è rigenerabile, quindi +non è una misura. Tre affermazioni delle issue sono risultate sbagliate, una +casella è risultata già chiusa, e una correzione che avevo scritto in bozza era +a sua volta sbagliata. + +### 2.1 Larghezze e rune + +| Affermazione | Verdetto | +|---|---| +| `truncate` (`report.go:270`) taglia per rune | **Vero.** | +| `fmt` riempie `%-Ns` in rune, non in colonne | **Vero.** `"日本語のリスト"` (7 rune, 14 colonne) con `%-10s` riceve 3 spazi e esce a 17 colonne. Sbagliano **entrambe** le metà della coppia `truncate(s,N)` + `%-Ns`: sistemare solo il taglio lascia la colonna disallineata. | + +### 2.2 `ansi.Truncate` (x/ansi v0.11.6) + +Tutto misurato, niente dedotto: + +- larghezza in colonne di display; `n=0` → `""`; `n=1` → `"…"`; non panica. +- mai più larga di `n`. +- su un cluster largo a cavallo del taglio **scarta l'intero cluster** e può + tornare **più stretta** di `n`: `Truncate("日本語のリスト", 2, "…")` = `"…"` + (1 colonna, non 2). È per questo che serve un pad misurato dopo il taglio. +- **È ANSI-aware**: `Truncate("\x1b[1mabcdef\x1b[0m", 3, "…")` = + `"\x1b[1mab…\x1b[0m"`, larghezza 3, sequenze preservate. +- **Ma non chiude uno stile aperto**: `Truncate("\x1b[31mabcdef", 3, "…")` = + `"\x1b[31mab…"` — nessun reset in coda, lo stile cola oltre il taglio. Conta + per §4.3: l'input di `clampWidth` viene da `help.View`, che chiude i propri + stili, ma l'invariante va inchiodata da un test, non assunta. +- `Truncate("ab\ncd", 3, "…")` = `"ab\n…"`: il newline passa. Gli helper di + §3 dichiarano input a riga singola. + +### 2.3 I tre difetti di layout + +| # | Cosa dice la issue | Cosa dice la misura | +|---|---|---| +| #136.1 | la riga è larga 94 colonne | **Vero per il golden**: `testdata/budget.golden` con `625.00 / 1000.00 EUR (remaining 375.00)` misura 94 colonne (90 di contenuto + 4 di `th.Box`). Non è un numero fisso: con `337.50 / 450.00` fa 93, con `12.50 / 20.00` fa 89. **Cresce con gli importi, nessun clamp.** Il minimo teorico è ~86: sfora 80 sempre. | +| #136.2 | fra 99,5% e 100% label e barra litigano | **Vero al centesimo**: 99,4 → `99%` + 19 blocchi (accordo); 99,5 / 99,6 / 99,9 → `100%` + 19 blocchi (disaccordo); 100 → `100%` + 20. E 97,6 → `98%` + 19 blocchi: è la misura che decide §4.5. | +| #137 | `Total` duplica `TOTAL` | **Vero.** Fixture: `demoEntries` su luglio 2026, `Rates{Default:50}`, `Currencies{mobile:USD}`, `DefaultCurrency:EUR`, `GroupByTotal` → riga bucket `["Total" "18.00" "17.00" "337.50 EUR + 512.50 USD"]` e riga totali `["TOTAL" "18.00" "17.00" ""]`. (La issue cita `308.75 + 90.00`: altra fixture, stesso difetto.) | +| #138.1 | 84 colonne in un terminale da 80 | **Vero, ma serve la congiunzione**: label lunga **e** Hours/Billed più larghi di 8. Fixture `{label 46 col, 1234567.50, 1234567.50, "625.00 EUR"}` → 64 col a width 60, 84 a width 80. Con la stessa label e `12.50` non sfora mai (55 col a width 60). Overflow esatto = 2 × (larghezza numerica − 8). | +| #138.1 | «5 colonne inutilizzate a width 60» | **La issue aveva ragione per il suo caso, la mia bozza aveva torto.** Con label lunghe e cifre normali (il caso dei golden `report`/`report_narrow`) l'inutilizzato è 5. Con label corte è 21+, perché Item è tappato dalla label più lunga e la tabella non si allarga mai. Dipende dalla fixture, e va detto così. | +| #138.1 | (corollario) | «con label corte non sfora mai» è **falso**, ma il numero va misurato **rendendo**, non calcolando: label corta + Hours/Billed a 10 caratteri rende **44** colonne a width 40 (a mano ne avevo dedotte 47, dimenticando che `reportAmountWidth` riprende spazio ad Amount come ultima risorsa; 47 è la larghezza *naturale*, che si ottiene a ogni width ≥ 48). | +| #138.1 | (limite del fix) | Dopo il fix la tabella **non può** stare in 40 colonne con cifre a 10: il minimo è `chrome(10) + floor Item + 10 + 10 + header Amount(6)` = **48** con label lunghe e **43** con label corte. Misurato rendendo. Un test che pretende ≤ 40 fallisce contro il codice corretto. | + +### 2.4 Il resto della #28 + +- **Demo, preset non mensili: casella già chiusa.** `demoEntries` usa giorni + offset modulo lo span dalla #4. Misurato: `this_month` 8 entry / 7 giorni + distinti, `last_7d` 8/5, `last_30d` 8/7, `this_week` 8/5. Nessuna è vuota. +- **`task_tags` con una stringa nuda non degrada, esplode.** Verificato con + `httptest`: `json: cannot unmarshal string into Go struct field + rawEntry.data.task_tags`. Fallisce l'intera `TimeEntries`, non solo i tag. +- **`lipgloss.NewStyle()` in produzione sotto `internal/`: una sola + occorrenza**, `footer.go:62`. L'altro hit del grep è un commento in + `report_table.go:163`. La seconda casella della #141 è quindi già + soddisfatta: va registrata, non lavorata. +- **`countTrue` è scandito due volte** (`Filter` chiama `Empty()` e poi + ricalcola, `filter.go:40-43`) e i tre blocchi duplicati di rebuild esistono + (`report.go:83`, `app.go:719`, `rates.go:855`). + +### 2.5 Fatti che decidono l'implementazione + +- **L'header determina la larghezza di colonna** in `lipgloss/table`: a parità + di celle `"12.50"`, header `"H"` → 7 colonne, header `"Billed"` → 8. Quindi + l'aritmetica di budget di §4.6 **deve** misurare Hours e Billed includendo + gli header, come `reportItemWidth` già fa per Item e Amount + (`report_table.go:102-103`). Misurando solo i dati il budget assume 5+5=10 + mentre il renderer usa 5+6=11, e la tabella sfora di 1. +- **`th.Box` costa esattamente 4 colonne** (RoundedBorder + `Padding(0,1)`, + `theme.go:54`). Misurato: `Box.Render("12345")` → 9 colonne. +- **Su Home il cambio di mese, di settimana e di preset non ricarica niente.** + `home.go:33-53` muta `month`/`periodMode`, `range.go:139-141` scrive + `m.preset` e fa `pop`; il reload parte solo da `k.Generate` + (`home.go:75-79`). Decide §4.7. + +## 3. Architettura + +Una primitiva sotto tutto: `ansi.Truncate`, già dipendenza diretta dalla #59. +File nuovo `internal/tui/width.go`: + +```go +// truncateWidth cuts s to at most cols DISPLAY columns, ellipsis included. +// cols <= 0 returns "". s must be a single line: the behavior with "\n" is +// undefined (ansi.Truncate lets the newline through). +func truncateWidth(s string, cols int) string + +// cell renders s into exactly cols display columns: truncated when too wide, +// space-padded when too narrow. It replaces the truncate(s,N) + "%-Ns" pair, +// where BOTH halves measured runes instead of columns. cols <= 0 returns "". +// Same single-line contract as truncateWidth. +func cell(s string, cols int) string +``` + +Il pad non è cosmetico: siccome `ansi.Truncate` può tornare più stretta di +`cols` su un cluster largo (§2.2), senza pad misurato la colonna si disallinea +comunque, solo nell'altra direzione. + +Conseguenze: la `truncate` rune-based **si elimina**, e con lei la +`shaveToWidth` di `report_table.go`, che è una shave-loop che fa a mano ciò che +`ansi.Truncate` fa in un passo. + +`shaveToWidth` ha **5 chiamanti**, non 2: `report_table.go:209,210` e — questo +è il punto che si perde facilmente — `palette.go:214,219,253`. Eliminarla senza +migrare anche il palette non compila. Anche `report.go:269`, +`report_table.go:142` e `report_table.go:195` la citano nei commenti e vanno +aggiornati, altrimenti restano a nominare funzioni che non esistono più. + +**Divergenza voluta da `truncate`, a `cols == 1`:** `truncate("x", 1)` = `"…"`, +perché la guardia `n <= 1` scatta prima del controllo "ci sta" +(`report.go:274`). `cell("x", 1)` = `"x"`. Il comportamento nuovo è quello +giusto e nessun call site usa `cols < 20`, ma la frase «per ASCII l'output è +identico» vale **per `cols >= 2`**. + +## 4. Gli interventi + +### 4.1 `width.go` e i due helper (#135, prima metà) + +Nuovo file, due funzioni, test con fixture a rune larghe **obbligatorie** e i +casi di bordo di §2.2 (`cols<=0`, `cols==1`, cluster a cavallo, input con +escape terminati e non terminati). + +### 4.2 Migrazione dei call site (#135, seconda metà) + +**15 siti da migrare**, su 17 righe elencate — le altre 2 +(`report_table.go:254,256`) stanno dentro `shaveToWidth`, che sparisce con la +funzione. (La bozza diceva 19: era un grep che contava anche le due menzioni di +`truncate(...)` nel commento a `report_table.go:244-245`.) + +- `budget.go:75` +- `rates_view.go:53, 57, 76, 82, 92, 93, 143, 147, 149` +- `entries.go:583, 593, 643, 648` +- `log.go:540` + +Regola: coppia `truncate(s,N)` + verbo `%-Ns` → `cell(s, N)` con `%s`; sito +senza pad → `truncateWidth(s, N)`. + +**Proprietà attesa:** per contenuto ASCII e `cols >= 2` l'output è identico, e i +golden esistenti restano verdi. Va **verificata rigenerando**, non dichiarata: +la spec non ha implementato la migrazione. + +### 4.3 `clampWidth` senza renderer di default (#141) + +```go +return ansi.Truncate(s, width-1, "") + th.Help.Render("…") +``` + +Più un test che inchioda che l'input non porta stili aperti oltre il taglio +(§2.2: `ansi.Truncate` non chiude una SGR non terminata). + +Il commento deve dire che i test **non possono** accorgersi della differenza, +perché `TestMain` fissa `termenv.Ascii` e uno stile costruito sul renderer di +default rende identico a uno costruito su quello iniettato — così nessuno lo +reintroduce credendo che la suite lo prenda. + +La seconda casella della #141 («grep di ogni `lipgloss.NewStyle()`») è **già +soddisfatta**: §2.4 la registra, non c'è lavoro. + +### 4.4 La riga `Total` duplicata (#137) + +In `reportRows`: se `r.GroupBy == report.GroupByTotal && len(r.Buckets) == 1`, +non emettere la riga bucket. `firstTotal` diventa 0, e con `firstTotal == 0` lo +`StyleFunc` fa già la cosa giusta (riga 0 = `TOTAL`, in grassetto). Il caso +`len(Buckets) == 0` non arriva a `reportRows`: `report.go:208` intercetta con +«No hours to show.». + +Per `GroupByTotal` il bucket unico raccoglie tutte le entry, quindi i suoi +Amounts sono **esattamente** i subtotali: sopprimerlo non perde nulla, nemmeno +il caveat sul drift di arrotondamento che vale alle granularità fini. + +**Nessun golden esistente si muove**: tutti i golden del report girano su +`goldenReport()`, che è `GroupByList`, e in `testdata/` non esiste una riga +bucket `Total`. Vanno quindi **creati** due golden nuovi sotto `GroupByTotal`, +uno a valuta singola e uno multi-valuta. Un piano che si aspetta un golden +rosso qui sta descrivendo qualcosa che non c'è. + +### 4.5 Il budget in 80 colonne (#136) + +`budgetModel.view` riceve la larghezza come già fa `reportModel.view`. + +``` +usable = width - 4 // th.Box: misurato, non assunto +fixed = 1 + budgetBarWidth + 1 + pctW + 2 +nameW = usable - fixed - figuresW +``` + +`pctW` e `figuresW` sono i massimi **misurati sulle righe vere** (le cifre non +hanno larghezza fissa: è per questo che una riserva costante si rompe). Nome +reso con `cell(name, nameW)`. + +**Ordine di valutazione, una passata sola** — la circolarità va rotta, non +lasciata all'implementatore: + +1. calcola `figuresW` **con** `(remaining …)` e da lì `nameW`; +2. se `nameW < budgetMinNameWidth` (**costante = 12**, la stessa soglia sotto + cui `reportMinItemWidth` dice che «le label smettono di essere label»), + ricalcola `figuresW` **senza** `(remaining …)` e da lì `nameW`, e non lo + reintroduce; +3. se `nameW` resta sotto la soglia anche così, `nameW = budgetMinNameWidth` e + **la riga sfora**. Accettato ed esplicito, esattamente come la tabella + report accetta di sforare sotto la sua soglia. Un `nameW` negativo non deve + mai raggiungere `cell` (che comunque ritorna `""`). + +La soglia sotto cui si sfora è +`4 + 1 + budgetBarWidth + 1 + pctW + 2 + figuresW(senza remaining) + 12`, cioè +**63** con le cifre del golden (`625.00 / 1000.00 EUR`, pct 3) e **65** con +importi a quattro cifre e una percentuale sopra 100 (pct 4). Misurato +simulando la funzione. Non è «~48»: quel numero vale per la tabella report +(`reportAmountWidth`) e applicarlo qui farebbe credere che 60 colonne stiano +sopra la soglia di degrado, quando invece è la prima larghezza che sfora. + +`width <= 0` (primo render, prima del `WindowSizeMsg`) → layout naturale +attuale, `nameW = 24`, stesso fallback che `reportItemWidth` ha già +(`report_table.go:108-110`). + +A width 80 col golden attuale: con `(remaining)` `nameW` sarebbe 10 → sotto 12 → +si ricalcola senza, `nameW = 29`. + +**La barra:** si arrotonda **per difetto la label**, non per eccesso il +riempimento. Arrotondare il riempimento porterebbe la barra piena a 97,6% sotto +una label `98%` (§2.3), e una barra piena significa «budget esaurito». Meglio +`99%` con 19 blocchi che `100%` con 20 a budget non finito. La label resta non +clampata sopra 100: è il motivo per cui questa schermata non usa +`bubbles/progress`. + +### 4.6 Le colonne numeriche misurate (#138, punto 1) + +`reportItemWidth` e `reportAmountWidth` misurano Hours e Billed dal contenuto +**inclusi gli header** (§2.5) invece di sottrarre `2*reportNumWidth`; la +costante sparisce. + +Il guadagno per Item dove è vincolato dal budget è **+5** (16 riservate → 11 +misurate: `"Hours"`=5, `"Billed"`=6), non +6. Misurando solo i dati il budget +assumerebbe 10 e il renderer userebbe 11: la tabella sforerebbe di 1, cioè +proprio la classe di bug che questa tranche chiude. + +Il test vuole la **congiunzione**: label lunga **e** Hours/Billed più larghi di +8. Con label lunga e cifre normali la tabella non sfora (55 col a width 60), e +con label corta nemmeno a width 60 — un test che manca uno dei due fattori +passa contro il bug. + +E deve fermarsi alle larghezze raggiungibili: il minimo post-fix è 48 con label +lunga e 43 con label corta, quindi asserire `≤ 40` fa fallire il test **contro +il codice corretto**. Sweep misurato, larghezza resa dal codice attuale: + +| width | label lunga + cifre larghe | label corta + cifre larghe | +|---|---|---| +| 40 | 48 ✗ | 44 ✗ | +| 44 | 48 ✗ | 47 ✗ | +| 48 | 52 ✗ | 47 ✓ | +| 60 | 64 ✗ | 47 ✓ | +| 80 | 84 ✗ | 47 ✓ | +| 100 | 86 ✗ | 47 ✓ | + +Quindi: per la label lunga usa **60, 80, 100** (oggi 64, 84, 86 — rosso su +tutte; post-fix il minimo 48 sta in tutte). Per la label corta usa **44**, la +larghezza in cui oggi rende 47 e sfora di 3 mentre il minimo post-fix è 43: +è l'unica finestra in cui quella direzione dell'overflow è sia rossa prima sia +verde dopo. + +Effetto sui golden: cambiano solo dove Item è vincolato dal budget — `report` e +`report_narrow`; `report_wide`, multi-valuta e `partial_month` restano intatti. +Da verificare rigenerando. + +### 4.7 Range pinnato e scroll dei Filters (#28) + +**Il pin ha bisogno di un trasporto, non di un ricalcolo.** Ripinnare +richiamando `currentRange()` nell'handler del messaggio ricalcola il range +*dopo* il load: un load alle 23:59:59 su R1 il cui messaggio arriva alle +00:00:01 pinnerebbe R2 ≠ R1, e il pin nascerebbe disallineato dalle entry che +descrive — cioè proprio il drift che esiste per eliminare. Quindi: + +- `entriesMsg` e `entriesReloadedMsg` **portano** `start`/`end`, calcolati dal + cmd che ha fatto il load; +- l'handler pinna **quelli** su `m.loadedStart` / `m.loadedEnd`; +- `m.activeRange()` torna la coppia pinnata se presente, altrimenti ricalcola + con `currentRange()`. Il fallback è raggiungibile davvero (schermata rates + aperta da Home prima di ogni load), non è codice morto. + +**Classificazione dei 10 call site** (sono 10, non 11: l'undicesima riga del +grep è la definizione a `app.go:266`): + +| Sito | Cosa fa | Verdetto | +|---|---|---| +| `home.go:99` `rangeLabel` | descrive il range che il **prossimo** `enter` caricherà | **fresco.** Pinnarlo congela la label: su Home mese, settimana e preset cambiano senza reload (§2.5), quindi la navigazione mese sembrerebbe rotta. | +| `app.go:287` `reloadEntriesCmd` | **è** il load | fresco, e passa la coppia al messaggio | +| `entries.go:564` `reloadForBrowser` | **è** un load | fresco, e passa la coppia a `entriesReloadedMsg`, così un reload del browser aggiorna il pin invece di lasciarlo stantio | +| `app.go:714` handler `entriesMsg` | costruisce il report all'arrivo | usa la coppia **del messaggio** e la pinna | +| `entries.go:436` `updateEntryCmd` | decide se l'entry modificata è uscita dal range **visibile** | **pinnato.** «moved outside the current range» deve riferirsi al range delle entry a schermo, altrimenti a mezzanotte il messaggio mente | +| `report.go:67` `dailySeries` | serie giornaliera sulle entry visibili | pinnato | +| `report.go:81` cambio GroupBy | rebuild sulle entry già caricate | pinnato | +| `report.go:127` schermata budget | rebuild | pinnato | +| `report.go:184` `applyReport` | rebuild | pinnato | +| `rates.go:854` cambio tariffe | rebuild | pinnato | + +**Scroll dei Filters.** `scrollPalette` diventa `scrollWindow(idx, top, rows) +int` condiviso; il commento a `palette.go:139` chiedeva già esattamente questo. + +Il cursore dei Filters è `(sec, row)` su quattro sezioni con header e righe +`(none)` interposte, quindi `idx` non esiste finché non si definisce la +proiezione. **La finestra è sulle righe visuali dell'intera vista** (header +inclusi), non sulle opzioni della sola sezione attiva: l'overflow di cui si +lamenta la #28 è quello della vista intera, e una finestra per-sezione +lascerebbe fuori schermo gli header delle sezioni successive. L'indice del +cursore è la riga visuale su cui sta `(sec, row)`. + +### 4.8 Il resto della #28 + +- **`task_tags`**: `UnmarshalJSON` sull'elemento tag che accetta sia la stringa + nuda sia l'oggetto `{name}`. Motivo: lo schema auto-generato della pagina + range di ClickUp tipizza `task_tags` come `["string"]` mentre l'esempio + concreto dei docs è un array di oggetti `{name, tag_fg, tag_bg, creator}`, e + oggi una stringa nuda fa esplodere l'intera `TimeEntries` (§2.4). Col decoder + tollerante l'assunzione muore invece di essere annotata. La verifica contro un + workspace vero resta alla **#129**, che esiste già per lo stesso tipo di check + sul PUT dei tag di entry. +- **Micro-nit**: `applyReport` fa DRY dei tre blocchi di rebuild; `Empty()` + + `Filter()` non scandiscono `countTrue` due volte. +- **Demo**: casella chiusa come già risolta, con un test che inchioda la densità + per `last_7d` / `this_week` / `last_30d` così non regredisca. + +## 5. Fuori scope (dichiarato) + +- Scroll del **browser entries**: non è in nessuna delle 6 issue. Dopo + l'estrazione di `scrollWindow` costa poco, ma è un'altra issue. +- **#138 punto 2** (`th.Box` col colore bordo): muove i golden di ogni + schermata che usa `th.Box`; va nella tranche dei temi, come dice la issue + stessa. +- **Rotella del mouse** sui Filters: è #74. + +## 6. Test + +- Ogni difetto ha un test **visto fallire** contro il difetto, col transcript + allegato. Non è una raccomandazione: su questo progetto quattro test in una + sola tranche sono stati colti a passare contro il bug che dovevano prendere. +- **Unica eccezione, dichiarata: la #141.** È behaviorally inert — `MaxWidth` e + `ansi.Truncate` tagliano in modo indipendente dal profilo colore, e + `TestMain` fissa `termenv.Ascii`, quindi nessun test comportamentale può + fallire contro quel difetto. La prova è il grep di §2.4 più il test + anti-bleed di §4.3, non un red-green. +- Ogni test di larghezza nasce con una fixture a **rune larghe** o con la + **congiunzione** che il difetto richiede (§4.6). Il test di larghezza della + tabella report è passato contro il bug per una tranche intera perché tutte le + label erano ASCII. +- Golden rigenerati con `go test ./internal/tui -update`, mai a mano. +- Gate pre-commit: `gofmt -l .`, `go vet ./...`, + `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, `go build ./...`, + `go test ./... -race`. + +## 7. Vincoli globali + +- `internal/report` e `internal/duration` restano **puri**: nessun I/O, nessun + import di `config`/`clickup`/`tui`. +- Nessuna funzione di stile in produzione chiama `lipgloss.NewStyle()`: gli + stili vengono dal renderer iniettato del `theme`. +- Mai chiamare l'API ClickUp reale; solo `httptest` con `client.BaseURL`. +- Tutto ciò che vive nel repo è in **inglese** — codice, identificatori, + commenti, stringhe UI, nomi e messaggi dei test, messaggi di commit. + Eccezione: i doc di design in `docs/superpowers/`. +- Conventional Commits. **Mai** `Co-Authored-By`. diff --git a/internal/clickup/timeentries.go b/internal/clickup/timeentries.go index 22c03c1..f8330bc 100644 --- a/internal/clickup/timeentries.go +++ b/internal/clickup/timeentries.go @@ -34,6 +34,31 @@ func (f *flexString) UnmarshalJSON(b []byte) error { return fmt.Errorf("flexString: unhandled value: %s", b) } +// tagName is a tag as ClickUp returns it on a time entry. The documented example +// is an object ({name, tag_fg, tag_bg, creator}), but the API reference's +// generated schema for the date-range endpoint types task_tags as an array of +// strings. A bare string used to fail the WHOLE TimeEntries decode, not just the +// tags, so both shapes are accepted and the assumption is gone rather than +// annotated (#28). A live confirmation against a real workspace is tracked in +// #129. +type tagName struct{ Name string } + +func (t *tagName) UnmarshalJSON(b []byte) error { + var s string + if err := json.Unmarshal(b, &s); err == nil { + t.Name = s + return nil + } + var o struct { + Name string `json:"name"` + } + if err := json.Unmarshal(b, &o); err != nil { + return fmt.Errorf("tagName: unhandled value: %s", b) + } + t.Name = o.Name + return nil +} + // rawEntry mirrors an entry of the "data" array from /team/{id}/time_entries. type rawEntry struct { ID string `json:"id"` @@ -48,16 +73,12 @@ type rawEntry struct { ID int `json:"id"` Username string `json:"username"` } `json:"user"` - Start string `json:"start"` // epoch ms as a string - Duration string `json:"duration"` // ms as a string (negative if a timer is running) - TaskTags []struct { - Name string `json:"name"` - } `json:"task_tags"` - Tags []struct { - Name string `json:"name"` - } `json:"tags"` // the entry's own time-tracking tags - Billable *bool `json:"billable"` // absent (nil) means "bill everything": defaults to true - Description string `json:"description"` + Start string `json:"start"` // epoch ms as a string + Duration string `json:"duration"` // ms as a string (negative if a timer is running) + TaskTags []tagName `json:"task_tags"` + Tags []tagName `json:"tags"` // the entry's own time-tracking tags + Billable *bool `json:"billable"` // absent (nil) means "bill everything": defaults to true + Description string `json:"description"` } // toTimeEntry converts a rawEntry into the domain type. Errors if start/duration diff --git a/internal/clickup/timeentries_test.go b/internal/clickup/timeentries_test.go index 04b4b05..8b4cdca 100644 --- a/internal/clickup/timeentries_test.go +++ b/internal/clickup/timeentries_test.go @@ -3,6 +3,7 @@ package clickup import ( "context" "net/http" + "slices" "testing" "time" ) @@ -83,3 +84,47 @@ func TestTimeEntriesParsesEntryTags(t *testing.T) { t.Errorf("EntryTags = %v, want [focus client-A]", e.EntryTags) } } + +// ClickUp's documented example returns task_tags as objects +// ({name, tag_fg, tag_bg, creator}), but the reference's generated schema for +// the date-range endpoint types task_tags as an array of strings. A bare +// string used to fail the WHOLE decode — "cannot unmarshal string into Go +// struct field rawEntry.data.task_tags" — losing every entry, not just the +// tags (#28). +func TestTimeEntriesAcceptsBothTagShapes(t *testing.T) { + tests := []struct { + name string + tags string + want []string + }{ + {"objects, as documented", `[{"name":"urgent"},{"name":"frontend"}]`, []string{"urgent", "frontend"}}, + {"bare strings, as the generated schema types it", `["urgent","frontend"]`, []string{"urgent", "frontend"}}, + {"mixed", `[{"name":"urgent"},"frontend"]`, []string{"urgent", "frontend"}}, + {"empty", `[]`, nil}, + {"null", `null`, nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body := `{"data":[ + {"id":"e1","task":{"id":"t1","name":"T"},"user":{"id":1,"username":"u"}, + "start":"1700000000000","duration":"3600000","task_tags":` + tc.tags + `} + ]}` + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(body)) + }) + defer srv.Close() + + got, err := c.TimeEntries(context.Background(), "team1", + time.UnixMilli(0), time.UnixMilli(2_000_000_000_000), nil) + if err != nil { + t.Fatalf("TimeEntries error: %v", err) + } + if len(got) != 1 { + t.Fatalf("got %d entries, want 1", len(got)) + } + if !slices.Equal(got[0].Tags, tc.want) { + t.Errorf("Tags = %v, want %v", got[0].Tags, tc.want) + } + }) + } +} diff --git a/internal/report/filter.go b/internal/report/filter.go index ce938e7..bb4616c 100644 --- a/internal/report/filter.go +++ b/internal/report/filter.go @@ -37,10 +37,10 @@ func countTrue(m map[string]bool) int { // at least one of its tags is selected, its status is selected, and (when // Billable is set) its Billable value matches. func Filter(entries []TimeEntry, c FilterCriteria) []TimeEntry { - if c.Empty() { + nL, nT, nS := countTrue(c.Lists), countTrue(c.Tags), countTrue(c.Statuses) + if nL+nT+nS == 0 && c.Billable == nil { return entries } - nL, nT, nS := countTrue(c.Lists), countTrue(c.Tags), countTrue(c.Statuses) out := make([]TimeEntry, 0, len(entries)) for _, e := range entries { if nL > 0 && !c.Lists[e.ListName] { diff --git a/internal/tui/app.go b/internal/tui/app.go index 67ff4b0..bf4418b 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -51,7 +51,10 @@ const ( // Async messages. type ( - entriesMsg struct{ entries []report.TimeEntry } + entriesMsg struct { + entries []report.TimeEntry + start, end time.Time + } teamsMsg struct{ teams []clickup.Team } membersMsg struct{ members []clickup.Member } statusesMsg struct{ byTask map[string]string } @@ -132,6 +135,11 @@ type Model struct { // behavior. Toggled from Home with 'w'. periodMode string + // loadedStart/loadedEnd are the range the currently loaded entries were + // fetched for, pinned from the pair the load itself resolved (#28). Zero + // means nothing has been loaded yet. + loadedStart, loadedEnd time.Time + // injectable clock (default: time.Now) now func() time.Time @@ -278,6 +286,22 @@ func (m Model) currentRange() (start, end time.Time) { return report.RangeForPreset(m.preset, m.year, m.month, m.now(), m.loc) } +// activeRange is the range the LOADED entries actually cover: the pinned pair +// when a load has happened, else what the next load would use. +// +// The split matters because a relative preset moves under you. Every rebuild +// over already-loaded entries goes through here, so it describes its own data. +// Every surface that describes the NEXT load — Home's label, and the loads +// themselves — uses currentRange() instead: Home changes month, week mode and +// preset WITHOUT reloading, so a pinned label would freeze while the user +// navigates. +func (m Model) activeRange() (start, end time.Time) { + if !m.loadedStart.IsZero() { + return m.loadedStart, m.loadedEnd + } + return m.currentRange() +} + // reloadEntriesCmd picks the source for time entries: demo data (no I/O) // in demo mode, otherwise the real API call. origin identifies the screen // that dispatched the load, so a failure can be routed back there (see @@ -387,7 +411,7 @@ func loadEntriesCmd(c *clickup.Client, teamID string, start, end time.Time, scop if err != nil { return retryableErrMsg{origin: origin, err: err} } - return entriesMsg{entries: entries} + return entriesMsg{entries: entries, start: start, end: end} } } @@ -698,6 +722,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case entriesMsg: m.entries = msg.entries + // Pin BEFORE rebuilding: activeRange (and every rebuild reached through + // it, including rebuildReport below) must see the range this load + // actually resolved, not one recomputed after the fact (#28). + m.loadedStart, m.loadedEnd = msg.start, msg.end m.assignStatuses() // re-stamp session-cached statuses onto the freshly loaded entries m.pruneFilters() // drop filter selections whose value no longer occurs in the new entries groupBy := m.report.GroupBy @@ -708,17 +736,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // member grouping is team-only: never let it leak into a "me" report. groupBy = report.GroupByTotal } - if _, ok := m.locOrErr(); !ok { - return m, nil - } - start, end := m.currentRange() - p, ok := m.pricingOrErr() - if !ok { + if !m.rebuildReport(groupBy) { return m, nil } - m.report = report.Build(m.visibleEntries(), groupBy, p, start, end, m.loc) - m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) // Report is reached three ways (Home enter, Report's own reload, and // the logDone reload) and all three arrive here: re-rooting rather // than replacing makes every arrival converge on nav == [Home], which @@ -728,6 +748,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case entriesReloadedMsg: m.entries = msg.entries + // Pin BEFORE rebuilding, same as entriesMsg above: a browser reload is + // a load, so it must refresh the pin rather than leave a stale one (#28). + m.loadedStart, m.loadedEnd = msg.start, msg.end m.assignStatuses() m.pruneFilters() if !m.applyReport() { // rebuilds m.report + m.rep; returns false on loc/pricing error @@ -967,11 +990,11 @@ func (m Model) screenBody() string { case screenRange: return m.rangeScreen.view(m.theme) case screenFilters: - return m.filtersScreen.view(m.theme) + return m.filtersScreen.view(m.theme, filtersRows(m.height)) case screenListBrowser: return m.browserScreen.view(m.theme) case screenBudget: - return m.budgetScreen.view(m.theme) + return m.budgetScreen.view(m.theme, m.width) case screenEntries: return m.entriesView(m.theme) case screenError: diff --git a/internal/tui/app_test.go b/internal/tui/app_test.go index be58827..b965c06 100644 --- a/internal/tui/app_test.go +++ b/internal/tui/app_test.go @@ -431,7 +431,11 @@ func TestEntriesMsgThreadsConfiguredTimezoneThroughRangeAndBuild(t *testing.T) { entries := []report.TimeEntry{ {ID: "1", ListID: "l", ListName: "L", Start: boundary, Duration: time.Hour, Billable: true}, } - updated, _ := m.Update(entriesMsg{entries: entries}) + // start/end mirror what loadEntriesCmd/reloadEntriesCmd resolve in the real + // flow (currentRange() in m.loc, which New() already set from cfg.Timezone + // above) and carry them in the message (#28), same as production. + start, end := m.currentRange() + updated, _ := m.Update(entriesMsg{entries: entries, start: start, end: end}) mm := updated.(Model) if mm.screen != screenReport { diff --git a/internal/tui/budget.go b/internal/tui/budget.go index 89ac3ed..3a36250 100644 --- a/internal/tui/budget.go +++ b/internal/tui/budget.go @@ -2,6 +2,7 @@ package tui import ( "fmt" + "math" "strings" "github.com/charmbracelet/bubbles/key" @@ -61,18 +62,91 @@ func renderBudgetBar(th theme, percentUsed float64) string { filled := int(fillPct / 100 * budgetBarWidth) bar := budgetFillStyle(th, percentUsed).Render(strings.Repeat(string(gaugeFull), filled)) + th.Help.Render(strings.Repeat(string(gaugeEmpty), budgetBarWidth-filled)) - return fmt.Sprintf("%s %.0f%%", bar, percentUsed) + // The fill truncates (int()), so the label FLOORS to match. Rounding the + // label instead is what made 99.5% read "100%" over 19 of 20 blocks; and + // rounding the FILL to match a rounded label would be worse — it would show + // a full bar from 97.5%, and a full bar means the budget is spent. Above 100 + // the label stays unclamped: that asymmetry is the whole point of this + // function (see the doc comment). + return fmt.Sprintf("%s %.0f%%", bar, math.Floor(percentUsed)) } -func (bm budgetModel) view(th theme) string { +const ( + // boxChrome is what th.Box spends on itself: a rounded border plus one + // column of padding on each side. Measured, not assumed: + // th.Box.Render("12345") renders 9 columns. + boxChrome = 4 + // budgetNameWidth is the natural list-name column, used before the first + // WindowSizeMsg — the same "nothing is sized yet" fallback reportItemWidth has. + budgetNameWidth = 24 + // budgetMinNameWidth is where a list name stops being a name, mirroring + // reportMinItemWidth's floor on the report table. + budgetMinNameWidth = 12 +) + +// budgetFigures renders the money part of a row. remaining is the most redundant +// field on the line — it is Budget minus Billed — so it is what gives way on a +// narrow terminal. +func budgetFigures(l report.BudgetLine, withRemaining bool) string { + if withRemaining { + return fmt.Sprintf("%.2f / %.2f %s (remaining %.2f)", l.Billed, l.Budget, l.Currency, l.Remaining) + } + return fmt.Sprintf("%.2f / %.2f %s", l.Billed, l.Budget, l.Currency) +} + +// budgetLayout resolves the list-name column and whether "(remaining …)" fits. +// +// Everything except the name is MEASURED from the real rows, because none of it +// is fixed-width: the figures grow with the amounts and the percentage grows past +// 100. Reserving a constant for them is exactly how this screen came to render 94 +// columns into an 80-column terminal. +// +// One pass, in this order — the circularity (figures depend on the drop, the drop +// depends on the name, the name depends on the figures) is resolved here and not +// left to the caller: +// +// 1. compute with remaining; if the name column clears the floor, done; +// 2. otherwise recompute without remaining, and never put it back; +// 3. if the name is still under the floor, the floor wins and the row +// overflows. That happens below boxChrome + 1 + budgetBarWidth + 1 + pctW + +// 2 + figures + budgetMinNameWidth — around 63 to 65 columns with realistic +// amounts. Below it no split saves the row, and pretending otherwise would +// mean a name column too narrow to read. Accepted and explicit, the same +// trade-off the report table makes below its own floor. +func budgetLayout(lines []report.BudgetLine, width int) (nameW int, showRemaining bool) { + if width <= 0 { // before the first WindowSizeMsg nothing is sized against it + return budgetNameWidth, true + } + pctW := 0 + for _, l := range lines { + pctW = max(pctW, lipgloss.Width(fmt.Sprintf("%.0f%%", math.Floor(l.PercentUsed)))) + } + widestFigures := func(withRemaining bool) int { + w := 0 + for _, l := range lines { + w = max(w, lipgloss.Width(budgetFigures(l, withRemaining))) + } + return w + } + fixed := 1 + budgetBarWidth + 1 + pctW + 2 + usable := width - boxChrome + + if nameW = usable - fixed - widestFigures(true); nameW >= budgetMinNameWidth { + return nameW, true + } + return max(usable-fixed-widestFigures(false), budgetMinNameWidth), false +} + +func (bm budgetModel) view(th theme, width int) string { title := th.Title.Render("Budget burn-down") if len(bm.lines) == 0 { return title + "\n\n" + th.Box.Render("No budgets configured.") } + nameW, showRemaining := budgetLayout(bm.lines, width) var rows strings.Builder for _, l := range bm.lines { - rows.WriteString(fmt.Sprintf("%-24s %s %.2f / %.2f %s (remaining %.2f)\n", - truncate(l.ListName, 24), renderBudgetBar(th, l.PercentUsed), l.Billed, l.Budget, l.Currency, l.Remaining)) + rows.WriteString(fmt.Sprintf("%s %s %s\n", + cell(l.ListName, nameW), renderBudgetBar(th, l.PercentUsed), budgetFigures(l, showRemaining))) } body := th.Box.Render(strings.TrimRight(rows.String(), "\n")) return title + "\n\n" + body diff --git a/internal/tui/budget_bar_test.go b/internal/tui/budget_bar_test.go new file mode 100644 index 0000000..53962c2 --- /dev/null +++ b/internal/tui/budget_bar_test.go @@ -0,0 +1,69 @@ +package tui + +import ( + "fmt" + "math" + "strings" + "testing" +) + +// budgetBarFixtures pairs a percentage with its expected full-block count, +// HAND-MEASURED against the current (correct) renderBudgetBar and written down +// as literals — not recomputed here from budgetBarWidth/100 arithmetic. A +// wantFull expression built from the same formula as the implementation would +// share any bug in that formula and could never catch one: it would drift in +// lockstep with budget.go and still pass. Measured with: +// +// go test ./internal/tui -run TestZZMeasureBarFixtures -v +// +// (a throwaway test that logged renderBudgetBar's output and its full-block +// count for each percentage below; the numbers here are copied from that log, +// not derived). +// +// Covers: zero (0 blocks), the two tick points either side of the first block +// (4.9/5.0) and of the twentieth (94.9/95.0), the near-100 cluster that used +// to disagree with its label (97.6/99.4/99.5/99.6/99.9, all 19 of 20 — the +// bug this task fixes), exact 100 (20, full), and a value past 100 (104.7, +// still 20 full — the fill clamps but the label must not). +var budgetBarFixtures = []struct { + pct float64 + wantFull int +}{ + {0, 0}, + {4.9, 0}, + {5.0, 1}, + {9.9, 1}, + {10.0, 2}, + {50, 10}, + {94.9, 18}, + {95.0, 19}, + {97.6, 19}, + {99.4, 19}, + {99.5, 19}, + {99.6, 19}, + {99.9, 19}, + {100, 20}, + {104.7, 20}, +} + +// The label rounded while the fill truncated, so from 99.5% up the label read +// "100%" over 19 of 20 blocks. Flooring the LABEL is the fix, not rounding the +// fill: a rounded fill would show a full bar from 97.5%, and a full bar means +// the budget is spent. The label stays unclamped above 100 — that is why this +// screen does not use bubbles/progress. +func TestBudgetBarLabelAgreesWithTheFill(t *testing.T) { + th := testTheme(true) + for _, tc := range budgetBarFixtures { + bar := renderBudgetBar(th, tc.pct) + full := strings.Count(bar, string(gaugeFull)) + if full != tc.wantFull { + t.Errorf("pct=%.1f: %d full blocks, want %d (%q)", tc.pct, full, tc.wantFull, bar) + } + // The label must state the floored percentage, so it can never claim a + // milestone the bar has not reached. + wantLabel := fmt.Sprintf("%.0f%%", math.Floor(tc.pct)) + if !strings.HasSuffix(bar, " "+wantLabel) { + t.Errorf("pct=%.1f: %q does not end in %q", tc.pct, bar, wantLabel) + } + } +} diff --git a/internal/tui/budget_test.go b/internal/tui/budget_test.go index 22db2f4..c3c5fcb 100644 --- a/internal/tui/budget_test.go +++ b/internal/tui/budget_test.go @@ -6,6 +6,7 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/marcoarnulfo/clickup-cli/internal/report" ) @@ -15,7 +16,7 @@ func TestBudgetViewRendersProgressBar(t *testing.T) { {ListID: "list-1", ListName: "Website", Currency: "EUR", Budget: 1000, Billed: 600, Remaining: 400, PercentUsed: 60}, } - out := newBudget(lines).view(testTheme(true)) + out := newBudget(lines).view(testTheme(true), 80) for _, want := range []string{"Website", "60%", "600.00", "1000.00", "EUR"} { if !strings.Contains(out, want) { t.Errorf("budget view missing %q; got:\n%s", want, out) @@ -27,7 +28,7 @@ func TestBudgetViewRendersProgressBar(t *testing.T) { } func TestBudgetViewEmptyShowsMessage(t *testing.T) { - out := newBudget(nil).view(testTheme(true)) + out := newBudget(nil).view(testTheme(true), 80) if !strings.Contains(out, "No budgets") { t.Errorf("empty budget view should say no budgets are configured; got:\n%s", out) } @@ -94,3 +95,90 @@ func TestBudgetBReturnsReport(t *testing.T) { t.Errorf("b from budget -> %v, want screenReport", got) } } + +// The budget line is unbounded: it grows with the figures. Measured before the +// fix, testdata/budget.golden is 94 columns (90 of content + 4 of th.Box), and +// 80 columns is a bare terminal and a split tmux pane (#136). +// +// The widths are the ones this fixture can actually fit: the row needs +// box(4) + 1 + gauge(20) + 1 + pct + 2 + figures + name-floor(12), which is 65 +// for these two lines. Asserting at 60 would demand the impossible — below the +// threshold the row overflows by design, and the spec says so. +func TestBudgetViewNeverExceedsWidth(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + {ListName: "Mobile app", Billed: 1040, Budget: 1000, Currency: "EUR", Remaining: -40, PercentUsed: 104}, + }) + for _, width := range []int{65, 80, 100, 120} { + for _, line := range strings.Split(bm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line rendered %d columns: %q", width, w, line) + } + } + } +} + +// Small figures let the row fit a narrow terminal, which is what proves the +// layout is measured rather than reserved: same code, more room, wider name. +func TestBudgetViewFitsNarrowTerminalsWithSmallFigures(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: "Website", Billed: 12.5, Budget: 20, Currency: "EUR", Remaining: 7.5, PercentUsed: 62.5}, + }) + for _, width := range []int{60, 80} { + for _, line := range strings.Split(bm.view(th, width), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line rendered %d columns: %q", width, w, line) + } + } + } +} + +// A wide-rune list name must not push the row past its column: the name column +// is the one that absorbs the slack, so it is also the one that used to +// misalign. An ASCII fixture passes against the bug. +func TestBudgetViewHoldsItsWidthWithWideRunes(t *testing.T) { + th := testTheme(true) + bm := newBudget([]report.BudgetLine{ + {ListName: strings.Repeat("🚀", 24), Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + }) + for _, line := range strings.Split(bm.view(th, 80), "\n") { + if w := lipgloss.Width(line); w > 80 { + t.Errorf("line rendered %d columns: %q", w, line) + } + } +} + +// "(remaining X)" is the most redundant field on the row (it is budget minus +// billed), so it is what gives way when the name column would fall below the +// floor. One pass: compute with it, and if the name is too narrow, recompute +// without it and never put it back. Measured for this fixture at width 80: with +// remaining the name would get 10 columns, under the floor of 12, so remaining +// goes and the name gets 29. +func TestBudgetLayoutDropsRemainingBeforeStarvingTheName(t *testing.T) { + lines := []report.BudgetLine{ + {ListName: "Website", Billed: 625, Budget: 1000, Currency: "EUR", Remaining: 375, PercentUsed: 62.5}, + } + nameW, showRemaining := budgetLayout(lines, 80) + if showRemaining { + t.Errorf("at width 80 remaining should have been dropped; name column is %d", nameW) + } + if nameW != 29 { + t.Errorf("nameW = %d, want 29", nameW) + } + // A roomy terminal keeps remaining AND a wider name. + wideName, wideRem := budgetLayout(lines, 100) + if !wideRem { + t.Errorf("at width 100 remaining should fit; name column is %d", wideName) + } +} + +// Before the first WindowSizeMsg the width is 0 and nothing is sized against it: +// the screen keeps its natural layout, the same fallback reportItemWidth has. +func TestBudgetLayoutFallsBackBeforeTheFirstWindowSize(t *testing.T) { + nameW, showRemaining := budgetLayout([]report.BudgetLine{{ListName: "Website"}}, 0) + if nameW != budgetNameWidth || !showRemaining { + t.Errorf("budgetLayout(width=0) = (%d, %v), want (%d, true)", nameW, showRemaining, budgetNameWidth) + } +} diff --git a/internal/tui/demo.go b/internal/tui/demo.go index 0311447..d379d36 100644 --- a/internal/tui/demo.go +++ b/internal/tui/demo.go @@ -177,7 +177,7 @@ func (m Model) demoEntriesSnapshot(start, end time.Time, assignees []int) []repo // clipped to [start, end). func (m Model) demoEntriesCmd(start, end time.Time, assignees []int) tea.Cmd { snap := m.demoEntriesSnapshot(start, end, assignees) - return func() tea.Msg { return entriesMsg{entries: snap} } + return func() tea.Msg { return entriesMsg{entries: snap, start: start, end: end} } } // demoHistoryChanges is a small fixed change history for demo mode (Task 8), diff --git a/internal/tui/demo_test.go b/internal/tui/demo_test.go index 486b623..fd29a0d 100644 --- a/internal/tui/demo_test.go +++ b/internal/tui/demo_test.go @@ -305,3 +305,26 @@ func demoJuly2026() (time.Time, time.Time) { start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) return start, start.AddDate(0, 1, 0) } + +// Demo mode used to be near-empty for the non-month presets because the +// fixtures lived on fixed month days; #4 made them day offsets wrapped modulo +// the range span. This pins that, so the presets stay demoable (#28). It is a +// regression-guard on a box that is ALREADY closed: it must pass on the first +// run, and a failure means #4's wrapping regressed. +func TestDemoEntriesPopulateEveryPreset(t *testing.T) { + now := time.Date(2026, time.July, 24, 12, 0, 0, 0, time.UTC) + for _, preset := range []string{report.PresetThisMonth, report.PresetLast7d, report.PresetLast30d, report.PresetThisWeek} { + t.Run(preset, func(t *testing.T) { + start, end := report.RangeForPreset(preset, 2026, time.July, now, time.UTC) + entries := demoEntries(start, end) + if len(entries) != 8 { + t.Errorf("got %d entries, want all 8 fixtures inside the range", len(entries)) + } + for _, e := range entries { + if e.Start.Before(start) || !e.Start.Before(end) { + t.Errorf("entry %s at %v falls outside [%v, %v)", e.ID, e.Start, start, end) + } + } + }) + } +} diff --git a/internal/tui/entries.go b/internal/tui/entries.go index 352cabe..207c0f1 100644 --- a/internal/tui/entries.go +++ b/internal/tui/entries.go @@ -21,8 +21,9 @@ import ( // the fresh entries plus a status line to show inline, without leaving // screenEntries. type entriesReloadedMsg struct { - entries []report.TimeEntry - status string + entries []report.TimeEntry + status string + start, end time.Time } // entriesErrMsg is a browser-action error that keeps the user in the entries @@ -427,13 +428,14 @@ func (m Model) deleteEntryCmd(id string) tea.Cmd { // updateEntryCmd updates entry id (real API) or relies on the demo override // already recorded on the Model (see submitEntriesEdit, which sets // m.demoOverrides before this cmd is built) and reloads the browser either -// way. moved/status are computed against the CURRENT range (m.currentRange, +// way. moved/status are computed against the LOADED range (m.activeRange, // [start,end) in m.loc) before the goroutine, since the closure captures mm -// by value: an edit that moves the entry outside the visible range still -// succeeds, just with a status line saying so instead of silently vanishing. +// by value: an edit that moves the entry outside the range the entries ON +// SCREEN were loaded for still succeeds, just with a status line saying so +// instead of silently vanishing. func (m Model) updateEntryCmd(id string, start time.Time, dur time.Duration, note string, billable bool) tea.Cmd { mm := m - s, e := m.currentRange() + s, e := m.activeRange() moved := start.Before(s) || !start.Before(e) status := "Entry saved." if moved { @@ -564,7 +566,7 @@ func reloadForBrowser(m Model, status string) tea.Msg { start, end := m.currentRange() assignees := m.reloadAssignees() if m.demo { - return entriesReloadedMsg{entries: m.demoEntriesSnapshot(start, end, assignees), status: status} + return entriesReloadedMsg{entries: m.demoEntriesSnapshot(start, end, assignees), status: status, start: start, end: end} } ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) defer cancel() @@ -572,7 +574,7 @@ func reloadForBrowser(m Model, status string) tea.Msg { if err != nil { return entriesErr(err) } - return entriesReloadedMsg{entries: entries, status: status} + return entriesReloadedMsg{entries: entries, status: status, start: start, end: end} } func (m Model) entriesView(th theme) string { @@ -580,7 +582,7 @@ func (m Model) entriesView(th theme) string { if es.mode == entriesConfirmDelete && len(es.entries) > 0 { e := es.entries[es.idx] return th.Title.Render("Delete entry") + "\n\n" + - "Delete " + th.Accent.Render(truncate(e.TaskName, 40)) + " (" + + "Delete " + th.Accent.Render(truncateWidth(e.TaskName, 40)) + " (" + duration.FormatHours(e.Duration) + ")?" } if es.mode == entriesEdit { @@ -590,7 +592,7 @@ func (m Model) entriesView(th theme) string { return entriesHistoryView(th, es, m.loc) } if es.mode == entriesTags { - b := th.Title.Render("Tags") + " " + th.Accent.Render(truncate(tagPickerTaskName(es), 40)) + "\n\n" + b := th.Title.Render("Tags") + " " + th.Accent.Render(truncateWidth(tagPickerTaskName(es), 40)) + "\n\n" if es.tagLoading { b += th.Help.Render("Loading tags…") + "\n" return b @@ -639,13 +641,13 @@ func (m Model) entriesView(th theme) string { bill = "$" } owner := "(" + ownerLabel(e, m.userID) + ")" - line := fmt.Sprintf("%s %-24s %6s %s %s", - when, truncate(e.TaskName, 24), duration.FormatHours(e.Duration), bill, owner) + line := fmt.Sprintf("%s %s %6s %s %s", + when, cell(e.TaskName, 24), duration.FormatHours(e.Duration), bill, owner) if !canEdit(e, m.userID) { line += " — read-only" } if len(e.EntryTags) > 0 { - line += " " + truncate(tagBadges(e.EntryTags), 20) + line += " " + truncateWidth(tagBadges(e.EntryTags), 20) } if i == es.idx { line = th.Accent.Render(line) diff --git a/internal/tui/filters.go b/internal/tui/filters.go index 7d83b67..6558fe8 100644 --- a/internal/tui/filters.go +++ b/internal/tui/filters.go @@ -61,6 +61,7 @@ type filtersModel struct { sections []filterSection // [Lists, Tags, Statuses, Billable] sec int // active section index row int // active row within the section + top int // first visible visual row (#28) loadingStatuses bool } @@ -167,6 +168,7 @@ func (m Model) updateFilters(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m = m.pop() return m, nil } + fs.top = scrollWindow(filtersCursorRow(fs), fs.top, filtersContentRows(filtersRows(m.height))) m.filtersScreen = fs return m, nil } @@ -184,11 +186,50 @@ func allChosen(s filterSection) bool { return true } -func (fs filtersModel) view(th theme) string { - if fs.loadingStatuses { - return th.Title.Render("Loading statuses…") +// filtersRows is how many lines view() may emit in total, title included. The +// subtraction accounts for the blank line and the footer that View always +// appends; the floor says the screen shrinks but never vanishes. +func filtersRows(height int) int { + return max(6, height-2) +} + +// filtersTitleRows is the title block's physical line cost, measured (not +// assumed): th.Title.Render("Filters") itself renders as TWO lines, not one — +// its MarginBottom(1) pads out a second, space-filled line below "Filters" — +// and view() adds one further blank line after that. Three, not two. +const filtersTitleRows = 3 + +// filtersContentRows is how many of filtersVisualRows' rows view(th, rows) can +// actually show out of a total budget of rows: rows, minus the title block +// (filtersTitleRows), minus one more for the line-counting convention +// strings.Count(s, "\n")+1 uses (the tests measure rows this way, and it counts +// the string's own trailing newline as one further line). updateFilters must +// scroll to the same window this slices, or the cursor tracks a window wider +// than what actually renders. +func filtersContentRows(rows int) int { + return max(rows-filtersTitleRows-1, 0) +} + +// filtersCursorRow is the visual row (sec, row) sits on — the index +// filtersVisualRows puts it at. Headers and the "(none)" placeholder count. +func filtersCursorRow(fs filtersModel) int { + n := 0 + for si, sec := range fs.sections { + if si == fs.sec { + return n + 1 + fs.row // this section's header, then row + } + n += 1 + max(1, len(sec.options)) // header + options, or the "(none)" row } - b := th.Title.Render("Filters") + "\n\n" + return n +} + +// filtersVisualRows renders the screen's CONTENT as one flat slice of visual +// rows: a section header, then its options (or a single "(none)"), for each +// section. The title is not part of the slice. The window in view() is taken +// over THIS slice, so the cursor's index and the rendering can never disagree +// about what row something is on. +func filtersVisualRows(fs filtersModel, th theme) []string { + rows := make([]string, 0, len(fs.sections)*2) for si, sec := range fs.sections { head := sec.title if si == fs.sec { @@ -196,9 +237,9 @@ func (fs filtersModel) view(th theme) string { } else { head = " " + head } - b += head + "\n" + rows = append(rows, head) if len(sec.options) == 0 { - b += " " + th.Help.Render("(none)") + "\n" + rows = append(rows, " "+th.Help.Render("(none)")) } for ri, o := range sec.options { box := "[ ]" @@ -209,8 +250,24 @@ func (fs filtersModel) view(th theme) string { if si == fs.sec && ri == fs.row { line = " " + box + " " + th.Accent.Render(o) } - b += line + "\n" + rows = append(rows, line) } } + return rows +} + +func (fs filtersModel) view(th theme, rows int) string { + if fs.loadingStatuses { + return th.Title.Render("Loading statuses…") + } + visual := filtersVisualRows(fs, th) + window := filtersContentRows(rows) + top := min(fs.top, max(0, len(visual)-window)) + end := min(len(visual), top+window) + + b := th.Title.Render("Filters") + "\n\n" + for _, line := range visual[top:end] { + b += line + "\n" + } return b } diff --git a/internal/tui/filters_test.go b/internal/tui/filters_test.go index 395d3a3..dbf3242 100644 --- a/internal/tui/filters_test.go +++ b/internal/tui/filters_test.go @@ -1,6 +1,7 @@ package tui import ( + "fmt" "slices" "strings" "testing" @@ -11,7 +12,10 @@ import ( "github.com/marcoarnulfo/clickup-cli/internal/report" ) -func filtersFixture() Model { +// filtersScreenFixture is a Model parked on screenFilters with two entries +// (one billable, one not) so the screen's toggle/apply/discard behavior has +// something concrete to act on. +func filtersScreenFixture() Model { entries := []report.TimeEntry{ {ListName: "Website", Tags: []string{"frontend"}, Status: "in progress", Billable: true}, {ListName: "Mobile", Tags: []string{"backend"}, Status: "done", Billable: false}, @@ -21,8 +25,22 @@ func filtersFixture() Model { return m } +// filtersFixture builds entries whose list names produce n Lists options, plus +// one tag and one status, so the Filters screen has something to scroll. +func filtersFixture(n int) []report.TimeEntry { + entries := make([]report.TimeEntry, 0, n) + for i := range n { + entries = append(entries, report.TimeEntry{ + ListName: fmt.Sprintf("List %02d", i), + Tags: []string{"backend"}, + Status: "done", + }) + } + return entries +} + func TestFiltersToggleAndApply(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() // section 0 = Lists; toggle first option (row 0) u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{' '}}) m = u.(Model) @@ -40,7 +58,7 @@ func TestFiltersToggleAndApply(t *testing.T) { // route to screenError instead of switching to screenReport with a stale // (unfiltered) report — applyReport's false return must not be papered over. func TestFiltersApplyWithBadRoundingRoutesToErrorScreen(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() m.cfg = config.Config{} m.cfg.Billing.Rounding.Increment = "not-a-duration" u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{' '}}) @@ -56,7 +74,7 @@ func TestFiltersApplyWithBadRoundingRoutesToErrorScreen(t *testing.T) { } func TestFiltersTabChangesSection(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyTab}) m = u.(Model) if m.filtersScreen.sec != 1 { @@ -65,7 +83,7 @@ func TestFiltersTabChangesSection(t *testing.T) { } func TestFiltersEscDiscards(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{' '}}) m = u.(Model) u, _ = m.updateFilters(tea.KeyMsg{Type: tea.KeyEsc}) @@ -82,7 +100,7 @@ func TestFiltersEscDiscards(t *testing.T) { // Non-billable only) built on top of the existing report.FilterCriteria.Billable // field — not a private pre-filter (A3, binding). func TestFiltersBillableDefaultsToAll(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() sec := m.filtersScreen.sections[3] if sec.title != "Billable" { t.Fatalf("section 3 = %q, want Billable", sec.title) @@ -93,7 +111,7 @@ func TestFiltersBillableDefaultsToAll(t *testing.T) { } func TestFiltersBillableRestrictsVisibleEntries(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() // Tab to the Billable section (index 3). for i := 0; i < 3; i++ { u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyTab}) @@ -116,7 +134,7 @@ func TestFiltersBillableRestrictsVisibleEntries(t *testing.T) { } func TestFiltersBillableRadioIsExclusive(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() for i := 0; i < 3; i++ { u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyTab}) m = u.(Model) @@ -148,7 +166,7 @@ func TestFiltersBillableRadioIsExclusive(t *testing.T) { // Billable radio section — applying it would select all three mutually // exclusive options at once, breaking the exactly-one-selected invariant. func TestFiltersBillableANoOp(t *testing.T) { - m := filtersFixture() + m := filtersScreenFixture() for i := 0; i < 3; i++ { u, _ := m.updateFilters(tea.KeyMsg{Type: tea.KeyTab}) m = u.(Model) @@ -175,7 +193,7 @@ func TestFiltersBillableANoOp(t *testing.T) { // arm never fired — dropped here deliberately, not a regression (#59 Task 3). func TestFiltersKeyLabels(t *testing.T) { t.Parallel() - m := filtersFixture() + m := filtersScreenFixture() want := []string{" ", "?", "a", "ctrl+c", "ctrl+p", "down", "enter", "esc", "j", "k", "q", "shift+tab", "tab", "up"} if got := enabledLabels(keysFor(m)); !slices.Equal(got, want) { t.Errorf("filters labels = %v, want %v", got, want) @@ -190,3 +208,79 @@ func TestReportFOpensFilters(t *testing.T) { t.Errorf("f should open filters, got %v", m.screen) } } + +// The Filters screen had no window at all: many lists or tags simply ran off the +// bottom of a short terminal, with no way to reach them (#28). +func TestFiltersViewNeverExceedsItsRowBudget(t *testing.T) { + th := testTheme(true) + fs := newFilters(filtersFixture(40), nil, nil, nil, nil) + for _, rows := range []int{6, 10, 20} { + if got := strings.Count(fs.view(th, rows), "\n") + 1; got > rows { + t.Errorf("row budget %d: view rendered %d lines", rows, got) + } + } +} + +// The window must follow the cursor. Asserting on the rendered cursor is not an +// option: under termenv.Ascii the active row is byte-identical to the others +// (th.Accent renders nothing), and "▸" marks the section header, which scrolls +// out. So assert on the window indices instead — but with the SAME expression +// production uses (filtersContentRows(filtersRows(m.height)), not a literal +// -2), or this test would happily pass a cursor that updateFilters actually +// scrolled off the rendered screen. +func TestFiltersWindowFollowsTheCursor(t *testing.T) { + m := newTestModel() + m.filtersScreen = newFilters(filtersFixture(40), nil, nil, nil, nil) + m.screen = screenFilters + m.height = 12 + for range 30 { // walk down well past the first window + mm, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m = mm.(Model) + } + fs := m.filtersScreen + if fs.row == 0 && fs.sec == 0 { + t.Fatal("the cursor never moved: the fixture has no options to walk") + } + row, rows := filtersCursorRow(fs), filtersContentRows(filtersRows(m.height)) + if row < fs.top || row >= fs.top+rows { + t.Errorf("cursor at visual row %d is outside the rendered window [%d, %d)", row, fs.top, fs.top+rows) + } +} + +// scrollWindow is the palette's own scrolling, extracted so both screens share +// one idiom. Same behavior, now with a name. +func TestScrollWindow(t *testing.T) { + tests := []struct { + name string + idx, top, rows, want int + }{ + {"inside stays", 3, 2, 5, 2}, + {"above pulls up", 1, 4, 5, 1}, + {"below pushes down", 9, 2, 5, 5}, + {"first row", 0, 0, 5, 0}, + {"zero rows", 7, 3, 0, 0}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := scrollWindow(tc.idx, tc.top, tc.rows); got != tc.want { + t.Errorf("scrollWindow(%d, %d, %d) = %d, want %d", tc.idx, tc.top, tc.rows, got, tc.want) + } + }) + } +} + +// The cursor's visual row must account for section headers and the "(none)" +// placeholder an empty section still renders, or the window scrolls to the wrong +// place. With two Lists options, Lists row 1 is visual row 2 (header + first +// option); the Tags section that follows starts after header + 2 options. +func TestFiltersCursorRowCountsHeadersAndOptions(t *testing.T) { + fs := newFilters(filtersFixture(2), nil, nil, nil, nil) + fs.sec, fs.row = 0, 1 + if got := filtersCursorRow(fs); got != 2 { + t.Errorf("filtersCursorRow at Lists row 1 = %d, want 2", got) + } + fs.sec, fs.row = 1, 0 + if got := filtersCursorRow(fs); got != 4 { + t.Errorf("filtersCursorRow at Tags row 0 = %d, want 4", got) + } +} diff --git a/internal/tui/footer.go b/internal/tui/footer.go index 093e8e4..4d85fd8 100644 --- a/internal/tui/footer.go +++ b/internal/tui/footer.go @@ -4,6 +4,7 @@ import ( "github.com/charmbracelet/bubbles/help" "github.com/charmbracelet/bubbles/key" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" ) // footerView renders one screen's advertised bindings as the bottom help line, @@ -57,9 +58,16 @@ func clampWidth(th theme, s string, width int) string { if width <= 1 || lipgloss.Width(s) <= width { return s } - // MaxWidth truncates ANSI-aware; the ellipsis is added separately so the - // cut is visible rather than looking like a footer that simply ends. - return lipgloss.NewStyle().MaxWidth(width-1).Render(s) + th.Help.Render("…") + // ansi.Truncate cuts ANSI-aware without a renderer. MaxWidth produces the + // same truncation point for single-line input without tabs — which is all + // this function receives, since the full help is deliberately unbounded and + // returns before clamping. On tab-containing input the two diverge: Render + // expands tabs to four spaces before truncating, while ansi.Truncate does + // not. The style carries no color, so the output is identical regardless of + // color profile. This is why the change is invisible to tests — the guard is + // the grep that this style is never built, not the suite. The ellipsis is + // added separately, styled from the theme, so the cut is visible. + return ansi.Truncate(s, width-1, "") + th.Help.Render("…") } // pairHelp returns a display-only binding that advertises two related bindings diff --git a/internal/tui/footer_golden_test.go b/internal/tui/footer_golden_test.go index 5539c21..53a866e 100644 --- a/internal/tui/footer_golden_test.go +++ b/internal/tui/footer_golden_test.go @@ -73,8 +73,8 @@ func TestGoldenFooters(t *testing.T) { // task's own template — produces the identical keyMap). {"report", newTestModelOnReport}, - // screenFilters (filters_test.go:176 TestFiltersKeyLabels). - {"filters", filtersFixture}, + // screenFilters (filters_test.go:194 TestFiltersKeyLabels). + {"filters", filtersScreenFixture}, // screenMembers (members_test.go:89 TestMembersKeyLabels). {"members", membersFixture}, diff --git a/internal/tui/footer_test.go b/internal/tui/footer_test.go index e286742..7bd528e 100644 --- a/internal/tui/footer_test.go +++ b/internal/tui/footer_test.go @@ -140,3 +140,71 @@ func TestFooterNeverExceedsWidth(t *testing.T) { }) } } + +// clampWidth must not build a style on lipgloss's default renderer: that is the +// one thing the injected-renderer discipline exists to prevent, and this file's +// own opening comment says so about help.New(). +// +// This is invisible to the suite by construction, but not because TestMain +// pins termenv.Ascii: clampWidth's own doc comment has the measured reason — +// the style it renders through carries no color, so the output is identical +// on any color profile, the injected renderer's included. Do not add a test +// that claims otherwise — pin the truncation instead, and keep the grep in +// the checklist. +func TestClampWidthNeverExceedsTheTerminal(t *testing.T) { + t.Parallel() + th := testTheme(true) + long := "↑/↓ move · enter run · esc close · ctrl+c force quit · ? help · g group · e export" + for _, width := range []int{20, 40, 66, 80} { + got := clampWidth(th, long, width) + if w := lipgloss.Width(got); w > width { + t.Errorf("clampWidth(width=%d) rendered %d columns", width, w) + } + if !strings.HasSuffix(got, "…") { + t.Errorf("clampWidth(width=%d) = %q, want a visible ellipsis", width, got) + } + } +} + +// The footer's input comes from help.View, which closes its own styles. If a +// caller ever passes an unterminated one, ansi.Truncate will NOT close it and +// the style bleeds past the cut — this pins the boundary of what clampWidth +// promises, so the discovery happens here and not on a user's terminal. +func TestClampWidthDoesNotCloseAnOpenStyle(t *testing.T) { + t.Parallel() + th := testTheme(true) + got := clampWidth(th, "\x1b[31m"+strings.Repeat("x", 40), 10) + // The red escape must survive the truncation. + if !strings.Contains(got, "\x1b[31m") { + t.Fatalf("expected the caller's escape to survive, got %q", got) + } + // ansi.Truncate does not emit a reset, so the truncated part alone should + // have no resets. The ellipsis rendering is separate — any resets in the + // full output come from th.Help.Render, not from clampWidth itself. + ellipsis := th.Help.Render("…") + truncated := strings.TrimSuffix(got, ellipsis) + if strings.Count(truncated, "\x1b[0m") != 0 { + t.Errorf("clampWidth closed a style it did not open: %q", truncated) + } +} + +// Verify that TestClampWidthDoesNotCloseAnOpenStyle works with both Ascii and +// TrueColor profiles. This guards against the anti-bleed check being fragile to +// the theme's color profile. +func TestClampWidthDoesNotCloseAnOpenStyleWithPaletteTheme(t *testing.T) { + t.Parallel() + th := paletteTheme(true) + got := clampWidth(th, "\x1b[31m"+strings.Repeat("x", 40), 10) + // The red escape must survive the truncation. + if !strings.Contains(got, "\x1b[31m") { + t.Fatalf("expected the caller's escape to survive, got %q", got) + } + // ansi.Truncate does not emit a reset, so the truncated part alone should + // have no resets. The ellipsis rendering is separate — any resets in the + // full output come from th.Help.Render, not from clampWidth itself. + ellipsis := th.Help.Render("…") + truncated := strings.TrimSuffix(got, ellipsis) + if strings.Count(truncated, "\x1b[0m") != 0 { + t.Errorf("clampWidth closed a style it did not open: %q", truncated) + } +} diff --git a/internal/tui/golden_test.go b/internal/tui/golden_test.go index c9c5b66..3e5c2a9 100644 --- a/internal/tui/golden_test.go +++ b/internal/tui/golden_test.go @@ -184,6 +184,39 @@ func TestGoldenReportPartialMonth(t *testing.T) { golden(t, "report_partial_month", newReport(goldenReport(), "", goldenDailyPartialMonth()).view(testTheme(true), 80)) } +// The Total grouping is what the report loads with, so it is the first table a +// user (and the README GIF) ever sees. Two goldens: one currency, and the +// multi-currency shape where TOTAL's Amount is empty and the subtotals carry +// the figures (#137). +func TestGoldenReportTotalGrouping(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 15.5, BilledHours: 12.5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 15.5, BilledHours: 12.5, Amount: 625}}, + TotalHours: 15.5, BilledHours: 12.5, TotalAmount: 625, + } + golden(t, "report_total", reportTable(testTheme(true), r, 80)) +} + +func TestGoldenReportTotalGroupingMultiCurrency(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 18, BilledHours: 17, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 337.5}, {Currency: "USD", Amount: 512.5}}}}, + CurrencySubtotals: []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 7.75, BilledHours: 6.75, Amount: 337.5}, + {Currency: "USD", Hours: 10.25, BilledHours: 10.25, Amount: 512.5}, + }, + TotalHours: 18, BilledHours: 17, + } + golden(t, "report_total_multicurrency", reportTable(testTheme(true), r, 80)) +} + func TestGoldenExport(t *testing.T) { t.Parallel() golden(t, "export", newExport(goldenReport()).view(testTheme(true))) @@ -195,12 +228,12 @@ func TestGoldenBudget(t *testing.T) { {ListID: "l1", ListName: "Website", Currency: "EUR", Budget: 1000, Billed: 625, Remaining: 375, PercentUsed: 62.5}, } - golden(t, "budget", newBudget(lines).view(testTheme(true))) + golden(t, "budget", newBudget(lines).view(testTheme(true), 80)) } func TestGoldenBudgetEmpty(t *testing.T) { t.Parallel() - golden(t, "budget_empty", newBudget(nil).view(testTheme(true))) + golden(t, "budget_empty", newBudget(nil).view(testTheme(true), 80)) } func TestGoldenMembers(t *testing.T) { @@ -216,7 +249,15 @@ func TestGoldenRange(t *testing.T) { func TestGoldenFilters(t *testing.T) { t.Parallel() - golden(t, "filters", newFilters(goldenEntries(), nil, nil, nil, nil).view(testTheme(true))) + golden(t, "filters", newFilters(goldenEntries(), nil, nil, nil, nil).view(testTheme(true), 24)) +} + +// TestGoldenFiltersScrolled pins the windowed view a short terminal actually +// renders: 40 lists is far more than fits in 12 rows, so the golden should cut +// off partway through the Lists section with the cursor still inside it (#28). +func TestGoldenFiltersScrolled(t *testing.T) { + t.Parallel() + golden(t, "filters_scrolled", newFilters(filtersFixture(40), nil, nil, nil, nil).view(testTheme(true), 12)) } func TestGoldenSetup(t *testing.T) { diff --git a/internal/tui/home.go b/internal/tui/home.go index defc589..7484b3c 100644 --- a/internal/tui/home.go +++ b/internal/tui/home.go @@ -95,6 +95,9 @@ func (m Model) homeMembersNote() string { } // rangeLabel returns a short label for the active range shown on Home. +// +// Uses currentRange(), not activeRange(): Home changes month, week mode and +// preset without reloading, so a pinned label would freeze mid-navigation (#28). func (m Model) rangeLabel() string { start, end := m.currentRange() return report.PeriodLabel(start, end) diff --git a/internal/tui/log.go b/internal/tui/log.go index 30e9a9d..693f24c 100644 --- a/internal/tui/log.go +++ b/internal/tui/log.go @@ -537,7 +537,7 @@ func (lg logModel) view(th theme) string { b += "Choose the task:\n\n" for i, tk := range lg.tasks { cursor := " " - line := truncate(tk.Name, 40) + line := truncateWidth(tk.Name, 40) if i == lg.taskIdx { cursor = "▸ " line = th.Accent.Render(line) diff --git a/internal/tui/palette.go b/internal/tui/palette.go index 6146d54..27d7a69 100644 --- a/internal/tui/palette.go +++ b/internal/tui/palette.go @@ -136,17 +136,28 @@ func (m Model) updateOverlay(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m, nil } -// scrollPalette moves the visible window so idx is always inside it. This is the -// scrolling the Filters screen still lacks (#28); the palette gets it right from -// the start. +// scrollPalette moves the visible window so idx is always inside it, on the +// shared idiom in scrollWindow. func scrollPalette(p paletteModel, rows int) paletteModel { - if p.idx < p.top { - p.top = p.idx + p.top = scrollWindow(p.idx, p.top, rows) + return p +} + +// scrollWindow moves a visible window of `rows` rows so idx stays inside it, and +// returns the new top. Shared by the palette and the Filters screen (#28): the +// palette had this from the start, and Filters now uses the same idiom instead +// of a second one. +func scrollWindow(idx, top, rows int) int { + if rows <= 0 { + return 0 } - if p.idx >= p.top+rows { - p.top = p.idx - rows + 1 + if idx < top { + return idx } - return p + if idx >= top+rows { + return idx - rows + 1 + } + return top } // paletteRows is how many action rows fit. The subtraction accounts for the box @@ -211,12 +222,12 @@ func (p paletteModel) box(th theme, boxW, rows int) string { b.WriteString(th.Border.Render("╭─ ") + th.Accent.Render(paletteTitle) + th.Border.Render(" "+strings.Repeat("─", dashes)+"╮") + "\n") - q := shaveToWidth("> "+p.query, innerW) + q := truncateWidth("> "+p.query, innerW) b.WriteString(paletteLine(th, th.Cell.Render(q), innerW-lipgloss.Width(q))) b.WriteString(th.Border.Render("├"+strings.Repeat("─", boxW-2)+"┤") + "\n") if len(p.items) == 0 { - msg := shaveToWidth("no matching action", innerW) + msg := truncateWidth("no matching action", innerW) b.WriteString(paletteLine(th, th.Help.Render(msg), innerW-lipgloss.Width(msg))) } for i := p.top; i < len(p.items) && i < p.top+rows; i++ { @@ -250,7 +261,7 @@ func paletteRow(th theme, it paletteItem, selected bool, innerW int) string { if hintW > 0 { labelW -= 2 // two spaces between the label and the key } - label := shaveToWidth(it.a.label, max(labelW, 1)) + label := truncateWidth(it.a.label, max(labelW, 1)) gap := innerW - 2 - lipgloss.Width(label) - hintW out := style.Render(cursor) + highlight(th, label, it.idx) diff --git a/internal/tui/palette_test.go b/internal/tui/palette_test.go index 4f0d89a..7cc2d50 100644 --- a/internal/tui/palette_test.go +++ b/internal/tui/palette_test.go @@ -422,7 +422,7 @@ func TestPaletteHighlightUsesTheAccentStyle(t *testing.T) { // // The query is chosen so its match lands EXACTLY on the ellipsis column, not // merely "somewhere past the cut": "t" is label's 20th rune (index 19), and -// shaveToWidth(label, 20) replaces that very column with "…". That is the only +// truncateWidth(label, 20) replaces that very column with "…". That is the only // boundary this test can actually observe. An index further out (the original // version of this test used "z9", matching at columns 25 and 35) is never even // read: the render loop below only walks the truncated string's own 20 runes, @@ -438,10 +438,10 @@ func TestPaletteHighlightSurvivesTruncation(t *testing.T) { t.Fatal(`Match("t", label) did not match`) } if want := []int{19}; !slices.Equal(idx, want) { - t.Fatalf("idx = %v, want %v — this test needs the match to land exactly on the ellipsis column shaveToWidth(label, 20) produces", idx, want) + t.Fatalf("idx = %v, want %v — this test needs the match to land exactly on the ellipsis column truncateWidth(label, 20) produces", idx, want) } - short := shaveToWidth(label, 20) + short := truncateWidth(label, 20) got := highlight(th, short, idx) if w := lipgloss.Width(got); w != 20 { t.Errorf("highlighted label is %d cells, want 20: %q", w, got) diff --git a/internal/tui/range_pin_test.go b/internal/tui/range_pin_test.go new file mode 100644 index 0000000..0fd3129 --- /dev/null +++ b/internal/tui/range_pin_test.go @@ -0,0 +1,133 @@ +package tui + +import ( + "testing" + "time" + + "github.com/marcoarnulfo/clickup-cli/internal/report" +) + +// A relative preset re-resolved time.Now() on every rebuild, so regrouping after +// midnight relabeled the report with a range the loaded entries never covered. +// The range is pinned by the load and carried in the message, so a rebuild +// cannot drift away from its own data (#28). +// +// m.loc is pinned to UTC on purpose: with time.Local the two instants below fall +// on the same local day east of Greenwich, the fixture stops exercising the +// drift, and the test would pass in CI while failing on a developer's machine. +// cfg.Timezone is pinned alongside it: entriesMsg's handler calls locOrErr +// before every rebuild (#83), which re-resolves m.loc from cfg.Timezone +// (falling back to time.Local, not to whatever m.loc already holds) — without +// this, m.loc set above would be clobbered back to the host's zone on the +// very first Update, one assertion before the drift this test is about (found +// by running this test on a UTC+2 machine, where it failed with CEST offsets +// instead of the intended one-day drift). +func TestRebuildKeepsTheRangeTheEntriesWereLoadedFor(t *testing.T) { + before := time.Date(2026, time.July, 24, 23, 59, 59, 0, time.UTC) + after := time.Date(2026, time.July, 25, 0, 0, 1, 0, time.UTC) + now := before + m := newTestModel() + m.cfg.Timezone = "UTC" + m.loc = time.UTC + m.preset = report.PresetLast7d + m.now = func() time.Time { return now } + + // The load resolves its range while "now" is still `before` — this is + // what loadEntriesCmd/demoEntriesCmd do, synchronously, right before + // dispatching the fetch. + start, end := m.currentRange() + + // "now" advances HERE, between the load resolving its range and the + // entriesMsg carrying it actually being processed by Update — the + // latency the whole task exists to cover (a load at 23:59:59 whose + // result lands at 00:00:01). If the handler re-derived the range from + // m.currentRange() instead of using the message's own start/end, it + // would resolve it against "now" as it stands at THIS call, i.e. after + // midnight — this is the only window where re-deriving and carrying the + // range actually diverge; testing before this point cannot tell them apart. + now = after + if newStart, _ := m.currentRange(); newStart.Equal(start) { + t.Fatal("the fixture does not exercise the drift: currentRange did not move across midnight") + } + + mm, _ := m.Update(entriesMsg{entries: goldenEntries(), start: start, end: end}) + m = mm.(Model) + if !m.report.Start.Equal(start) { + t.Fatalf("report.Start = %v, want the loaded range start %v (the handler must use the message's own range, not re-derive it)", m.report.Start, start) + } + + // A later rebuild over the same loaded entries (e.g. cycling groupBy) + // must still read the pinned range, not currentRange() at whatever "now" + // happens to be by then. + if !m.applyReport() { + t.Fatal("applyReport returned false") + } + if !m.report.Start.Equal(start) { + t.Errorf("after a later rebuild report.Start = %v, want the pinned %v", m.report.Start, start) + } +} + +// Home's label describes the range the NEXT load will fetch, so it must stay +// fresh: month, week mode and preset all change on Home WITHOUT a reload, and a +// pinned label would freeze while the user navigates. +func TestHomeLabelFollowsTheSelectionNotThePin(t *testing.T) { + m := newTestModel() + m.cfg.Timezone = "UTC" // see TestRebuildKeepsTheRangeTheEntriesWereLoadedFor: locOrErr re-resolves m.loc from this on every Update + m.loc = time.UTC + m.now = func() time.Time { return time.Date(2026, time.July, 24, 12, 0, 0, 0, time.UTC) } + start, end := m.currentRange() + mm, _ := m.Update(entriesMsg{entries: goldenEntries(), start: start, end: end}) + m = mm.(Model) + pinned := m.rangeLabel() + + m.month-- // Home's PrevMonth, which triggers no reload + if got := m.rangeLabel(); got == pinned { + t.Errorf("rangeLabel stayed %q after changing month: the label must describe the next load", got) + } +} + +// Nothing is pinned before the first load, and the fallback is reachable in the +// live flow: the rates screen opens from Home and rebuilds the report there. +func TestActiveRangeFallsBackBeforeAnyLoad(t *testing.T) { + m := newTestModel() + m.loc = time.UTC + wantStart, wantEnd := m.currentRange() + gotStart, gotEnd := m.activeRange() + if !gotStart.Equal(wantStart) || !gotEnd.Equal(wantEnd) { + t.Errorf("activeRange() = (%v, %v), want currentRange() (%v, %v)", gotStart, gotEnd, wantStart, wantEnd) + } +} + +// A browser reload is a load: it must refresh the pin, not leave a stale one. +func TestBrowserReloadRepinsTheRange(t *testing.T) { + m := newTestModel() + m.cfg.Timezone = "UTC" // see TestRebuildKeepsTheRangeTheEntriesWereLoadedFor: locOrErr re-resolves m.loc from this on every Update + m.loc = time.UTC + // m.year/m.month are pinned away from July on purpose: the default + // preset is this_month, and currentRange() reads them directly (not + // m.now()). If the handler mis-recomputed the range instead of using the + // message's own start/end, m.currentRange() would resolve against + // WHATEVER real month the suite happens to run in; leaving them at + // newTestModel()'s construction-time default (today's real month) would + // make that mutation invisible whenever the suite is run in July, and + // this repo runs it plenty. Pinning them to January makes the verdict + // independent of the calendar. + m.year, m.month = 2026, time.January + old := time.Date(2026, time.June, 1, 0, 0, 0, 0, time.UTC) + m.loadedStart, m.loadedEnd = old, old.AddDate(0, 1, 0) + start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + end := start.AddDate(0, 1, 0) + mm, _ := m.Update(entriesReloadedMsg{entries: goldenEntries(), status: "ok", start: start, end: end}) + m = mm.(Model) + if got, _ := m.activeRange(); !got.Equal(start) { + t.Errorf("activeRange() = %v after a browser reload, want the reloaded %v", got, start) + } + // Also assert on the rebuilt report, not just activeRange(): the handler + // must pin loadedStart/loadedEnd BEFORE calling applyReport(), and this is + // the only assertion that would catch that ordering being reversed — + // activeRange() alone reads m.loadedStart/loadedEnd directly and would + // still report the fresh pin even if the rebuild ran first on the stale one. + if !m.report.Start.Equal(start) { + t.Errorf("report.Start = %v after a browser reload, want the reloaded %v (the pin must be set BEFORE the rebuild)", m.report.Start, start) + } +} diff --git a/internal/tui/rates.go b/internal/tui/rates.go index 265f6c6..d4de7b4 100644 --- a/internal/tui/rates.go +++ b/internal/tui/rates.go @@ -844,17 +844,9 @@ func (m Model) saveRates(rt ratesModel) (tea.Model, tea.Cmd) { if g == "" { g = report.GroupByTotal } - if _, ok := m.locOrErr(); !ok { + if !m.rebuildReport(g) { return m, nil } - p, ok := m.pricingOrErr() - if !ok { - return m, nil - } - start, end := m.currentRange() - m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) - m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) m = m.pop() return m, nil } diff --git a/internal/tui/rates_test.go b/internal/tui/rates_test.go index ce498ec..1eabeb8 100644 --- a/internal/tui/rates_test.go +++ b/internal/tui/rates_test.go @@ -8,6 +8,7 @@ import ( "time" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/marcoarnulfo/clickup-cli/internal/config" "github.com/marcoarnulfo/clickup-cli/internal/report" ) @@ -529,6 +530,39 @@ func TestRatesRejectsInvalidCurrencyInline(t *testing.T) { } } +// validCurrency gates the in-app editor (TestRatesRejectsInvalidCurrencyInline +// above), but a config file loaded from disk never runs through it: nothing +// in config.Load or config.Valid checks Billing.Currencies/DefaultCurrency/ +// Currency against ISO 4217. A hand-edited or externally-synced config.yml +// can carry any string, and effectiveCurrency renders it as-is. +// +// listsView's Cur column used to pad that string with fmt's "%-5s", which +// counts runes, not display columns. A currency whose only rune is wide (one +// rune, two display columns, as with several CJK currency symbols) then +// consumed 6 display columns instead of 5, shifting Budget and Source one +// column to the right on that row alone. cell(cur, 5) pads by display +// columns, so the row keeps the same width as any other list's row +// regardless of what the currency string is made of. +func TestRatesListsViewKeepsRowWidthWithWideRuneCurrency(t *testing.T) { + t.Parallel() + cfg := config.Config{Rate: 30, Currency: "EUR"} + rt := newRates(billingEntries(), cfg) + // One rune, two display columns — reachable only via a config file, never + // via the in-app editor, which validCurrency would reject. + rt.currencies["1"] = "円" + got := rt.listsView(testTheme(true)) + lines := strings.Split(got, "\n") + + // lines[0] is the header; lines[1] is "Website" (listID "1", the + // wide-rune currency); lines[2] is "Internal" (listID "2", falling back + // to the ordinary ASCII "EUR"). + wideRow, asciiRow := lines[1], lines[2] + if w, want := lipgloss.Width(wideRow), lipgloss.Width(asciiRow); w != want { + t.Errorf("row with a wide-rune currency is %d columns wide, want %d (same as the ASCII-currency row):\n%q\n%q", + w, want, wideRow, asciiRow) + } +} + // 'd' in the Lists section clears the rate only. The currency and the budget // are cleared by reopening their own field and submitting an empty value — // which is what their help text and error messages now say. diff --git a/internal/tui/rates_view.go b/internal/tui/rates_view.go index 839d4f7..31035c2 100644 --- a/internal/tui/rates_view.go +++ b/internal/tui/rates_view.go @@ -39,6 +39,17 @@ func billingRow(th theme, sel bool, line string) string { } func (rt ratesModel) listsView(th theme) string { + // "List", "Rate", "Cur", "Budget" and "Source" are ASCII literals, where + // runes and columns coincide by construction, so the %-Ns verbs in the + // header stay aligned with the data rows' cell(...,N) columns below. + // + // The Cur column is the one data column here that is NOT a %-Ns verb: the + // currency comes from effectiveCurrency, which traces back to the user's + // own config (currencies/default_currency/currency in config.go) with no + // ISO check enforced — Valid() never looks at it. A wide-rune currency + // (e.g. "円") would misalign every column after it under %-5s, which pads + // by counting runes; cell(cur, 5) pads by display columns instead, the + // same fix this whole file's non-ASCII columns already use. b := th.Help.Render(fmt.Sprintf(" %-24s %10s %-5s %10s %s", "List", "Rate", "Cur", "Budget", "Source")) + "\n" if len(rt.rows) == 0 { return b + th.Help.Render(" No lists in the current report — press 'b' to browse the workspace.") + "\n" @@ -49,12 +60,12 @@ func (rt ratesModel) listsView(th theme) string { rate, tag = v, "list rate" } bud, hasBud := rt.budgets[r.listID] - line := fmt.Sprintf("%-24s %10.2f %-5s %10s %s", - truncate(r.name, 24), rate, rt.effectiveCurrency(r.listID), moneyOrDash(bud, hasBud), tag) + line := fmt.Sprintf("%s %10.2f %s %10s %s", + cell(r.name, 24), rate, cell(rt.effectiveCurrency(r.listID), 5), moneyOrDash(bud, hasBud), tag) b += billingRow(th, i == rt.sel[secLists], line) } sel := rt.rows[rt.sel[secLists]] - note := fmt.Sprintf("Effective for %s: %.2f %s", truncate(sel.name, 24), rt.rateFor(sel.listID), rt.effectiveCurrency(sel.listID)) + note := fmt.Sprintf("Effective for %s: %.2f %s", truncateWidth(sel.name, 24), rt.rateFor(sel.listID), rt.effectiveCurrency(sel.listID)) if n := rt.pairsForList(sel.listID); n > 0 { note += fmt.Sprintf(" · %d (list,member) override(s) take precedence here", n) } else if len(rt.memberRates) > 0 { @@ -64,6 +75,8 @@ func (rt ratesModel) listsView(th theme) string { } func (rt ratesModel) membersView(th theme) string { + // "Member" is an ASCII literal, where runes and columns coincide by + // construction, so %-30s stays aligned with cell(...,30) in the data rows. b := th.Help.Render(fmt.Sprintf(" %-30s %10s %s", "Member", "Rate", "Source")) + "\n" if len(rt.members) == 0 { return b + th.Help.Render(" No members in the current report — run a team-scope report first.") + "\n" @@ -73,24 +86,27 @@ func (rt ratesModel) membersView(th theme) string { if v, ok := rt.memberRates[mr.id]; ok { rate, tag = v, "member rate" } - line := fmt.Sprintf("%-30s %10.2f %s", truncate(fmt.Sprintf("%s (%d)", mr.name, mr.id), 30), rate, tag) + line := fmt.Sprintf("%s %10.2f %s", cell(fmt.Sprintf("%s (%d)", mr.name, mr.id), 30), rate, tag) b += billingRow(th, i == rt.sel[secMembers], line) } sel := rt.members[rt.sel[secMembers]] note := "A member rate wins over any per-list rate, on every list." if n := rt.listsForMember(sel.id); n > 0 { - note = fmt.Sprintf("%s is overridden on %d list(s) by a (list,member) rate.", truncate(sel.name, 24), n) + note = fmt.Sprintf("%s is overridden on %d list(s) by a (list,member) rate.", truncateWidth(sel.name, 24), n) } return b + "\n" + th.Help.Render(note) + "\n" } func (rt ratesModel) overridesView(th theme) string { + // "List" and "Member" are ASCII literals, where runes and columns coincide + // by construction, so %-20s and %-22s stay aligned with the cell(...,20) + // and cell(...,22) data rows below. b := th.Help.Render(fmt.Sprintf(" %-20s %-22s %10s %s", "List", "Member", "Rate", "Instead of")) + "\n" for i, o := range rt.overrides { below, src := rt.rateBelowPair(o.listID, o.member) - line := fmt.Sprintf("%-20s %-22s %10.2f %.2f (%s)", - truncate(rt.listName(o.listID), 20), - truncate(fmt.Sprintf("%s (%d)", rt.memberName(o.member), o.member), 22), + line := fmt.Sprintf("%s %s %10.2f %.2f (%s)", + cell(rt.listName(o.listID), 20), + cell(fmt.Sprintf("%s (%d)", rt.memberName(o.member), o.member), 22), o.rate, below, src) b += billingRow(th, i == rt.sel[secOverrides], line) } @@ -140,13 +156,13 @@ func (rt ratesModel) draftView(th theme) string { if rt.draft.step == draftPickList { b := th.Help.Render("New override — choose the list:") + "\n" for i, r := range rt.rows { - b += billingRow(th, i == rt.draft.idx, truncate(r.name, 40)) + b += billingRow(th, i == rt.draft.idx, truncateWidth(r.name, 40)) } return b } - b := th.Help.Render(fmt.Sprintf("New override on %s — choose the member:", truncate(rt.listName(rt.draft.listID), 24))) + "\n" + b := th.Help.Render(fmt.Sprintf("New override on %s — choose the member:", truncateWidth(rt.listName(rt.draft.listID), 24))) + "\n" for i, mr := range rt.members { - b += billingRow(th, i == rt.draft.idx, truncate(fmt.Sprintf("%s (%d)", mr.name, mr.id), 40)) + b += billingRow(th, i == rt.draft.idx, truncateWidth(fmt.Sprintf("%s (%d)", mr.name, mr.id), 40)) } return b } diff --git a/internal/tui/report.go b/internal/tui/report.go index 2ee84a2..2a60d8e 100644 --- a/internal/tui/report.go +++ b/internal/tui/report.go @@ -60,11 +60,11 @@ func (m Model) memberFilterNote() string { return fmt.Sprintf(" (%d/%d members)", k, n) } -// dailySeries is the per-day hours of the visible entries over the current +// dailySeries is the per-day hours of the visible entries over the loaded // range. It lives on the Model because reportModel has no entries — the report // is a rendering of an already-aggregated value. func (m Model) dailySeries() []float64 { - start, end := m.currentRange() + start, end := m.activeRange() return report.DailyHours(m.visibleEntries(), start, end, m.loc) } @@ -75,15 +75,7 @@ func (m Model) updateReport(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m = m.pop() case key.Matches(msg, k.GroupBy): g := nextGroupBy(m.report.GroupBy, m.scope) - if _, ok := m.locOrErr(); !ok { - return m, nil - } - start, end := m.currentRange() - if p, ok := m.pricingOrErr(); ok { - m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) - m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) - } + m.rebuildReport(g) case key.Matches(msg, k.ChangeRange): m = m.pop() case key.Matches(msg, k.Reload): @@ -124,7 +116,7 @@ func (m *Model) openBudgetView() bool { if !ok { return false } - start, end := m.currentRange() + start, end := m.activeRange() perList := report.Build(m.visibleEntries(), report.GroupByList, p, start, end, m.loc) billed := billedByListFromBuckets(perList.Buckets, p) budgets := service.BudgetsFromConfig(m.cfg) @@ -165,15 +157,13 @@ func listNamesFromBuckets(buckets []report.Bucket) map[string]string { return out } -// applyReport rebuilds m.report from the visible entries over the current -// range, keeping the active grouping. It returns false when the config's -// pricing rules failed to parse, in which case pricingOrErr has already -// routed the model to screenError and the caller must not overwrite that. -func (m *Model) applyReport() bool { - g := m.report.GroupBy - if g == "" { - g = report.GroupByTotal - } +// rebuildReport rebuilds m.report and m.rep for the given grouping, over the +// visible entries and the pinned range. It returns false when the timezone or +// the pricing rules failed to parse, in which case the caller must not overwrite +// the error screen those helpers already routed to. It is the single rebuild +// used by applyReport, by the entries handler and by the grouping and rates +// changes, which each carried their own copy. +func (m *Model) rebuildReport(groupBy string) bool { if _, ok := m.locOrErr(); !ok { return false } @@ -181,13 +171,24 @@ func (m *Model) applyReport() bool { if !ok { return false } - start, end := m.currentRange() - m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) + start, end := m.activeRange() + m.report = report.Build(m.visibleEntries(), groupBy, p, start, end, m.loc) m.report.Scope = m.scope m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) return true } +// applyReport calls rebuildReport with the active grouping (m.report.GroupBy, +// defaulting to GroupByTotal when unset) — see rebuildReport's doc comment +// for the false-return cases. +func (m *Model) applyReport() bool { + g := m.report.GroupBy + if g == "" { + g = report.GroupByTotal + } + return m.rebuildReport(g) +} + // hoursOf renders hours the same way export.SummaryLine does, via // duration.FormatHours — one hours formatter shared across TUI and export. func hoursOf(h float64) string { @@ -259,24 +260,3 @@ func (rm reportModel) sparkView(th theme, width int) string { label, chart := raw[:cut], raw[cut:] return th.Help.Render(label) + th.Accent.Render(chart) } - -// truncate shortens to n runes (not bytes), to avoid breaking UTF-8 characters -// in task names with accents or emoji. An empty s always yields "", whatever -// n is: there is nothing to truncate, so there is nothing to mark as cut. -// Otherwise n <= 1 always yields "…": r[:n-1] would otherwise index -// negatively (a panic) for n <= 0, and the whole package now relies on that -// guard rather than on each call site keeping n >= some floor itself (see -// report_table.go's shaveToWidth for why that matters). -func truncate(s string, n int) string { - if s == "" { - return "" - } - if n <= 1 { - return "…" - } - r := []rune(s) - if len(r) <= n { - return s - } - return string(r[:n-1]) + "…" -} diff --git a/internal/tui/report_table.go b/internal/tui/report_table.go index 0803747..33d3c6b 100644 --- a/internal/tui/report_table.go +++ b/internal/tui/report_table.go @@ -13,8 +13,11 @@ import ( // Item column first: the numeric columns are the reason to have a grid, so // Item gives up its space before they do. Amount is the one exception, and // only as a last resort — see reportAmountWidth for why that is safe. +// +// Hours and Billed are not in this block: reportNumWidths measures them from +// the rows instead of reserving a flat width, so a wider value can never push +// the table past the terminal edge the way a fixed reservation did (#138). const ( - reportNumWidth = 8 // reserved for each of Hours and Billed reportMinItemWidth = 12 // below this the labels stop being labels reportNatItemWidth = 32 // the fixed width the report had before #66 // reportTableChrome is the border and padding the table spends on itself: @@ -28,20 +31,29 @@ var reportHeaders = []string{"Item", "Hours", "Billed", "Amount"} // reportRows renders the report as table cells and reports the index at which // the total rows begin. Bucket rows come first, then either a single TOTAL row // (one currency) or a TOTAL row with an empty Amount followed by one subtotal -// row per currency. +// row per currency. The exception is GroupByTotal with a single bucket: that +// bucket row is suppressed as a duplicate of TOTAL (#137), so TOTAL is row 0 +// and there are no bucket rows at all. // // Per-bucket Amounts are indicative: PerDay rounding can drift a few cents // from the subtotals at a fine grouping. CurrencySubtotals is authoritative // and is never re-derived from the bucket rows. func reportRows(r report.Report) ([][]string, int) { rows := make([][]string, 0, len(r.Buckets)+len(r.CurrencySubtotals)+1) - for _, b := range r.Buckets { - rows = append(rows, []string{ - b.Label, - fmt.Sprintf("%.2f", b.Hours), - fmt.Sprintf("%.2f", b.BilledHours), - formatAmounts(b.Amounts, r.DefaultCurrency), - }) + // Under GroupByTotal the single bucket IS the totals row: it collects every + // entry, so its hours, billed hours and Amounts equal the totals and the + // currency subtotals exactly — not approximately, so there is not even the + // PerDay rounding caveat that applies at finer groupings. Emitting both put + // "Total" directly above "TOTAL", differing only in case and color (#137). + if !(r.GroupBy == report.GroupByTotal && len(r.Buckets) == 1) { + for _, b := range r.Buckets { + rows = append(rows, []string{ + b.Label, + fmt.Sprintf("%.2f", b.Hours), + fmt.Sprintf("%.2f", b.BilledHours), + formatAmounts(b.Amounts, r.DefaultCurrency), + }) + } } firstTotal := len(rows) @@ -85,19 +97,35 @@ func formatAmounts(amounts []report.CurrencyAmount, fallback string) string { return strings.Join(parts, " + ") } -// reportItemWidth splits width between the fixed numeric columns and the Item +// reportNumWidths measures the Hours and Billed columns from the rows, HEADERS +// INCLUDED. The headers are not decoration in this arithmetic: lipgloss/table +// sizes a column from every row it is given, the header row among them, so a +// "Billed" header holds 6 columns even when every value is 5. reportNumWidth +// used to reserve a flat 8 for each and nothing enforced it — a wider value +// simply pushed the table past the terminal edge by 2 x (width - 8) (#138). +func reportNumWidths(rows [][]string) (hours, billed int) { + hours = lipgloss.Width(reportHeaders[1]) + billed = lipgloss.Width(reportHeaders[2]) + for _, row := range rows { + hours = max(hours, lipgloss.Width(row[1])) + billed = max(billed, lipgloss.Width(row[2])) + } + return hours, billed +} + +// reportItemWidth splits width between the numeric columns and the Item // column. It never stretches Item past the longest label (empty space is not a // feature) and never shrinks it below reportMinItemWidth — unless the labels // themselves are shorter than that floor. // -// Reserving reportNumWidth for Hours and Billed is a worst case: real values -// are shorter, so the table often renders narrower than the terminal. Narrower -// is fine; wider is the bug this arithmetic exists to prevent — but this -// function alone cannot promise it: it assumes Amount keeps its natural, -// untruncated width, and a multi-currency bucket's Amount can blow well past -// whatever is left once Item is already at its floor. reportAmountWidth is the -// other half of that promise: it is what actually claws space back from -// Amount, as a last resort, once Item has none left to give. +// Hours and Billed are measured by reportNumWidths, not reserved at a worst +// case, so the table renders exactly as wide as its content needs. This +// function alone cannot promise the whole table fits, though: it assumes +// Amount keeps its natural, untruncated width, and a multi-currency bucket's +// Amount can blow well past whatever is left once Item is already at its +// floor. reportAmountWidth is the other half of that promise: it is what +// actually claws space back from Amount, as a last resort, once Item has none +// left to give. func reportItemWidth(rows [][]string, width int) int { maxLabel := lipgloss.Width(reportHeaders[0]) amount := lipgloss.Width(reportHeaders[3]) @@ -108,8 +136,9 @@ func reportItemWidth(rows [][]string, width int) int { if width <= 0 { return min(maxLabel, reportNatItemWidth) } + hours, billed := reportNumWidths(rows) floor := min(reportMinItemWidth, maxLabel) - item := width - reportTableChrome - 2*reportNumWidth - amount + item := width - reportTableChrome - hours - billed - amount return max(floor, min(maxLabel, item)) } @@ -126,20 +155,37 @@ func reportItemWidth(rows [][]string, width int) int { // own rows at the bottom of the same table, at full precision — but amountW // is one column width shared by every row, subtotals included, so a subtotal // cell truncates exactly like a bucket cell once its natural width no longer -// fits. That only happens below about 52 columns (a 3-currency subtotal like -// "1234567.00 EUR" needs 14 of its own); at 60 columns and above — the width -// this file's own tests treat as the table's design range -// (TestReportTableNeverExceedsWidth starts there) — every subtotal fits -// whole. Below roughly 48 columns the Item column has already hit its -// reportMinItemWidth floor and the whole table is in the degraded regime that -// floor exists to accept; a truncated subtotal in that band is one more -// symptom of the same accepted trade-off, not a new one. +// fits. +// +// Where that line falls is not a fixed column count. Rendering whole needs +// budget — the terminal's width minus chrome, Item and the COMBINED width of +// Hours and Billed (reportNumWidths) — to reach at least the subtotal cell's +// own natural width (computed below), and none of those three quantities is +// reserved at a flat size: all are measured from the rows. Widen any of +// them by a column — more digits in Hours or Billed, or a longer currency +// code in the subtotal itself — and the terminal width the subtotal needs to +// render whole grows by exactly that much; narrow them and it shrinks by the +// same amount. There is no single number to name here that would stay true +// as those widths change. TestReportTableNeverExceedsWidthWithLongLabelAndWideNumbers +// pins one concrete case, with the numbers it needs measured and asserted in +// the test itself. Once Item has hit its reportMinItemWidth floor the whole +// table is already in the degraded regime that floor exists to accept; a +// truncated subtotal in that regime is one more symptom of the same accepted +// trade-off, not a new one. // // width <= 0 is the first render, before the terminal has sent its // WindowSizeMsg: nothing is sized against it yet, so Amount stays at its // natural width, same as Item. Otherwise this never stretches Amount past -// what the content needs, only ever shrinks it — down to 1, never lower: -// truncate's own guard (n <= 1) is what makes that safe, not a floor here. +// what the content needs, only ever shrinks it — down to 1, never lower. The +// floor is not about keeping the COLUMN alive: Headers(reportHeaders...) +// passes "Amount" whole, and lipgloss/table sizes a column from every cell it +// is given, the header included, so even at floor 0 the column still renders +// at the header's own width (verified: floor 1 renders that same width, never +// 1 column). What floor 0 actually does is worse: truncateWidth(s, 0) +// collapses every DATA cell to "", so the row shows nothing where a value +// belongs — not even the "…" that would signal a cut. Floor 1 keeps that +// signal, since truncateWidth(s, 1) still renders a one-column ellipsis, +// which is why max(1, ...) below is the floor and not max(0, ...). func reportAmountWidth(rows [][]string, width, itemW int) int { natural := lipgloss.Width(reportHeaders[3]) for _, row := range rows { @@ -148,7 +194,8 @@ func reportAmountWidth(rows [][]string, width, itemW int) int { if width <= 0 { return natural } - budget := width - reportTableChrome - 2*reportNumWidth - itemW + hours, billed := reportNumWidths(rows) + budget := width - reportTableChrome - hours - billed - itemW return max(1, min(natural, budget)) } @@ -191,7 +238,7 @@ func reportStyleFunc(th theme, firstTotal int) table.StyleFunc { // // Three lipgloss/table defaults are wrong for this grid and are all turned // off: BorderColumn draws separators between columns, Wrap sends a long label -// onto a second line instead of letting truncate cut it, and the zero +// onto a second line instead of letting truncateWidth cut it, and the zero // BorderStyle renders the frame through the default renderer. // // Sizing is implicit and Table.Width is deliberately NOT called. With @@ -206,8 +253,8 @@ func reportTable(th theme, r report.Report, width int) string { itemW := reportItemWidth(rows, width) amountW := reportAmountWidth(rows, width, itemW) for i := range rows { - rows[i][0] = shaveToWidth(rows[i][0], itemW) - rows[i][3] = shaveToWidth(rows[i][3], amountW) + rows[i][0] = truncateWidth(rows[i][0], itemW) + rows[i][3] = truncateWidth(rows[i][3], amountW) } return table.New(). Border(lipgloss.RoundedBorder()). @@ -220,40 +267,3 @@ func reportTable(th theme, r report.Report, width int) string { StyleFunc(reportStyleFunc(th, firstTotal)). String() } - -// shaveToWidth truncates s until it renders at no more than w display -// columns. truncate cuts by RUNE count (it never breaks a multi-byte UTF-8 -// character), but the widths this file computes (reportItemWidth, -// reportAmountWidth) and lipgloss/table's own resizer all measure in DISPLAY -// columns (lipgloss.Width). The two agree for ASCII but not for double-width -// runes (CJK, emoji) or a multi-currency Amount like "12345.00 EUR + 6789.00 -// USD + 4321.00 GBP": a value truncated to w runes can still render at up to -// 2w columns, which is exactly the "table wider than the terminal" bug this -// whole file exists to prevent. Bucket labels are ClickUp list/task names and -// a wide workspace can carry any number of currencies, so both are reachable, -// not theoretical. Shave one more rune at a time until the rendered width -// actually fits. -// -// Safe for w >= 1, without the caller keeping its own floor: truncate itself -// guards n <= 1. reportItemWidth's own floor (reportMinItemWidth, itself -// bounded below by "Item"'s own width) keeps its w at 4 or above, so the loop -// never gets near that guard for the Item column; reportAmountWidth has no -// equivalent floor and can legitimately hand back 1 — exactly the case the -// guard exists for. -// -// w <= 0 is NOT safe and the caller must not pass it: truncate(s, 0) (like -// truncate(s, 1)) returns "…", which is 1 column wide, so "> w" stays true -// forever and the loop never returns. Both of today's callers keep w at 1 or -// above (see above), so this is unreached, not merely untested — the guard -// below exists so a future caller that breaks that invariant gets an empty -// string back instead of a hang with no diagnosis. -func shaveToWidth(s string, w int) string { - if w <= 0 { - return "" - } - s = truncate(s, w) - for lipgloss.Width(s) > w { - s = truncate(s, len([]rune(s))-1) - } - return s -} diff --git a/internal/tui/report_table_test.go b/internal/tui/report_table_test.go index 7056b7f..01501a6 100644 --- a/internal/tui/report_table_test.go +++ b/internal/tui/report_table_test.go @@ -180,6 +180,175 @@ func TestReportRowsMultiCurrencyTotals(t *testing.T) { } } +// Under GroupByTotal — the grouping the report loads with — the single bucket IS +// the totals row: same hours, same billed, and the same amounts, because one +// bucket collects every entry. The table used to show "Total" directly above +// "TOTAL", differing only in case and color (#137). +func TestReportRowsSuppressesTheBucketUnderTotalGrouping(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 15.5, BilledHours: 12.5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 15.5, BilledHours: 12.5, Amount: 625}}, + TotalHours: 15.5, BilledHours: 12.5, TotalAmount: 625, + } + rows, firstTotal := reportRows(r) + if len(rows) != 1 { + t.Fatalf("got %d rows, want 1 (the TOTAL row alone): %v", len(rows), rows) + } + if firstTotal != 0 { + t.Errorf("firstTotal = %d, want 0 (the TOTAL row is the first row)", firstTotal) + } + if rows[0][0] != "TOTAL" { + t.Errorf("rows[0][0] = %q, want %q", rows[0][0], "TOTAL") + } +} + +// Multi-currency keeps its shape: TOTAL with an empty Amount, then one subtotal +// row per currency. The suppressed bucket carried "A + B", which is the same +// information those subtotal rows carry. +func TestReportRowsSuppressesTheBucketUnderTotalGroupingMultiCurrency(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Total", Hours: 18, BilledHours: 17, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 337.5}, {Currency: "USD", Amount: 512.5}}}}, + CurrencySubtotals: []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 7.75, BilledHours: 6.75, Amount: 337.5}, + {Currency: "USD", Hours: 10.25, BilledHours: 10.25, Amount: 512.5}, + }, + TotalHours: 18, BilledHours: 17, + } + rows, firstTotal := reportRows(r) + if len(rows) != 3 { + t.Fatalf("got %d rows, want 3 (TOTAL + two subtotals): %v", len(rows), rows) + } + if firstTotal != 0 { + t.Errorf("firstTotal = %d, want 0", firstTotal) + } + if rows[0][3] != "" { + t.Errorf("TOTAL Amount = %q, want empty (the subtotals carry the figures)", rows[0][3]) + } +} + +// The suppression targets the degenerate single-bucket case, not GroupByTotal +// in general: today internal/report's aggregation (aggregate.go's groupKeys, +// GroupByTotal's default branch) only ever emits one bucket for that grouping, +// but nothing in this package enforces that invariant, and the guard reads +// r.GroupBy == GroupByTotal alongside len(r.Buckets) == 1 for a reason — if a +// future change to groupKeys ever produced more than one bucket under +// GroupByTotal, dropping every bucket row on GroupBy alone would silently +// discard real data with nothing to fall back on within a currency's +// breakdown. This pins that the narrower, two-part condition is what is +// actually checked, using two synthetic buckets that report.Build itself +// would never produce today. +func TestReportRowsKeepsMultipleBucketsUnderTotalGrouping(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{ + {Label: "Total A", Hours: 10, BilledHours: 10, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 500}}}, + {Label: "Total B", Hours: 5, BilledHours: 5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 250}}}, + }, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 15, BilledHours: 15, Amount: 750}}, + TotalHours: 15, BilledHours: 15, TotalAmount: 750, + } + rows, firstTotal := reportRows(r) + if len(rows) != 3 || firstTotal != 2 { + t.Fatalf("got %d rows firstTotal=%d, want 3 rows firstTotal=2 (both buckets survive, then TOTAL): %v", + len(rows), firstTotal, rows) + } + if rows[0][0] != "Total A" || rows[1][0] != "Total B" { + t.Errorf("bucket rows = %q, %q, want both bucket labels to survive", rows[0][0], rows[1][0]) + } +} + +// Every other grouping keeps its bucket rows: the suppression is about the +// degenerate one-bucket case of GroupByTotal, not about totals in general. +func TestReportRowsKeepsBucketsUnderOtherGroupings(t *testing.T) { + t.Parallel() + r := report.Report{ + GroupBy: report.GroupByList, + DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: "Website", Hours: 12.5, BilledHours: 12.5}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 12.5, BilledHours: 12.5, Amount: 625}}, + TotalHours: 12.5, BilledHours: 12.5, TotalAmount: 625, + } + rows, firstTotal := reportRows(r) + if len(rows) != 2 || firstTotal != 1 { + t.Fatalf("got %d rows firstTotal=%d, want 2 rows firstTotal=1: %v", len(rows), firstTotal, rows) + } +} + +// reportNumWidth reserved 8 columns for each of Hours and Billed but nothing +// enforced it: a wider value pushed the table past the terminal edge by exactly +// 2 x (width - 8). The overflow needs BOTH factors — a long label (so Item is +// budget-bound rather than capped by its own content) and numbers wider than 8. +// A test missing either one passes against the bug: measured, a long label with +// ordinary hours renders 55 columns at width 60. +// +// 40 and 44 are deliberately absent: the fix cannot fit this table in fewer than +// 48 columns (chrome 10 + Item floor 12 + 10 + 10 + Amount header 6), so +// asserting there would fail against correct code. +func TestReportTableNeverExceedsWidthWithLongLabelAndWideNumbers(t *testing.T) { + t.Parallel() + th := testTheme(true) + const longLabel = "Website redesign and content migration backlog" + rep := reportWithHours(longLabel, 1234567.5) + for _, width := range []int{60, 80, 100} { + got := lipgloss.Width(strings.Split(reportTable(th, rep, width), "\n")[0]) + if got > width { + t.Errorf("width %d: table rendered %d columns", width, got) + } + } +} + +// The other direction: short labels leave Item capped by its own content, but +// wide numbers still push the table past a narrow terminal. 44 is the width +// where the current code renders 47 (overflowing by 3) and the fix renders 44 +// (fitting exactly: Item stays at its content cap of 7 and Amount is trimmed +// by the budget down to 7). 43 is the floor the fix falls back to below this +// width, not what it renders at 44 itself. +func TestReportTableNeverExceedsWidthWithShortLabelAndWideNumbers(t *testing.T) { + t.Parallel() + th := testTheme(true) + rep := reportWithHours("Website", 1234567.5) + if got := lipgloss.Width(strings.Split(reportTable(th, rep, 44), "\n")[0]); got > 44 { + t.Errorf("table rendered %d columns in a 44-column terminal", got) + } +} + +// reportWithHours builds a one-bucket report whose Hours and Billed carry the +// given value, so a test can dial the numeric width independently of the label. +func reportWithHours(label string, hours float64) report.Report { + return report.Report{ + GroupBy: report.GroupByList, DefaultCurrency: "EUR", + Buckets: []report.Bucket{{Label: label, Hours: hours, BilledHours: hours, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}}, + CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: hours, BilledHours: hours, Amount: 625}}, + TotalHours: hours, BilledHours: hours, TotalAmount: 625, + } +} + +// Measuring the data only would make the budget assume 8 (4+4, from "1.00" +// twice) where the renderer uses 11 (5+6, from the "Hours"/"Billed" headers), +// and the table would overflow by 3 — the same bug in a smaller size. +func TestReportNumWidthsIncludeTheHeaders(t *testing.T) { + t.Parallel() + rows := [][]string{{"Website", "1.00", "1.00", "5.00 EUR"}} + hours, billed := reportNumWidths(rows) + if hours != lipgloss.Width(reportHeaders[1]) || billed != lipgloss.Width(reportHeaders[2]) { + t.Errorf("reportNumWidths = (%d, %d), want the header widths (%d, %d)", + hours, billed, lipgloss.Width(reportHeaders[1]), lipgloss.Width(reportHeaders[2])) + } +} + // TestReportStyleFunc is the ONLY check on the grid's colors. The package // goldens run under termenv.Ascii, which strips backgrounds and bold, so a // broken zebra or an uncolored TOTAL would leave every golden byte-identical. diff --git a/internal/tui/report_test.go b/internal/tui/report_test.go index 9cf230b..e2ecd89 100644 --- a/internal/tui/report_test.go +++ b/internal/tui/report_test.go @@ -340,15 +340,14 @@ func TestSparkViewAllZeroSeriesHasNoTrailingWhitespace(t *testing.T) { } // TestTruncateEmptyStringAlwaysEmpty covers the deliberately empty Amount -// cell in the multi-currency TOTAL row (see reportRows): with the n <= 1 -// guard alone, truncate("", n) for n <= 1 returned "…" — an ellipsis standing -// in for nothing, rendered whenever that cell's column shrinks to 1 or less. -// An empty input has nothing to mark as cut, whatever n is. +// cell in the multi-currency TOTAL row (see reportRows): whatever cols +// shrinks to, there is nothing to mark as cut, so truncateWidth must never +// turn an empty cell into a lone ellipsis. func TestTruncateEmptyStringAlwaysEmpty(t *testing.T) { t.Parallel() - for _, n := range []int{0, 1, 5} { - if got := truncate("", n); got != "" { - t.Errorf("truncate(%q, %d) = %q, want %q", "", n, got, "") + for _, cols := range []int{0, 1, 5} { + if got := truncateWidth("", cols); got != "" { + t.Errorf("truncateWidth(%q, %d) = %q, want %q", "", cols, got, "") } } } diff --git a/internal/tui/testdata/budget.golden b/internal/tui/testdata/budget.golden index 37eec25..61aecca 100644 --- a/internal/tui/testdata/budget.golden +++ b/internal/tui/testdata/budget.golden @@ -1,6 +1,6 @@ Budget burn-down -╭────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Website ████████████░░░░░░░░ 62% 625.00 / 1000.00 EUR (remaining 375.00) │ -╰────────────────────────────────────────────────────────────────────────────────────────────╯ \ No newline at end of file +╭──────────────────────────────────────────────────────────────────────────────╮ +│ Website ████████████░░░░░░░░ 62% 625.00 / 1000.00 EUR │ +╰──────────────────────────────────────────────────────────────────────────────╯ \ No newline at end of file diff --git a/internal/tui/testdata/filters_scrolled.golden b/internal/tui/testdata/filters_scrolled.golden new file mode 100644 index 0000000..52817f6 --- /dev/null +++ b/internal/tui/testdata/filters_scrolled.golden @@ -0,0 +1,11 @@ +Filters + + +▸ Lists + [ ] List 00 + [ ] List 01 + [ ] List 02 + [ ] List 03 + [ ] List 04 + [ ] List 05 + [ ] List 06 diff --git a/internal/tui/testdata/palette_narrow.golden b/internal/tui/testdata/palette_narrow.golden index bf5cba7..c5cdb74 100644 --- a/internal/tui/testdata/palette_narrow.golden +++ b/internal/tui/testdata/palette_narrow.golden @@ -5,13 +5,13 @@ Report July 2026 — scope me — grouped by list — tz UTC ├──────────────────────────────────┤ ho│ ▸ Grouping g │ │ Change range/scope m/s │ -╭─│ Reload r │ -│ │ Export e │ -├─│ Rates p │ -│ │ Filters f │ -│ │ Budgets b │ -│ │ Entries v │ -╰─╰──────────────────────────────────╯ +╭─│ Reload r │╮ +│ │ Export e ││ +├─│ Rates p │┤ +│ │ Filters f ││ +│ │ Budgets b ││ +│ │ Entries v ││ +╰─╰──────────────────────────────────╯╯ billable 12.50h · non-billable 3.00h ↑/↓ move · enter run · esc close … \ No newline at end of file diff --git a/internal/tui/testdata/report.golden b/internal/tui/testdata/report.golden index 2ef5536..6183702 100644 --- a/internal/tui/testdata/report.golden +++ b/internal/tui/testdata/report.golden @@ -5,11 +5,11 @@ Report July 2026 — scope me — grouped by list — tz UTC hours/day ▅ ▂▆ ▃█ -╭─────────────────────────────────────────────────────────────────────────╮ -│ Item Hours Billed Amount │ -├─────────────────────────────────────────────────────────────────────────┤ -│ Website — landing page redesign and checkou… 12.50 12.50 625.00 EUR │ -│ Internal 3.00 0.00 0.00 EUR │ -│ TOTAL 15.50 12.50 625.00 EUR │ -╰─────────────────────────────────────────────────────────────────────────╯ +╭──────────────────────────────────────────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├──────────────────────────────────────────────────────────────────────────────┤ +│ Website — landing page redesign and checkout har… 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰──────────────────────────────────────────────────────────────────────────────╯ billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_narrow.golden b/internal/tui/testdata/report_narrow.golden index 0ef5409..02d5657 100644 --- a/internal/tui/testdata/report_narrow.golden +++ b/internal/tui/testdata/report_narrow.golden @@ -5,11 +5,11 @@ Report July 2026 — scope me — grouped by list — tz UTC hours/day ▅ ▂▆ ▃█ -╭─────────────────────────────────────────────────────╮ -│ Item Hours Billed Amount │ -├─────────────────────────────────────────────────────┤ -│ Website — landing page … 12.50 12.50 625.00 EUR │ -│ Internal 3.00 0.00 0.00 EUR │ -│ TOTAL 15.50 12.50 625.00 EUR │ -╰─────────────────────────────────────────────────────╯ +╭──────────────────────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├──────────────────────────────────────────────────────────┤ +│ Website — landing page redes… 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰──────────────────────────────────────────────────────────╯ billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_total.golden b/internal/tui/testdata/report_total.golden new file mode 100644 index 0000000..7c1183d --- /dev/null +++ b/internal/tui/testdata/report_total.golden @@ -0,0 +1,5 @@ +╭──────────────────────────────────╮ +│ Item Hours Billed Amount │ +├──────────────────────────────────┤ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰──────────────────────────────────╯ \ No newline at end of file diff --git a/internal/tui/testdata/report_total_multicurrency.golden b/internal/tui/testdata/report_total_multicurrency.golden new file mode 100644 index 0000000..f73acb5 --- /dev/null +++ b/internal/tui/testdata/report_total_multicurrency.golden @@ -0,0 +1,7 @@ +╭───────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├───────────────────────────────────────────┤ +│ TOTAL 18.00 17.00 │ +│ subtotal EUR 7.75 6.75 337.50 EUR │ +│ subtotal USD 10.25 10.25 512.50 USD │ +╰───────────────────────────────────────────╯ \ No newline at end of file diff --git a/internal/tui/width.go b/internal/tui/width.go new file mode 100644 index 0000000..b01aac8 --- /dev/null +++ b/internal/tui/width.go @@ -0,0 +1,45 @@ +package tui + +import ( + "strings" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" +) + +// This file is the only sanctioned way to fit a string into a column. +// +// Every layout in this package is measured in DISPLAY COLUMNS, but Go counts +// runes in two places that used to be relied on: a rune-sliced cut, and fmt's +// "%-Ns" padding (fmt's width is a rune count, so a 7-rune 14-column string +// padded to 10 comes out 17 columns wide). The two agree only for ASCII, and +// ClickUp list and task names are exactly where emoji and CJK live. +// +// Both helpers take a SINGLE LINE. ansi.Truncate lets a "\n" through, so a +// multi-line input would produce a "cell" whose padding applies to the last line +// only; no call site passes one. + +// truncateWidth cuts s to at most cols display columns, the ellipsis included. +// cols <= 0 returns "". +func truncateWidth(s string, cols int) string { + if cols <= 0 { + return "" + } + return ansi.Truncate(s, cols, "…") +} + +// cell renders s into exactly cols display columns: truncated when too wide, +// space-padded when too narrow. It replaces the truncate(s, N) + "%-Ns" pair, +// where both halves counted runes. +// +// The pad is not cosmetic. ansi.Truncate drops a wide cluster whole rather than +// splitting it, so a cut can land NARROWER than cols ("日本語のリスト" cut to 2 +// gives "…", one column). Without the measured pad the column misaligns anyway, +// just in the other direction. +func cell(s string, cols int) string { + s = truncateWidth(s, cols) + if pad := cols - lipgloss.Width(s); pad > 0 { + s += strings.Repeat(" ", pad) + } + return s +} diff --git a/internal/tui/width_test.go b/internal/tui/width_test.go new file mode 100644 index 0000000..8928834 --- /dev/null +++ b/internal/tui/width_test.go @@ -0,0 +1,139 @@ +package tui + +import ( + "fmt" + "testing" + + "github.com/charmbracelet/lipgloss" +) + +// Display width and rune count agree only for ASCII, which is why every fixture +// here that matters is not ASCII. +func TestTruncateWidth(t *testing.T) { + tests := []struct { + name string + s string + cols int + want string + }{ + {"empty", "", 5, ""}, + {"fits", "hello", 10, "hello"}, + {"exact", "hello", 5, "hello"}, + {"cuts ascii", "hello world", 5, "hell…"}, + {"zero cols", "hello", 0, ""}, + {"negative cols", "hello", -3, ""}, + {"one col fits", "x", 1, "x"}, + {"one col cuts", "xy", 1, "…"}, + // A wide cluster is dropped whole rather than split, so the result can + // land NARROWER than cols. That is what makes the pad in cell() load-bearing. + {"wide runes cut on the boundary", "日本語のリスト", 2, "…"}, + {"wide runes cut inside", "日本語のリスト", 5, "日本…"}, + {"emoji", "🚀🚀🚀🚀", 3, "🚀…"}, + {"mixed", "ab🚀cd", 5, "ab🚀…"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := truncateWidth(tc.s, tc.cols) + if got != tc.want { + t.Errorf("truncateWidth(%q, %d) = %q, want %q", tc.s, tc.cols, got, tc.want) + } + if w := lipgloss.Width(got); tc.cols > 0 && w > tc.cols { + t.Errorf("truncateWidth(%q, %d) rendered %d columns, want at most %d", tc.s, tc.cols, w, tc.cols) + } + }) + } +} + +// cell must produce EXACTLY cols columns, which is the half that fmt's "%-Ns" +// gets wrong: fmt pads by rune count, so a 7-rune 14-column string padded to +// "%-10s" comes out 17 columns wide. +func TestCellIsExactlyColsWide(t *testing.T) { + for _, s := range []string{"", "x", "hello", "hello world", "日本語のリスト", "🚀🚀🚀🚀", "ab🚀cd"} { + for _, cols := range []int{1, 2, 5, 10, 24} { + got := cell(s, cols) + if w := lipgloss.Width(got); w != cols { + t.Errorf("cell(%q, %d) rendered %d columns, want exactly %d (%q)", s, cols, w, cols, got) + } + } + } +} + +func TestCellNonPositiveCols(t *testing.T) { + for _, cols := range []int{0, -1, -12} { + if got := cell("日本語", cols); got != "" { + t.Errorf("cell(%q, %d) = %q, want %q", "日本語", cols, got, "") + } + } +} + +// The migration in Task 2 must be a no-op for ASCII content at cols >= 2: that +// is what keeps every existing golden green, so the wide-rune tests are the only +// thing that distinguishes before from after. cols == 1 is the one documented +// divergence — the old rune cut returned "…" even when the input fit. +func TestCellMatchesTheOldPaddedCutForASCII(t *testing.T) { + for _, s := range []string{"", "x", "Website", "Website redesign", "Mobile app"} { + for _, cols := range []int{2, 5, 20, 24, 40} { + want := fmt.Sprintf("%-*s", cols, oldRuneCutForTest(s, cols)) + if got := cell(s, cols); got != want { + t.Errorf("cell(%q, %d) = %q, want %q (the pre-migration rendering)", s, cols, got, want) + } + } + } +} + +// oldRuneCutForTest is the rune-based cut this tranche deletes, kept in the test +// file only, as the reference the ASCII equivalence is measured against. +func oldRuneCutForTest(s string, n int) string { + if s == "" { + return "" + } + if n <= 1 { + return "…" + } + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[:n-1]) + "…" +} + +// The report footer cuts styled strings, so the cut must survive escapes. +// TestMain pins termenv.Ascii for this package, so a theme style renders without +// escapes: the fixture writes them by hand instead. +func TestTruncateWidthIsANSIAware(t *testing.T) { + const styled = "\x1b[1mabcdef\x1b[0m" + got := truncateWidth(styled, 3) + if want := "\x1b[1mab…\x1b[0m"; got != want { + t.Errorf("truncateWidth(%q, 3) = %q, want %q", styled, got, want) + } + if w := lipgloss.Width(got); w != 3 { + t.Errorf("truncated styled string rendered %d columns, want 3", w) + } +} + +// A style left OPEN by the caller is not closed by the cut: the escape survives +// and bleeds past it. Nothing in production feeds an unterminated style today +// (help.View closes its own), and this test exists so that a caller who starts +// doing it finds out here rather than on a user's terminal. +func TestTruncateWidthDoesNotCloseAnOpenStyle(t *testing.T) { + got := truncateWidth("\x1b[31mabcdef", 3) + if want := "\x1b[31mab…"; got != want { + t.Errorf("truncateWidth of an unterminated style = %q, want %q", got, want) + } +} + +// The rates list, the entries browser and the budget screen all pair a cut with +// a fixed-width column. With a wide-rune list name the row used to render wider +// than its own column, which is what pushed those screens past the terminal +// edge. The fixture is deliberately CJK: an ASCII name passes against the bug. +func TestFixedWidthRowsHoldTheirColumnWithWideRunes(t *testing.T) { + const wide = "日本語のリストの名前がとても長い場合" + if lipgloss.Width(wide) == len([]rune(wide)) { + t.Fatalf("fixture is not wide: %d runes, %d columns", len([]rune(wide)), lipgloss.Width(wide)) + } + for _, cols := range []int{20, 24, 30} { + if w := lipgloss.Width(cell(wide, cols)); w != cols { + t.Errorf("cell(wide, %d) rendered %d columns, want %d", cols, w, cols) + } + } +}