Table cell padding no longer works since 1ba1200

[maxatome]

The commit 1ba1200 of PR #388 breaks my bubbles/table:

Before:

After:

I configure the cell style as:

styleCell = lipgloss.NewStyle().Padding(0, 1, 0, 0)

so one space at the right of the cell, not at the bottom too.

Tx!

[mikelorant]

Are you able to put up some sample code to properly demonstrate this? I've been working on table cells today and it is possible my change (merged into master) may have some impact on the results.

[maxatome]

Hello @mikelorant, after investigating further, it is due to too long lines in table no longer truncated since 1ba1200.

See https://gist.github.com/maxatome/21d7fd2f08244b25994464b2ef776cf2

Launch it then reduce the width of the terminal: the text will wrap while it was truncated before.

Perhaps am I missing a new option to avoid wrapping?

[mikelorant]

I've taken the time to try and clarify the exact issue. Relying on resizing the window is complicating the code and was unnecessary to finding truncation issues.

Can you verify with this small example this is the problem you are experiencing?

package main

import (
	"github.com/charmbracelet/bubbles/table"
	tea "github.com/charmbracelet/bubbletea"
	"github.com/charmbracelet/lipgloss"
)

type model struct {
	table table.Model
}

func (m model) Init() tea.Cmd {
	return nil
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	var cmd tea.Cmd
	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "q", "ctrl+c":
			return m, tea.Quit
		}
	}
	m.table, cmd = m.table.Update(msg)
	return m, cmd
}

func (m model) View() string {
	style := lipgloss.NewStyle().BorderStyle(lipgloss.NormalBorder())

	return style.Render(m.table.View())
}

func main() {
	columns := []table.Column{
		{Title: "ID", Width: 4},
		{Title: "Title", Width: 10},
		{Title: "Description", Width: 10},
	}

	rows := []table.Row{
		{"1", "Lost in Time", "A thrilling adventure through the ages."},
		{"2", "Whispering Shadows", "Secrets unravel in a haunted town."},
		{"3", "Stolen Identity", "An innocent man's life turned upside down."},
		{"4", "Into the Abyss", "Exposing deep-rooted conspiracies."},
	}

	t := table.New(
		table.WithColumns(columns),
		table.WithRows(rows),
		table.WithFocused(true),
		table.WithWidth(20),
		table.WithHeight(4),
	)

	tea.NewProgram(model{t}).Run()
}

When I run this I get the following result:

To me, this shows that truncating is functioning and elipsis are added. However, the text is not truncated to the correct width. This pushes the text to the next line.

Just want to make sure we have a clear example and are on the same page for the issue you are experiencing before I dig into the exact cause of this bug.

[mikelorant]

Have worked out the main cause of the problem.

In the code above I have set a table width of 20.

The 3 fields above combined have a width 24 (4 + 10 + 10).

Adding some debugging output I got this:

Index 0 Width 30 Line  1     Lost in T…  A thrilli…
Index 1 Width 30 Line  2     Whisperin…  Secrets u…
Index 2 Width 30 Line  3     Stolen Id…  An innoce…
Index 3 Width 30 Line  4     Into the …  Exposing …
VP Width 20

We are creating in the viewport lines that are 30 wide but the viewport is only able to show 20.

How is the width set:

// WithWidth sets the width of the table.
func WithWidth(w int) Option {
	return func(m *Model) {
		m.viewport.Width = w
	}
}

Can you see the problem?

When rendering the row, we never take into account the total width of the viewport.

[mikelorant]

Next steps is to find out why the viewport is not trimming the line lengths correctly. It knows the visible area, these rows are 30 wide and need to be cut down to a width of 20.

[mikelorant]

Bit more a brain dump:

Looking at the MaxWidth section of Render()

// Truncate according to MaxWidth
if maxWidth > 0 {
  lines := strings.Split(str, "\n")

  for i := range lines {
    lines[i] = truncate.String(lines[i], uint(maxWidth))
  }

  str = strings.Join(lines, "\n")
}

However take a look at the Width section of Render():

// Word wrap
if !inline && width > 0 {
  wrapAt := width - leftPadding - rightPadding
  str = wordwrap.String(str, wrapAt)
  str = wrap.String(str, wrapAt) // force-wrap long strings
}

The width code does not take into account multiple lines.

[mikelorant]

@maaslalani How would you recommend we resolve this?

Options:

• Add MinWidth and MinHeight functions.
• Update Width to support multiple lines.
• Modify Viewport to iterate over the lines and set Width.

[mikelorant]

@wesleimp I know you are the primary author of this component so I thought it might be worthwhile bringing you into this issue to get some assistance.

Can you explain how width was meant to work? Is it meant to act as the exact value (not minimum or maximum)?

Right now, components like the header never take into account the width. Therefore the header sets the actual size of the component.

See this code:

func (m Model) headersView() string {
	var s = make([]string, 0, len(m.cols))
	for _, col := range m.cols {
		style := lipgloss.NewStyle().Width(col.Width).MaxWidth(col.Width).Inline(true)
		renderedCell := style.Render(runewidth.Truncate(col.Title, col.Width, "…"))
		s = append(s, m.styles.Header.Render(renderedCell))
	}
	return lipgloss.JoinHorizontal(lipgloss.Left, s...)
}

The table is assembled by combing the viewport with the header:

// View renders the component.
func (m Model) View() string {
	return m.headersView() + "\n" + m.viewport.View()
}

Referencing how the table components accepts width:

// WithWidth sets the width of the table.
func WithWidth(w int) Option {
	return func(m *Model) {
		m.viewport.Width = w
	}
}

There are some serious fundamental problems with this logic within this component.

To fix these problems we need to understand how this should work.

[mikelorant]

Please don't take this the wrong way, but I feel most of problems with the table component stem from a lack of tests. These needs as many tests if not more than the table that is part of Lipgloss. @maaslalani had very extensive testing and the benefits paid off when it comes to refactoring and fixes.

We might be better off pausing on fixes and instead focus on adding as many unit tests as possible to this. From there we can identify how things should work versus the reality. Then we can actually begin to address problems such as this one.

My challenges are not fixing this problem, but knowing how it should work. I don't have a frame of reference.

[mikelorant]

@ivanvc I think we should consider reverting #388 as this is causing a number of issues.

[ivanvc]

Please feel free to revert it. I wasn't aware that it'd break other components. ✌️

[maxatome]

Yes @mikelorant we are talking about the same problem, thanks for your investigations!

[maaslalani]

Please don't take this the wrong way, but I feel most of problems with the table component stem from a lack of tests. These needs as many tests if not more than the table that is part of Lipgloss. @maaslalani had very extensive testing and the benefits paid off when it comes to refactoring and fixes.

@mikelorant Totally agree on the lack of tests. We want to replace this table bubble with the lipgloss table for better rendering by the way, which should fix a majority of the issues we are seeing here as well.

[maaslalani]

Bit more a brain dump:

Looking at the MaxWidth section of Render()

// Truncate according to MaxWidth
if maxWidth > 0 {
  lines := strings.Split(str, "\n")

  for i := range lines {
    lines[i] = truncate.String(lines[i], uint(maxWidth))
  }

  str = strings.Join(lines, "\n")
}

However take a look at the Width section of Render():

// Word wrap
if !inline && width > 0 {
  wrapAt := width - leftPadding - rightPadding
  str = wordwrap.String(str, wrapAt)
  str = wrap.String(str, wrapAt) // force-wrap long strings
}

The width code does not take into account multiple lines.

It looks like we need to set MaxWidth on the table which accounts for the padding so that it truncates rather than wraps.

[mikelorant]

@maaslalani Won't work. All because of this:

// View renders the component.
func (m Model) View() string {
	return m.headersView() + "\n" + m.viewport.View()
}

Consider this:

• Width set to 20
• Computed width of 3 columns: 4, 10 and 10 = 24

You get the header view with a width of 24 put joined to a viewport truncated to 20 wide. These need to match otherwise you are going to get a very messed up table.

Proper solution is to add Width field to the model. This will allow the header to know the final width when rendered.

However, how do we "scale" when the columns are wider than the desired width? Do we just truncate at the end or do we attempt to narrow the columns?

Decide how it works first, then we code it.