From 3baa429f210ee8b317e8906095311c3bd8f5260a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 15 Mar 2026 19:08:50 -0700 Subject: [PATCH 1/3] Protect URL columns from being dropped in selectColumns() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column-dropping loop removed URL columns first (lowest priority) when their accurate width pushed the total past the terminal width. A dropped URL is worse than a garbled one — an overflowing table at least keeps the link clickable. Now the loop skips URL columns and drops only non-URL columns from the right. --- internal/output/output_test.go | 21 ++++++++++++++++++--- internal/output/render.go | 14 +++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/internal/output/output_test.go b/internal/output/output_test.go index e556a6a90..be2b970a4 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -2215,9 +2215,7 @@ func TestMarkdownRenderObjectPreservesURLs(t *testing.T) { } func TestStyledRenderTablePreservesURLs(t *testing.T) { - // URL must be >40 chars (to exercise the cap exemption) but short enough - // to fit beside "name" in an 80-char default terminal. - url := "https://3.basecampapi.com/1234567/todolists/98765.json" + url := "https://3.basecampapi.com/1234567/buckets/12345678/todolists/9876543210.json" data := []any{ map[string]any{"name": "Tasks", "todolists_url": url}, } @@ -2526,6 +2524,23 @@ func TestSelectColumnsExemptsURLColumnsForSuffixFields(t *testing.T) { } } +func TestSelectColumnsPreservesURLColumnsWhenOverflowing(t *testing.T) { + url := "https://3.basecampapi.com/1234567/buckets/12345678/people/9999999.json" + r := &Renderer{width: 80} + cols := []column{ + {key: "name", header: "Name", priority: 2}, + {key: "href", header: "Href", priority: 5}, + } + data := []map[string]any{ + {"name": "Alice", "href": url}, + } + selected := r.selectColumns(cols, data) + + require.Len(t, selected, 2, "both columns should survive even though total exceeds terminal width") + assert.Equal(t, "href", selected[1].key) + assert.Equal(t, len(url), selected[1].width, "URL column should retain its full measured width") +} + // ============================================================================= // updated_at Omission in Generic Tables // ============================================================================= diff --git a/internal/output/render.go b/internal/output/render.go index 277602956..7e7d0bb2a 100644 --- a/internal/output/render.go +++ b/internal/output/render.go @@ -529,7 +529,19 @@ func (r *Renderer) selectColumns(cols []column, data []map[string]any) []column if total <= r.width { break } - selected = selected[:len(selected)-1] + // Drop the rightmost non-URL column. A dropped URL is useless; + // an overflowing table at least keeps the link clickable. + dropped := false + for j := len(selected) - 1; j >= 1; j-- { + if !selected[j].containsURL { + selected = append(selected[:j], selected[j+1:]...) + dropped = true + break + } + } + if !dropped { + break // only the lead column and URL columns remain + } } return selected From 8ea3dd06af8f81b73e3d32e33e73ff94e891bd37 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 15 Mar 2026 19:29:23 -0700 Subject: [PATCH 2/3] Guard against vacuous pass in URL suffix column test Add a found-or-fail assertion so the test actually fails if the URL column gets dropped instead of silently passing with zero assertions. --- internal/output/output_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/output/output_test.go b/internal/output/output_test.go index be2b970a4..e503e3415 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -2516,12 +2516,15 @@ func TestSelectColumnsExemptsURLColumnsForSuffixFields(t *testing.T) { } selected := r.selectColumns(cols, data) + var found bool for _, col := range selected { if col.key == "todolists_url" { + found = true assert.Greater(t, col.width, 40, "URL column with _url suffix should retain actual width") assert.True(t, col.containsURL, "URL column should be flagged") } } + require.True(t, found, "todolists_url column must be present in selected columns") } func TestSelectColumnsPreservesURLColumnsWhenOverflowing(t *testing.T) { From 9939ddab7418675febc03bb5ab961c84206895bc Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 15 Mar 2026 19:35:28 -0700 Subject: [PATCH 3/3] Shrink renderer width so overflow test actually overflows The fixture totalled 79 chars which fit within width 80, so the drop loop never fired and the test passed vacuously with both old and new logic. Use width 60 to force the overflow path. --- internal/output/output_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/output/output_test.go b/internal/output/output_test.go index e503e3415..cfb1999a4 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -2529,7 +2529,7 @@ func TestSelectColumnsExemptsURLColumnsForSuffixFields(t *testing.T) { func TestSelectColumnsPreservesURLColumnsWhenOverflowing(t *testing.T) { url := "https://3.basecampapi.com/1234567/buckets/12345678/people/9999999.json" - r := &Renderer{width: 80} + r := &Renderer{width: 60} // URL (70) + name (5) + padding (4) = 79 > 60 cols := []column{ {key: "name", header: "Name", priority: 2}, {key: "href", header: "Href", priority: 5}, @@ -2539,7 +2539,7 @@ func TestSelectColumnsPreservesURLColumnsWhenOverflowing(t *testing.T) { } selected := r.selectColumns(cols, data) - require.Len(t, selected, 2, "both columns should survive even though total exceeds terminal width") + require.Len(t, selected, 2, "both columns should survive even when total exceeds terminal width") assert.Equal(t, "href", selected[1].key) assert.Equal(t, len(url), selected[1].width, "URL column should retain its full measured width") }