feat(cli): width-aware item-list table with STATUS/PRIORITY columns (#894)

TASK-2030 (PLAN-1985). Manual ANSI-safe renderer replaces tabwriter; terminal-width-aware title truncation; drops the modifier BY column.
2026-09-10 23:15:40 +00:00 · 2026-07-09 21:47:28 -04:00
parent 502d87b890
commit 4d1034a708
2 changed files with 402 additions and 24 deletions
@@ -3,14 +3,18 @@ package cli
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"os"
+	"regexp"
 	"sort"
 	"strings"
 	"text/tabwriter"
 	"time"
+	"unicode/utf8"

 	"github.com/PerpetualSoftware/pad/internal/models"
 	"github.com/fatih/color"
+	"golang.org/x/term"
 )

 // Color definitions for reuse across the CLI.
@@ -129,48 +133,232 @@ func RelativeTime(t time.Time) string {
 	}
 }

-// PrintItemTable prints items in a formatted table.
+// sgrPattern matches ANSI SGR (Select Graphic Rendition) escape sequences —
+// the color/reset codes fatih/color emits on a TTY. Compiled once and reused
+// to measure the visible width of a colorized string. Declared at package
+// scope so the compile cost is paid a single time.
+var sgrPattern = regexp.MustCompile("\x1b\\[[0-9;]*m")
+
+// displayWidth returns the visible column width of s: ANSI SGR escape
+// sequences are stripped, then the remaining runes are counted. Every rune is
+// approximated as one column — emoji and other wide runes therefore
+// under-count. That is a pre-existing limitation of the CLI's table rendering
+// (matching the old tabwriter behavior) and is intentionally out of scope; the
+// alternative is a new go-runewidth dependency we don't want to add.
+func displayWidth(s string) int {
+	return utf8.RuneCountInString(sgrPattern.ReplaceAllString(s, ""))
+}
+
+// padCell left-aligns s in a column of the given visible width by appending
+// spaces. If s is already at least that wide the string is returned unchanged
+// (never panics on a negative repeat count).
+func padCell(s string, width int) string {
+	gap := width - displayWidth(s)
+	if gap <= 0 {
+		return s
+	}
+	return s + strings.Repeat(" ", gap)
+}
+
+// truncateTitle limits an uncolorized, SGR-free title to maxRunes visible
+// columns, appending an ellipsis when it had to cut. Callers strip SGR escapes
+// first (see the row-build loop), so every rune counts as exactly one column —
+// matching displayWidth — and slicing by runes can never cut mid-escape.
+func truncateTitle(s string, maxRunes int) string {
+	if maxRunes <= 0 {
+		return ""
+	}
+	runes := []rune(s)
+	if len(runes) <= maxRunes {
+		return s
+	}
+	if maxRunes == 1 {
+		return "…"
+	}
+	return string(runes[:maxRunes-1]) + "…"
+}
+
+// itemStatusPriority pulls the "status" and "priority" string values out of an
+// item's fields JSON blob. Missing keys, non-string values, or malformed JSON
+// yield empty strings.
+func itemStatusPriority(fieldsJSON string) (status, priority string) {
+	if fieldsJSON == "" || fieldsJSON == "{}" || fieldsJSON == "null" {
+		return "", ""
+	}
+	var fields map[string]any
+	if err := json.Unmarshal([]byte(fieldsJSON), &fields); err != nil {
+		return "", ""
+	}
+	if s, ok := fields["status"].(string); ok {
+		status = s
+	}
+	if p, ok := fields["priority"].(string); ok {
+		priority = p
+	}
+	return status, priority
+}
+
+// PrintItemTable prints items in a width-aware, ANSI-safe table. It probes the
+// terminal width (falling back to 120 columns when stdout is piped or the size
+// is unavailable) and delegates layout to renderItemTable.
 func PrintItemTable(items []models.Item) {
+	maxWidth := 120
+	if w, _, err := term.GetSize(int(os.Stdout.Fd())); err == nil && w > 0 {
+		maxWidth = w
+	}
+	renderItemTable(os.Stdout, items, maxWidth)
+}
+
+// renderItemTable is the testable core of PrintItemTable: it lays the item
+// list out into maxWidth columns and writes to w. Columns are
+// REF/TITLE · STATUS · PRIORITY · COLLECTION · UPDATED. The four short columns
+// are sized to their widest cell; the flexible REF/TITLE column absorbs the
+// remaining width, truncating ONLY the title (never the ref) so each row fits.
+//
+// tabwriter is deliberately not used here: it counts ANSI color escape bytes
+// as visible width and so misaligns colorized output on a TTY. All width math
+// goes through displayWidth, which strips those escapes.
+func renderItemTable(w io.Writer, items []models.Item, maxWidth int) {
 	if len(items) == 0 {
-		fmt.Println("No items found.")
+		fmt.Fprintln(w, "No items found.")
 		return
 	}

-	w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
 	headerColor := color.New(color.Faint)
-	fmt.Fprintf(w, "%s\t%s\t%s\t%s\n",
-		headerColor.Sprint("TITLE"),
-		headerColor.Sprint("COLLECTION"),
-		headerColor.Sprint("UPDATED"),
-		headerColor.Sprint("BY"),
-	)
+
+	// Pre-render every cell so we can measure visible widths before laying
+	// out. The REF/TITLE cell is assembled later (it depends on the computed
+	// title budget), so we stash its raw pieces.
+	type row struct {
+		pin        string // rendered pin marker ("" or "* " in yellow)
+		ref        string // raw ref, e.g. "TASK-5" — never truncated
+		title      string // raw title — the only thing we truncate
+		archived   bool
+		status     string // rendered short-column cells
+		priority   string
+		collection string
+		updated    string
+	}
+
+	rows := make([]row, 0, len(items))
 	for _, item := range items {
-		pin := ""
+		var r row
 		if item.Pinned {
-			pin = color.YellowString("* ")
+			r.pin = color.YellowString("* ")
 		}
-		ref := ItemRef(item)
-		var titlePart string
-		if ref != "" {
-			titlePart = BoldCyan.Sprint(ref) + "  " + Bold.Sprint(item.Title)
+		r.ref = ItemRef(item)
+		// Strip any SGR escapes a title might carry (we apply our own Bold
+		// styling). This keeps displayWidth == rune count for the title, so
+		// truncateTitle can slice by runes without ever cutting mid-escape or
+		// mis-accounting the width budget.
+		r.title = sgrPattern.ReplaceAllString(item.Title, "")
+		r.archived = item.DeletedAt != nil
+
+		status, priority := itemStatusPriority(item.Fields)
+		if status != "" {
+			r.status = ColorizedStatus(status)
 		} else {
-			titlePart = Bold.Sprint(item.Title)
+			r.status = Dim.Sprint("—")
 		}
-		if item.DeletedAt != nil {
-			titlePart += "  " + Dim.Sprint("(archived)")
+		if priority != "" {
+			r.priority = PriorityColor(priority).Sprint(priority)
+		} else {
+			r.priority = Dim.Sprint("—")
 		}
+
 		collLabel := item.CollectionName
 		if item.CollectionIcon != "" {
 			collLabel = item.CollectionIcon + " " + collLabel
 		}
-		fmt.Fprintf(w, "%s%s\t%s\t%s\t%s\n",
-			pin, titlePart,
-			collLabel,
-			Dim.Sprint(RelativeTime(item.UpdatedAt)),
-			Dim.Sprint(item.LastModifiedBy),
+		r.collection = collLabel
+		r.updated = Dim.Sprint(RelativeTime(item.UpdatedAt))
+		rows = append(rows, r)
+	}
+
+	// Short-column widths: max visible width over the header + every row.
+	statusW := displayWidth(headerColor.Sprint("STATUS"))
+	priorityW := displayWidth(headerColor.Sprint("PRIORITY"))
+	collectionW := displayWidth(headerColor.Sprint("COLLECTION"))
+	updatedW := displayWidth(headerColor.Sprint("UPDATED"))
+	for _, r := range rows {
+		statusW = max(statusW, displayWidth(r.status))
+		priorityW = max(priorityW, displayWidth(r.priority))
+		collectionW = max(collectionW, displayWidth(r.collection))
+		updatedW = max(updatedW, displayWidth(r.updated))
+	}
+
+	const sep = 2 // spaces between columns; 4 gaps between the 5 columns.
+	titleBudget := maxWidth - statusW - priorityW - collectionW - updatedW - sep*4
+	// Clamp to a non-negative floor: on a genuinely narrow terminal the four
+	// fixed columns alone can exceed maxWidth, driving titleBudget negative.
+	// Per DR-1 the ref and the short columns are never truncated, so such a
+	// terminal necessarily overflows — we degrade to "ref + as much title as
+	// fits (often none)" rather than mangle the ref or slice an ANSI escape.
+	// The floor just keeps the width arithmetic well-defined (padCell already
+	// tolerates over-wide cells).
+	if titleBudget < 0 {
+		titleBudget = 0
+	}
+	gap := strings.Repeat(" ", sep)
+
+	// The final column is never padded (avoids trailing whitespace); every
+	// earlier cell is padded to its column width so the columns line up.
+	writeRow := func(refCell, status, priority, collection, updated string) {
+		fmt.Fprintln(w, strings.Join([]string{
+			padCell(refCell, titleBudget),
+			padCell(status, statusW),
+			padCell(priority, priorityW),
+			padCell(collection, collectionW),
+			updated,
+		}, gap))
+	}
+
+	// The flexible-column header obeys the same title budget as the data rows
+	// (a no-op at normal widths where "TITLE" easily fits) so the header can't
+	// overflow past the data cells in the degraded narrow-terminal regime.
+	writeRow(
+		headerColor.Sprint(truncateTitle("TITLE", titleBudget)),
+		headerColor.Sprint("STATUS"),
+		headerColor.Sprint("PRIORITY"),
+		headerColor.Sprint("COLLECTION"),
+		headerColor.Sprint("UPDATED"),
+	)
+
+	for _, r := range rows {
+		writeRow(
+			buildRefCell(r.pin, r.ref, r.title, r.archived, titleBudget),
+			r.status, r.priority, r.collection, r.updated,
 		)
 	}
-	w.Flush()
+}
+
+// buildRefCell assembles the flexible REF/TITLE cell within a visible budget.
+// Only the title is truncated — the pin marker, ref, and "(archived)" suffix
+// are always shown in full — so a narrow terminal degrades to ref + as much
+// title as fits (possibly none) rather than ever hiding or slicing the ref.
+// Truncation happens on the raw title before colorizing, so an ANSI escape is
+// never cut mid-sequence.
+func buildRefCell(pin, ref, title string, archived bool, budget int) string {
+	// Visible width consumed by everything except the title text.
+	fixed := displayWidth(pin)
+	refPrefix := ""
+	if ref != "" {
+		refPrefix = BoldCyan.Sprint(ref) + "  "
+		fixed += displayWidth(ref) + 2
+	}
+	archivedSuffix := ""
+	if archived {
+		archivedSuffix = "  " + Dim.Sprint("(archived)")
+		fixed += 2 + len("(archived)")
+	}
+
+	shown := truncateTitle(title, budget-fixed)
+
+	cell := pin + refPrefix
+	if shown != "" {
+		cell += Bold.Sprint(shown)
+	}
+	return cell + archivedSuffix
 }

 // PrintItemTitles prints just item titles.
@@ -0,0 +1,190 @@
+package cli
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+	"github.com/fatih/color"
+)
+
+// forcePlainColor disables ANSI so width/content assertions are deterministic
+// regardless of whether the test binary's stdout happens to be a TTY.
+func forcePlainColor(t *testing.T) {
+	t.Helper()
+	prev := color.NoColor
+	color.NoColor = true
+	t.Cleanup(func() { color.NoColor = prev })
+}
+
+func TestDisplayWidth_StripsANSIAndCountsRunes(t *testing.T) {
+	// A literal SGR-wrapped string must measure the same as its plain form.
+	colored := "\x1b[1;36mTASK-5\x1b[0m"
+	if got := displayWidth(colored); got != 6 {
+		t.Errorf("displayWidth(colored) = %d, want 6", got)
+	}
+	if got := displayWidth("TASK-5"); got != 6 {
+		t.Errorf("displayWidth(plain) = %d, want 6", got)
+	}
+	if displayWidth(colored) != displayWidth("TASK-5") {
+		t.Errorf("colored and plain widths differ: %d vs %d",
+			displayWidth(colored), displayWidth("TASK-5"))
+	}
+
+	// Multi-byte runes and the ellipsis each count as one column.
+	if got := displayWidth("café"); got != 4 {
+		t.Errorf("displayWidth(café) = %d, want 4", got)
+	}
+	if got := displayWidth("ab…"); got != 3 {
+		t.Errorf("displayWidth(ab…) = %d, want 3", got)
+	}
+}
+
+func TestRenderItemTable_Empty(t *testing.T) {
+	var buf bytes.Buffer
+	renderItemTable(&buf, nil, 120)
+	if got := buf.String(); got != "No items found.\n" {
+		t.Errorf("empty render = %q, want %q", got, "No items found.\n")
+	}
+}
+
+func TestRenderItemTable_TruncatesTitleWithinBudget(t *testing.T) {
+	forcePlainColor(t)
+
+	items := []models.Item{{
+		Title:            strings.Repeat("x", 300),
+		Fields:           `{"status":"open","priority":"high"}`,
+		CollectionName:   "Tasks",
+		CollectionPrefix: "TASK",
+		ItemNumber:       intPtr(1),
+		UpdatedAt:        time.Now(),
+	}}
+
+	const maxWidth = 60
+	var buf bytes.Buffer
+	renderItemTable(&buf, items, maxWidth)
+
+	out := buf.String()
+	for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") {
+		if w := displayWidth(line); w > maxWidth {
+			t.Errorf("line exceeds maxWidth %d (got %d): %q", maxWidth, w, line)
+		}
+	}
+	// The long title had to be cut, so an ellipsis must appear.
+	if !strings.Contains(out, "…") {
+		t.Errorf("expected truncation ellipsis in output, got:\n%s", out)
+	}
+}
+
+func TestRenderItemTable_NeverTruncatesRef(t *testing.T) {
+	forcePlainColor(t)
+
+	items := []models.Item{{
+		Title:            strings.Repeat("y", 200),
+		CollectionName:   "Tasks",
+		CollectionPrefix: "TASK",
+		ItemNumber:       intPtr(123),
+		UpdatedAt:        time.Now(),
+	}}
+
+	var buf bytes.Buffer
+	renderItemTable(&buf, items, 30) // deliberately cramped
+	if !strings.Contains(buf.String(), "TASK-123") {
+		t.Errorf("ref TASK-123 must survive even a cramped width, got:\n%s", buf.String())
+	}
+}
+
+func TestRenderItemTable_StripsANSIFromTitle(t *testing.T) {
+	forcePlainColor(t)
+
+	// A title carrying raw SGR escapes must not slice mid-escape or throw off
+	// the width budget: the escapes are stripped before truncation.
+	items := []models.Item{{
+		Title:            "\x1b[31mRED\x1b[0m" + strings.Repeat("z", 200),
+		CollectionName:   "Tasks",
+		CollectionPrefix: "TASK",
+		ItemNumber:       intPtr(1),
+		UpdatedAt:        time.Now(),
+	}}
+
+	const maxWidth = 60
+	var buf bytes.Buffer
+	renderItemTable(&buf, items, maxWidth)
+	out := buf.String()
+
+	if strings.Contains(out, "\x1b[31m") || strings.Contains(out, "\x1b[0m") {
+		t.Errorf("expected embedded SGR escapes to be stripped from the title, got: %q", out)
+	}
+	for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") {
+		if w := displayWidth(line); w > maxWidth {
+			t.Errorf("line exceeds maxWidth %d (got %d): %q", maxWidth, w, line)
+		}
+	}
+}
+
+func TestRenderItemTable_StatusAndPriorityFromFields(t *testing.T) {
+	forcePlainColor(t)
+
+	items := []models.Item{
+		{
+			Title:            "with fields",
+			Fields:           `{"status":"open","priority":"high"}`,
+			CollectionName:   "Tasks",
+			CollectionPrefix: "TASK",
+			ItemNumber:       intPtr(1),
+			UpdatedAt:        time.Now(),
+		},
+		{
+			Title:            "no fields",
+			Fields:           "{}",
+			CollectionName:   "Docs",
+			CollectionPrefix: "DOC",
+			ItemNumber:       intPtr(2),
+			UpdatedAt:        time.Now(),
+		},
+	}
+
+	var buf bytes.Buffer
+	renderItemTable(&buf, items, 120)
+	out := buf.String()
+
+	for _, want := range []string{"open", "high", "STATUS", "PRIORITY", "TASK-1", "DOC-2"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("expected output to contain %q, got:\n%s", want, out)
+		}
+	}
+	// The BY column is gone.
+	if strings.Contains(out, "BY") {
+		t.Errorf("BY column should have been dropped, got:\n%s", out)
+	}
+	// The fieldless row shows an em-dash placeholder for status/priority.
+	if !strings.Contains(out, "—") {
+		t.Errorf("expected em-dash placeholder for missing status/priority, got:\n%s", out)
+	}
+}
+
+func TestItemStatusPriority(t *testing.T) {
+	cases := []struct {
+		name         string
+		fields       string
+		wantS, wantP string
+	}{
+		{"both", `{"status":"open","priority":"high"}`, "open", "high"},
+		{"empty-object", "{}", "", ""},
+		{"empty-string", "", "", ""},
+		{"malformed", "{not json", "", ""},
+		{"non-string", `{"status":3,"priority":true}`, "", ""},
+		{"status-only", `{"status":"done"}`, "done", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			gotS, gotP := itemStatusPriority(tc.fields)
+			if gotS != tc.wantS || gotP != tc.wantP {
+				t.Errorf("itemStatusPriority(%q) = (%q, %q), want (%q, %q)",
+					tc.fields, gotS, gotP, tc.wantS, tc.wantP)
+			}
+		})
+	}
+}