bubble-table icon indicating copy to clipboard operation
bubble-table copied to clipboard

Columns seem to ignore lipgloss padding

Open misterikkit opened this issue 3 years ago • 7 comments

I want to pad my cell data with a space on both the left and the right. This is because in many terminals, double-clicking on the text of a cell will select the contents plus the column separator. The common behavior is to delimit only on whitespace. What I want is to quickly copy a cell's contents into my OS clipboard for pasting into other tools.

I tried this by adding a column style that uses .Padding(0, 1) as well as manually adding spaces to the row data, but either way the table seems to trim space before rendering. Any hints on how I should be doing this?

I skimmed through the code and did not find any explicit logic to remove leading or trailing space.

Note: For overflow, I don't care if the ellipsis touches the column separator, because a truncated value is not worth copying into a clipboard.

I am using Version: v0.15.0

Sample Program:

package main

import (
	"fmt"

	"github.com/charmbracelet/lipgloss"
	"github.com/evertras/bubble-table/table"
)

func main() {

	cols := []table.Column{
		table.NewColumn("a", "A", 10).
			WithStyle(
				lipgloss.NewStyle().Padding(0, 1),
			),
	}

	rows := []table.Row{
		table.NewRow(table.RowData{"a": "hello"}),
		table.NewRow(table.RowData{"a": "very long text"}),
	}

	t := table.New(cols).WithRows(rows)
	fmt.Println(t.View())
}

Expected Output:

┏━━━━━━━━━━┓
┃        A ┃
┣━━━━━━━━━━┫
┃    hello ┃
┃ very lo… ┃
┗━━━━━━━━━━┛

Actual Output:

┏━━━━━━━━━━┓
┃         A┃
┣━━━━━━━━━━┫
┃     hello┃
┃very long…┃
┗━━━━━━━━━━┛

misterikkit avatar Jan 27 '23 22:01 misterikkit

Interesting result: I can get closer to the format I want with table.NewStyledCell

package main

import (
	"fmt"

	"github.com/charmbracelet/lipgloss"
	"github.com/evertras/bubble-table/table"
)

func main() {

	cols := []table.Column{
		table.NewColumn("a", "A", 10),
	}

	paddedCell := lipgloss.NewStyle().Padding(0, 1) // for our own safety
	rows := []table.Row{
		table.NewRow(table.RowData{"a": table.NewStyledCell("hello", paddedCell)}),
		table.NewRow(table.RowData{"a": table.NewStyledCell("very long text", paddedCell)}),
	}

	t := table.New(cols).WithRows(rows)
	fmt.Println(t.View())
}

Output

┏━━━━━━━━━━┓
┃         A┃
┣━━━━━━━━━━┫
┃    hello ┃
┃     very ┃
┃    long… ┃
┗━━━━━━━━━━┛

misterikkit avatar Jan 27 '23 22:01 misterikkit

Apologies for the late reply on this, it slipped through my email and I haven't checked here for a little bit. I think the StyledCell approach is probably going to be the way to go here, but it does feel a bit rough that your original attempt didn't work. I'm going to leave this open for now, but I'm not entirely sure what the obvious fix will be and there is a workaround. 🤔

Evertras avatar Feb 13 '23 14:02 Evertras

What I found so far is this line. cellStyle is built by coping rowStyle  and inherit a column.style

func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Style, borderStyle lipgloss.Style) string {
	cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)

In its implementation in the lipgloss package, we can see that margins and padding properties are skipped


func (s Style) Inherit(i Style) Style {
	s.init()

	for k, v := range i.rules {
		switch k {
		case marginTopKey, marginRightKey, marginBottomKey, marginLeftKey:
			// Margins are not inherited
			continue
		case paddingTopKey, paddingRightKey, paddingBottomKey, paddingLeftKey:
			// Padding is not inherited
			continue
		case backgroundKey:
			s.rules[k] = v

			// The margins also inherit the background color
			if !s.isSet(marginBackgroundKey) && !i.isSet(marginBackgroundKey) {
				s.rules[marginBackgroundKey] = v
			}
		}

		if _, exists := s.rules[k]; exists {
			continue
		}
		s.rules[k] = v
	}
	return s
}

With that knowledge, I created a column with left padding

	columns := []table.Column{
		table.NewColumn(columnKeyName, "Name", 10).WithStyle(
			lipgloss.NewStyle().
				Foreground(lipgloss.Color("#88f")),
		),
		table.NewColumn(columnKeyCountry, "Country", 20).WithStyle(
			lipgloss.NewStyle().
				Foreground(lipgloss.Color("#f88")).
				PaddingLeft(5),
		),
		table.NewColumn(columnKeyCurrency, "Currency", 10),

and change the renderRowColumnData a little bit

func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Style, borderStyle lipgloss.Style) string {
	cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)

	_, _, _, colPadLeft := column.style.GetPadding()
	if colPadLeft > 0 {
		cellStyle = cellStyle.PaddingLeft(colPadLeft)
	}

Which results in the desired state: image


What we can do is:

  • modify the Inherit method or create a custom one overriding all values from the incoming style - this will be impossible to make in a finite period since contributions to charm are hella slow (or they do not like me dunno)
  • append that logic into renderRowColumnData (for paddings and margins)

I have some free time now, I can go with the second solution. cc. @Evertras

prgres avatar Nov 27 '23 20:11 prgres

Okay, it may be tricky because GetPadding and GetMargins return values or 0 if a property is not set. Creating a simple if statement as I did may result in a situation when a rowStyle sets padding to >0 value and the user wants to have a column with padding ==0 and it will not be overridden.

Without access lipgloss.Style.rules it may be hard to implement simply. We can cover the override margins/paddings with a next column flag but I do not if it is a best approach here

prgres avatar Nov 27 '23 21:11 prgres

https://github.com/charmbracelet/lipgloss/pull/71 😆

prgres avatar Nov 27 '23 21:11 prgres

It came out that we cannot set paddings and margins in base style because it breaks the layout

PaddingRight(10) image

MarginRight() image

So my objects are invalid now, I am going to create a PR with that

prgres avatar Nov 27 '23 21:11 prgres

PR: https://github.com/Evertras/bubble-table/pull/160

prgres avatar Nov 27 '23 21:11 prgres