From 2bba43850617035cf9c8b3d6f1e8ee355cd82593 Mon Sep 17 00:00:00 2001 From: bashbunni Date: Wed, 16 Oct 2024 18:08:31 -0700 Subject: [PATCH 1/6] test(table): modify test to show bug --- table/table_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/table/table_test.go b/table/table_test.go index 09175eb2..1e0da240 100644 --- a/table/table_test.go +++ b/table/table_test.go @@ -1149,9 +1149,9 @@ func TestStyleFunc(t *testing.T) { return lipgloss.NewStyle().Align(lipgloss.Center) // this is the first row of data case row == 0: - return lipgloss.NewStyle().Padding(0, 1).Align(lipgloss.Right) + return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right) default: - return lipgloss.NewStyle().Padding(0, 1) + return lipgloss.NewStyle().Margin(0, 1) } } From 1f9dbe855b41898f1fae2d8540faabb16aa1b9c0 Mon Sep 17 00:00:00 2001 From: bashbunni Date: Thu, 17 Oct 2024 13:24:46 -0700 Subject: [PATCH 2/6] fix(table): account for margin in cell height and width --- table/table.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/table/table.go b/table/table.go index 650f1400..0cf3de3e 100644 --- a/table/table.go +++ b/table/table.go @@ -552,10 +552,12 @@ func (t *Table) constructRow(index int, isOverflow bool) string { cell = t.data.At(index, c) } - cells = append(cells, t.style(index, c). - Height(height). + cellStyle := t.style(index, c) + cells = append(cells, cellStyle. + // Account for the margins in the cell sizing. + Height(height-cellStyle.GetVerticalMargins()). MaxHeight(height). - Width(t.widths[c]). + Width(t.widths[c]-cellStyle.GetHorizontalMargins()). MaxWidth(t.widths[c]). Render(ansi.Truncate(cell, cellWidth*height, "…"))) From 3bea852c7bc75a2524ce56ee282d11c64c82d869 Mon Sep 17 00:00:00 2001 From: bashbunni Date: Thu, 17 Oct 2024 14:07:01 -0700 Subject: [PATCH 3/6] test(table): add sizing tests for margins and padding --- table/table_test.go | 102 ++++++++++++++---- ...margin_and_padding_with_fixed_width.golden | 49 +++++++++ .../margin_and_padding_set.golden | 29 +++++ .../right-aligned_text_with_margins.golden | 9 ++ 4 files changed, 167 insertions(+), 22 deletions(-) create mode 100644 table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden create mode 100644 table/testdata/TestStyleFunc/margin_and_padding_set.golden create mode 100644 table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden diff --git a/table/table_test.go b/table/table_test.go index 1e0da240..72096f21 100644 --- a/table/table_test.go +++ b/table/table_test.go @@ -1142,30 +1142,88 @@ func TestTableHeightWithOffset(t *testing.T) { } func TestStyleFunc(t *testing.T) { - TestStyle := func(row, col int) lipgloss.Style { - switch { - // this is the header - case row == HeaderRow: - return lipgloss.NewStyle().Align(lipgloss.Center) - // this is the first row of data - case row == 0: - return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right) - default: - return lipgloss.NewStyle().Margin(0, 1) - } + tests := []struct { + name string + style StyleFunc + }{ + { + "right-aligned text with margins", + func(row, col int) lipgloss.Style { + switch { + case row == HeaderRow: + return lipgloss.NewStyle().Align(lipgloss.Center) + case row == 0: + return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right) + default: + return lipgloss.NewStyle().Margin(0, 1) + } + }, + }, + { + "margin and padding set", + func(row, col int) lipgloss.Style { + switch { + case row == HeaderRow: + return lipgloss.NewStyle().Align(lipgloss.Center) + default: + return lipgloss.NewStyle(). + Padding(1). + Margin(1). + // kept this example right-aligned as that seems to be + // the most vulnerable to accidental truncation. + Align(lipgloss.Right). + Background(lipgloss.Color("#874bfc")) + } + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + table := New(). + Border(lipgloss.NormalBorder()). + StyleFunc(tc.style). + Headers("LANGUAGE", "FORMAL", "INFORMAL"). + Row("Chinese", "Nǐn hǎo", "Nǐ hǎo"). + Row("French", "Bonjour", "Salut"). + Row("Japanese", "こんにちは", "やあ"). + Row("Russian", "Zdravstvuyte", "Privet"). + Row("Spanish", "Hola", "¿Qué tal?") + + golden.RequireEqual(t, []byte(table.String())) + }) } +} - table := New(). - Border(lipgloss.NormalBorder()). - StyleFunc(TestStyle). - Headers("LANGUAGE", "FORMAL", "INFORMAL"). - Row("Chinese", "Nǐn hǎo", "Nǐ hǎo"). - Row("French", "Bonjour", "Salut"). - Row("Japanese", "こんにちは", "やあ"). - Row("Russian", "Zdravstvuyte", "Privet"). - Row("Spanish", "Hola", "¿Qué tal?") - - golden.RequireEqual(t, []byte(table.String())) +func TestSizing(t *testing.T) { + t.Run("margin and padding with fixed width", func(t *testing.T) { + table := New(). + Border(lipgloss.NormalBorder()). + StyleFunc(func(row, col int) lipgloss.Style { + switch { + case row == HeaderRow: + return lipgloss.NewStyle().Align(lipgloss.Center) + default: + return lipgloss.NewStyle(). + Padding(2). + Margin(2). + Align(lipgloss.Right). + Background(lipgloss.Color("#874bfc")) + } + }). + Headers("LANGUAGE", "FORMAL", "INFORMAL"). + Row("Chinese", "Nǐn hǎo", "Nǐ hǎo"). + Row("French", "Bonjour", "Salut"). + Row("Japanese", "こんにちは", "やあ"). + Row("Russian", "Zdravstvuyte", "Privet"). + Row("Spanish", "Hola", "¿Qué tal?"). + Width(50). + Height(40) + + // TODO height isn't working - need to rebase master + t.Logf("\n%s", table.String()) + golden.RequireEqual(t, []byte(table.String())) + }) } func TestClearRows(t *testing.T) { diff --git a/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden b/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden new file mode 100644 index 00000000..c514522e --- /dev/null +++ b/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden @@ -0,0 +1,49 @@ +┌───────────────┬────────────────┬───────────────┐ +│ LANGUAGE │ FORMAL │ INFORMAL │ +├───────────────┼────────────────┼───────────────┤ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   │   │   │ +│  Chinese  │   Nǐn hǎo  │   Nǐ hǎo  │ +│   │   │   │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   │   │   │ +│   French  │   Bonjour  │   Salut  │ +│   │   │   │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   │   │   │ +│  Japanes  │  こんにち  │   やあ  │ +│   e  │   は  │   │ +│   │   │   │ +│   │   │ │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   │   │   │ +│  Russian  │  Zdravstv  │   Privet  │ +│   │   uyte  │   │ +│   │   │   │ +│ │   │ │ +│ │ │ │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   │   │   │ +│  Spanish  │   Hola  │   ¿Qué  │ +│   │   │   tal?  │ +│   │   │   │ +│ │ │   │ +│ │ │ │ +└───────────────┴────────────────┴───────────────┘ \ No newline at end of file diff --git a/table/testdata/TestStyleFunc/margin_and_padding_set.golden b/table/testdata/TestStyleFunc/margin_and_padding_set.golden new file mode 100644 index 00000000..00cecd43 --- /dev/null +++ b/table/testdata/TestStyleFunc/margin_and_padding_set.golden @@ -0,0 +1,29 @@ +┌────────────┬────────────────┬─────────────┐ +│ LANGUAGE │ FORMAL │ INFORMAL │ +├────────────┼────────────────┼─────────────┤ +│ │ │ │ +│   │   │   │ +│   Chinese  │   Nǐn hǎo  │   Nǐ hǎo  │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   French  │   Bonjour  │   Salut  │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│  Japanese  │   こんにちは  │   やあ  │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   Russian  │  Zdravstvuyte  │   Privet  │ +│   │   │   │ +│ │ │ │ +│ │ │ │ +│   │   │   │ +│   Spanish  │   Hola  │  ¿Qué tal?  │ +│   │   │   │ +│ │ │ │ +└────────────┴────────────────┴─────────────┘ \ No newline at end of file diff --git a/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden b/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden new file mode 100644 index 00000000..e81cb45b --- /dev/null +++ b/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden @@ -0,0 +1,9 @@ +┌──────────┬──────────────┬───────────┐ +│ LANGUAGE │ FORMAL │ INFORMAL │ +├──────────┼──────────────┼───────────┤ +│ Chinese │ Nǐn hǎo │ Nǐ hǎo │ +│ French │ Bonjour │ Salut │ +│ Japanese │ こんにちは │ やあ │ +│ Russian │ Zdravstvuyte │ Privet │ +│ Spanish │ Hola │ ¿Qué tal? │ +└──────────┴──────────────┴───────────┘ \ No newline at end of file From 0f3ea1612d8fa89b8f5e7922db4f75ffc9dca4de Mon Sep 17 00:00:00 2001 From: bashbunni Date: Thu, 17 Oct 2024 20:52:19 -0700 Subject: [PATCH 4/6] test(table): move TestSizing to another branch --- table/table_test.go | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/table/table_test.go b/table/table_test.go index 72096f21..9b6b0890 100644 --- a/table/table_test.go +++ b/table/table_test.go @@ -1195,37 +1195,6 @@ func TestStyleFunc(t *testing.T) { } } -func TestSizing(t *testing.T) { - t.Run("margin and padding with fixed width", func(t *testing.T) { - table := New(). - Border(lipgloss.NormalBorder()). - StyleFunc(func(row, col int) lipgloss.Style { - switch { - case row == HeaderRow: - return lipgloss.NewStyle().Align(lipgloss.Center) - default: - return lipgloss.NewStyle(). - Padding(2). - Margin(2). - Align(lipgloss.Right). - Background(lipgloss.Color("#874bfc")) - } - }). - Headers("LANGUAGE", "FORMAL", "INFORMAL"). - Row("Chinese", "Nǐn hǎo", "Nǐ hǎo"). - Row("French", "Bonjour", "Salut"). - Row("Japanese", "こんにちは", "やあ"). - Row("Russian", "Zdravstvuyte", "Privet"). - Row("Spanish", "Hola", "¿Qué tal?"). - Width(50). - Height(40) - - // TODO height isn't working - need to rebase master - t.Logf("\n%s", table.String()) - golden.RequireEqual(t, []byte(table.String())) - }) -} - func TestClearRows(t *testing.T) { defer func() { if r := recover(); r != nil { From 8f7dc3597a8172e49961de19dc8b3c31abf4399e Mon Sep 17 00:00:00 2001 From: bashbunni Date: Thu, 17 Oct 2024 21:25:32 -0700 Subject: [PATCH 5/6] chore: tidy + force truecolor in test --- table/table_test.go | 12 +++-- ...margin_and_padding_with_fixed_width.golden | 49 ------------------- table/testdata/TestStyleFunc.golden | 9 ---- .../right-aligned_text_with_margins.golden | 8 +-- 4 files changed, 11 insertions(+), 67 deletions(-) delete mode 100644 table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden delete mode 100644 table/testdata/TestStyleFunc.golden diff --git a/table/table_test.go b/table/table_test.go index 9b6b0890..ce4c8fb4 100644 --- a/table/table_test.go +++ b/table/table_test.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/x/ansi" "github.com/charmbracelet/x/exp/golden" + "github.com/muesli/termenv" ) var TableStyle = func(row, col int) lipgloss.Style { @@ -1142,6 +1143,7 @@ func TestTableHeightWithOffset(t *testing.T) { } func TestStyleFunc(t *testing.T) { + lipgloss.SetColorProfile(termenv.TrueColor) tests := []struct { name string style StyleFunc @@ -1152,15 +1154,15 @@ func TestStyleFunc(t *testing.T) { switch { case row == HeaderRow: return lipgloss.NewStyle().Align(lipgloss.Center) - case row == 0: - return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right) default: - return lipgloss.NewStyle().Margin(0, 1) + return lipgloss.NewStyle().Margin(0, 1).Align(lipgloss.Right) } }, }, { "margin and padding set", + // this test case uses background colors to differentiate margins + // and padding. func(row, col int) lipgloss.Style { switch { case row == HeaderRow: @@ -1169,8 +1171,8 @@ func TestStyleFunc(t *testing.T) { return lipgloss.NewStyle(). Padding(1). Margin(1). - // kept this example right-aligned as that seems to be - // the most vulnerable to accidental truncation. + // keeping right-aligned text as it's the most likely to + // be broken when truncated. Align(lipgloss.Right). Background(lipgloss.Color("#874bfc")) } diff --git a/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden b/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden deleted file mode 100644 index c514522e..00000000 --- a/table/testdata/TestSizing/margin_and_padding_with_fixed_width.golden +++ /dev/null @@ -1,49 +0,0 @@ -┌───────────────┬────────────────┬───────────────┐ -│ LANGUAGE │ FORMAL │ INFORMAL │ -├───────────────┼────────────────┼───────────────┤ -│ │ │ │ -│ │ │ │ -│   │   │   │ -│   │   │   │ -│  Chinese  │   Nǐn hǎo  │   Nǐ hǎo  │ -│   │   │   │ -│   │   │   │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│   │   │   │ -│   │   │   │ -│   French  │   Bonjour  │   Salut  │ -│   │   │   │ -│   │   │   │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│   │   │   │ -│   │   │   │ -│  Japanes  │  こんにち  │   やあ  │ -│   e  │   は  │   │ -│   │   │   │ -│   │   │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│   │   │   │ -│   │   │   │ -│  Russian  │  Zdravstv  │   Privet  │ -│   │   uyte  │   │ -│   │   │   │ -│ │   │ │ -│ │ │ │ -│ │ │ │ -│ │ │ │ -│   │   │   │ -│   │   │   │ -│  Spanish  │   Hola  │   ¿Qué  │ -│   │   │   tal?  │ -│   │   │   │ -│ │ │   │ -│ │ │ │ -└───────────────┴────────────────┴───────────────┘ \ No newline at end of file diff --git a/table/testdata/TestStyleFunc.golden b/table/testdata/TestStyleFunc.golden deleted file mode 100644 index e81cb45b..00000000 --- a/table/testdata/TestStyleFunc.golden +++ /dev/null @@ -1,9 +0,0 @@ -┌──────────┬──────────────┬───────────┐ -│ LANGUAGE │ FORMAL │ INFORMAL │ -├──────────┼──────────────┼───────────┤ -│ Chinese │ Nǐn hǎo │ Nǐ hǎo │ -│ French │ Bonjour │ Salut │ -│ Japanese │ こんにちは │ やあ │ -│ Russian │ Zdravstvuyte │ Privet │ -│ Spanish │ Hola │ ¿Qué tal? │ -└──────────┴──────────────┴───────────┘ \ No newline at end of file diff --git a/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden b/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden index e81cb45b..60e6d3c1 100644 --- a/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden +++ b/table/testdata/TestStyleFunc/right-aligned_text_with_margins.golden @@ -2,8 +2,8 @@ │ LANGUAGE │ FORMAL │ INFORMAL │ ├──────────┼──────────────┼───────────┤ │ Chinese │ Nǐn hǎo │ Nǐ hǎo │ -│ French │ Bonjour │ Salut │ -│ Japanese │ こんにちは │ やあ │ -│ Russian │ Zdravstvuyte │ Privet │ -│ Spanish │ Hola │ ¿Qué tal? │ +│ French │ Bonjour │ Salut │ +│ Japanese │ こんにちは │ やあ │ +│ Russian │ Zdravstvuyte │ Privet │ +│ Spanish │ Hola │ ¿Qué tal? │ └──────────┴──────────────┴───────────┘ \ No newline at end of file From 94c31a651514549d1a4605b5fedda86b7135cbc8 Mon Sep 17 00:00:00 2001 From: bashbunni Date: Fri, 18 Oct 2024 09:31:58 -0700 Subject: [PATCH 6/6] chore(lint): remove nil check on []string --- table/table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/table/table.go b/table/table.go index 0cf3de3e..2a970ffc 100644 --- a/table/table.go +++ b/table/table.go @@ -532,7 +532,7 @@ func (t *Table) constructRows(availableLines int) string { func (t *Table) constructRow(index int, isOverflow bool) string { var s strings.Builder - hasHeaders := t.headers != nil && len(t.headers) > 0 + hasHeaders := len(t.headers) > 0 height := t.heights[index+btoi(hasHeaders)] if isOverflow { height = 1