Skip to content

Commit

Permalink
Feature: select rows programatically (#84)
Browse files Browse the repository at this point in the history
* Add Row.Selected function

This would allow to select rows programatically. See #79 for
additional information.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Add test for selecting rows programatically

This will fail, as the current implementation doesn't really work,
however, this is useful to drive the right implementation.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Compute selected rows only when needed

Previously the selected rows were cached in the model whenever the
toggle mark key event was received. Moving that computation instead to
the Model.SelectedRows function fixes this by only computing these
rows when needed.

I've also identified some optimizations that will be addressed in
upcoming commits.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Add benchmark for Model.SelectedRows

This will be useful for future optimizations.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Reduce allocations when computing selected rows

By allocating the returned slice with an initial capacity but length 0
we reduce the number of allocations by roughly 80%. I've run the
benchmark using the following command:

    go test ./table/ -bench=BenchmarkSelectedRows -benchtime=100000x -count=10

I've run the benchmark with the previous commit and with this one, and
these are the results using benchstat for comparison:

name             old time/op    new time/op    delta
SelectedRows-12    18.3µs ± 1%    17.0µs ± 1%   -7.26%  (p=0.000 n=8+10)

name             old alloc/op   new alloc/op   delta
SelectedRows-12    84.2kB ± 0%    81.9kB ± 0%   -2.70%  (p=0.002 n=8+10)

name             old allocs/op  new allocs/op  delta
SelectedRows-12      11.0 ± 0%       2.0 ± 0%  -81.82%  (p=0.000 n=10+10)

As you can see there's also some small improvement in the overall
performance of the call.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Remove unnecessary field in Model

Given that the selected rows are now computed on the fly this field
isn't needed anymore.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Update table/row.go

Co-authored-by: Brandon Fulljames <bfullj@gmail.com>

* Refactor Model.SelectedRows tests

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Add newline before return

…and make the linter happy.

Signed-off-by: Leandro López (inkel) <inkel.ar@gmail.com>

* Revert "Reduce allocations when computing selected rows"

This reverts commit 396041d.

Co-authored-by: Brandon Fulljames <bfullj@gmail.com>
  • Loading branch information
inkel and Evertras authored Jun 7, 2022
1 parent 0d4a851 commit 7a9a691
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 11 deletions.
1 change: 0 additions & 1 deletion table/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type Model struct {
focused bool
keyMap KeyMap
selectableRows bool
selectedRows []Row
rowCursorIndex int

// Styles
Expand Down
11 changes: 9 additions & 2 deletions table/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func (m Model) HeaderStyle(style lipgloss.Style) Model {
// WithRows sets the rows to show as data in the table.
func (m Model) WithRows(rows []Row) Model {
m.rows = rows
m.selectedRows = nil

if m.pageSize != 0 {
maxPage := m.MaxPages()
Expand Down Expand Up @@ -92,7 +91,15 @@ func (m Model) HighlightedRow() Row {

// SelectedRows returns all rows that have been set as selected by the user.
func (m Model) SelectedRows() []Row {
return m.selectedRows
selectedRows := []Row{}

for _, row := range m.GetVisibleRows() {
if row.selected {
selectedRows = append(selectedRows, row)
}
}

return selectedRows
}

// HighlightStyle sets a custom style to use when the row is being highlighted
Expand Down
87 changes: 87 additions & 0 deletions table/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,90 @@ func TestPageOptions(t *testing.T) {
assert.Greater(t, len(model.renderFooter(10)), 10)
assert.Contains(t, model.renderFooter(10), "6/6")
}

func TestSelectRowsProgramatically(t *testing.T) {
const col = "id"

tests := map[string]struct {
rows []Row
selectedIds []int
}{
"no rows selected": {
[]Row{
NewRow(RowData{col: 1}),
NewRow(RowData{col: 2}),
NewRow(RowData{col: 3}),
},
[]int{},
},

"all rows selected": {
[]Row{
NewRow(RowData{col: 1}).Selected(true),
NewRow(RowData{col: 2}).Selected(true),
NewRow(RowData{col: 3}).Selected(true),
},
[]int{1, 2, 3},
},

"first row selected": {
[]Row{
NewRow(RowData{col: 1}).Selected(true),
NewRow(RowData{col: 2}),
NewRow(RowData{col: 3}),
},
[]int{1},
},

"last row selected": {
[]Row{
NewRow(RowData{col: 1}),
NewRow(RowData{col: 2}),
NewRow(RowData{col: 3}).Selected(true),
},
[]int{3},
},
}

model := New([]Column{
NewColumn(col, col, 1),
})

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
sel := model.WithRows(tt.rows).SelectedRows()

assert.Equal(t, len(tt.selectedIds), len(sel))
for i, id := range tt.selectedIds {
assert.Equal(t, id, sel[i].Data[col], "expecting row %d to have same %s column value", i)
}
})
}
}

func BenchmarkSelectedRows(b *testing.B) {
const N = 1000

b.ReportAllocs()

rows := make([]Row, 0, N)
for i := 0; i < N; i++ {
rows = append(rows, NewRow(RowData{"row": i}).Selected(i%2 == 0))
}

model := New([]Column{
NewColumn("row", "Row", 4),
}).WithRows(rows)

var sel []Row

b.ResetTimer()

for i := 0; i < b.N; i++ {
sel = model.SelectedRows()
}

Rows = sel
}

var Rows []Row
7 changes: 7 additions & 0 deletions table/row.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,10 @@ func (m Model) renderRow(rowIndex int, last bool) string {

return lipgloss.JoinHorizontal(lipgloss.Bottom, columnStrings...)
}

// Selected sets whether the row is selected or not.
func (r Row) Selected(selected bool) Row {
r.selected = selected

return r
}
8 changes: 0 additions & 8 deletions table/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ func (m *Model) toggleSelect() {
rows[m.rowCursorIndex].selected = !rows[m.rowCursorIndex].selected

m.rows = rows

m.selectedRows = []Row{}

for _, row := range m.GetVisibleRows() {
if row.selected {
m.selectedRows = append(m.selectedRows, row)
}
}
}

func (m Model) updateFilterTextInput(msg tea.Msg) (Model, tea.Cmd) {
Expand Down

0 comments on commit 7a9a691

Please sign in to comment.