Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(table): index out of range with varying row widths in StyleFunc #487

Closed
bashbunni opened this issue Mar 10, 2025 · 5 comments
Closed
Labels
bug Something isn't working

Comments

@bashbunni
Copy link
Member

if you do a conditional check in StyleFunc and the table rows have varying lengths, you'll get an index out of range error. This also happens if you don't do a check for row == HeaderRow.

func TestTableWithStyleFuncLinks(t *testing.T) {
	headers := []string{"Package", "Version", "Link"}
	data := [][]string{
		{"sourcegit", "0.19", "https://aur.archlinux.org/packages/sourcegit-bin", ""},
                {},
		{"Welcome", "いらっしゃいませ", "مرحباً"},
		{"Goodbye", "さようなら", "مع السلامة"},
	}
	table := New().
		Headers(headers...).
		Rows(data...).
		StyleFunc(func(row, col int) lipgloss.Style {
			if row == HeaderRow {
				return lipgloss.NewStyle()
			}
			if strings.Contains(data[row][col], "https://") {
				return lipgloss.NewStyle().Foreground(lipgloss.Color("#31BB71"))
			}
			return lipgloss.NewStyle()
		}).
		Width(60).
		Wrap(true)
		golden.RequireEqual(t, []byte(table.String()))
}
@andreynering
Copy link
Member

I'm not sure how we could help the user to avoid panics here. In theory, this is something the user has to address:

if i < len(data) && j < len(data[i]) {
        // do something
} else {
        // do nothing, index would overflow
}

To improve, we might have to consider a whole different function signature to styleFunc, but then it'd be a breaking change.

@bashbunni
Copy link
Member Author

bashbunni commented Mar 18, 2025

I wonder if we should maybe fill data with empty content to have an equal # of columns for all rows. We render empty cells anyway in the table.

e.g.

	data := [][]string{
		{"sourcegit", "0.19", "https://aur.archlinux.org/packages/sourcegit-bin", ""},
                {},
		{"Welcome", "いらっしゃいませ", "مرحباً"},
		{"Goodbye", "さようなら", "مع السلامة"},
	}

becomes

	data := [][]string{
		{"sourcegit", "0.19", "https://aur.archlinux.org/packages/sourcegit-bin", ""},
                {"", "", "", ""},
		{"Welcome", "いらっしゃいませ", "مرحباً", ""},
		{"Goodbye", "さようなら", "مع السلامة", ""},
	}

💭

@andreynering
Copy link
Member

We would be mutating the data the user is providing. I don't think it's a good idea.

@bashbunni
Copy link
Member Author

@andreynering ah true! We can keep it in the back of our minds then

@bashbunni
Copy link
Member Author

I'm going to make this a discussion until we decide on a solution. Will keep it labelled as a bug though.

@bashbunni bashbunni added the bug Something isn't working label Mar 18, 2025
@charmbracelet charmbracelet locked and limited conversation to collaborators Mar 18, 2025
@bashbunni bashbunni converted this issue into discussion #499 Mar 18, 2025

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants