chore: address review requests

Kujtim Hoxha and Crush created 

- Remove Slice() method from csync.Slice and use slices.Collect(s.Seq())
instead
- Optimize Seq2() method to avoid calling removed Slice() method
- Update Seq() to use Seq2() for better performance
- Update all references throughout the codebase to use the new pattern

💖 Generated with Crush
Co-Authored-By: Crush <crush@charm.land>

Change summary

internal/csync/slices.go                  | 19 +++++--------------
internal/csync/slices_test.go             |  7 ++++---
internal/tui/exp/list/filterable.go       |  2 +-
internal/tui/exp/list/filterable_group.go |  3 ++-
internal/tui/exp/list/grouped.go          |  4 +++-
internal/tui/exp/list/list.go             | 21 +++++++++++----------
6 files changed, 26 insertions(+), 30 deletions(-)

Detailed changes

internal/csync/slices.go 🔗 

@@ -112,15 +112,6 @@ func (s *Slice[T]) Len() int {
 	return len(s.inner)
 }
 
-// Slice returns a copy of the underlying slice.
-func (s *Slice[T]) Slice() []T {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-	result := make([]T, len(s.inner))
-	copy(result, s.inner)
-	return result
-}
-
 // SetSlice replaces the entire slice with a new one.
 func (s *Slice[T]) SetSlice(items []T) {
 	s.mu.Lock()
@@ -138,10 +129,8 @@ func (s *Slice[T]) Clear() {
 
 // Seq returns an iterator that yields elements from the slice.
 func (s *Slice[T]) Seq() iter.Seq[T] {
-	// Take a snapshot to avoid holding the lock during iteration
-	items := s.Slice()
 	return func(yield func(T) bool) {
-		for _, v := range items {
+		for _, v := range s.Seq2() {
 			if !yield(v) {
 				return
 			}
@@ -151,8 +140,10 @@ func (s *Slice[T]) Seq() iter.Seq[T] {
 
 // Seq2 returns an iterator that yields index-value pairs from the slice.
 func (s *Slice[T]) Seq2() iter.Seq2[int, T] {
-	// Take a snapshot to avoid holding the lock during iteration
-	items := s.Slice()
+	s.mu.RLock()
+	items := make([]T, len(s.inner))
+	copy(items, s.inner)
+	s.mu.RUnlock()
 	return func(yield func(int, T) bool) {
 		for i, v := range items {
 			if !yield(i, v) {

internal/csync/slices_test.go 🔗 

@@ -1,6 +1,7 @@
 package csync
 
 import (
+	"slices"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -145,7 +146,7 @@ func TestSlice(t *testing.T) {
 		assert.Equal(t, 4, s.Len())
 
 		expected := []int{1, 2, 4, 5}
-		actual := s.Slice()
+		actual := slices.Collect(s.Seq())
 		assert.Equal(t, expected, actual)
 
 		// Delete out of bounds
@@ -203,7 +204,7 @@ func TestSlice(t *testing.T) {
 		s.SetSlice(newItems)
 
 		assert.Equal(t, 3, s.Len())
-		assert.Equal(t, newItems, s.Slice())
+		assert.Equal(t, newItems, slices.Collect(s.Seq()))
 
 		// Verify it's a copy
 		newItems[0] = 999
@@ -224,7 +225,7 @@ func TestSlice(t *testing.T) {
 		original := []int{1, 2, 3}
 		s := NewSliceFrom(original)
 
-		copy := s.Slice()
+		copy := slices.Collect(s.Seq())
 		assert.Equal(t, original, copy)
 
 		// Verify it's a copy

internal/tui/exp/list/filterable.go 🔗 

@@ -97,7 +97,7 @@ func NewFilterableList[T FilterableItem](items []T, opts ...filterableListOption
 	f.list = New[T](items, f.listOptions...).(*list[T])
 
 	f.updateKeyMaps()
-	f.items = f.list.items.Slice()
+	f.items = slices.Collect(f.list.items.Seq())
 
 	if f.inputHidden {
 		return f

internal/tui/exp/list/filterable_group.go 🔗 

@@ -2,6 +2,7 @@ package list
 
 import (
 	"regexp"
+	"slices"
 	"sort"
 	"strings"
 
@@ -179,7 +180,7 @@ func (f *filterableGroupList[T]) inputHeight() int {
 
 func (f *filterableGroupList[T]) Filter(query string) tea.Cmd {
 	var cmds []tea.Cmd
-	for _, item := range f.items.Slice() {
+	for _, item := range slices.Collect(f.items.Seq()) {
 		if i, ok := any(item).(layout.Focusable); ok {
 			cmds = append(cmds, i.Blur())
 		}

internal/tui/exp/list/grouped.go 🔗 

@@ -1,6 +1,8 @@
 package list
 
 import (
+	"slices"
+
 	tea "github.com/charmbracelet/bubbletea/v2"
 	"github.com/charmbracelet/crush/internal/csync"
 	"github.com/charmbracelet/crush/internal/tui/components/core/layout"
@@ -89,7 +91,7 @@ func (g *groupedList[T]) convertItems() {
 func (g *groupedList[T]) SetGroups(groups []Group[T]) tea.Cmd {
 	g.groups = groups
 	g.convertItems()
-	return g.SetItems(g.items.Slice())
+	return g.SetItems(slices.Collect(g.items.Seq()))
 }
 
 func (g *groupedList[T]) Groups() []Group[T] {

internal/tui/exp/list/list.go 🔗 

@@ -1,6 +1,7 @@
 package list
 
 import (
+	"slices"
 	"strings"
 
 	"github.com/charmbracelet/bubbles/v2/key"
@@ -201,7 +202,7 @@ func (l *list[T]) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		return l, nil
 	case anim.StepMsg:
 		var cmds []tea.Cmd
-		for _, item := range l.items.Slice() {
+		for _, item := range slices.Collect(l.items.Seq()) {
 			if i, ok := any(item).(HasAnim); ok && i.Spinning() {
 				updated, cmd := i.Update(msg)
 				cmds = append(cmds, cmd)
@@ -288,7 +289,7 @@ func (l *list[T]) viewPosition() (int, int) {
 
 func (l *list[T]) recalculateItemPositions() {
 	currentContentHeight := 0
-	for _, item := range l.items.Slice() {
+	for _, item := range slices.Collect(l.items.Seq()) {
 		rItem, ok := l.renderedItems.Get(item.ID())
 		if !ok {
 			continue
@@ -561,7 +562,7 @@ func (l *list[T]) focusSelectedItem() tea.Cmd {
 		return nil
 	}
 	var cmds []tea.Cmd
-	for _, item := range l.items.Slice() {
+	for _, item := range slices.Collect(l.items.Seq()) {
 		if f, ok := any(item).(layout.Focusable); ok {
 			if item.ID() == l.selectedItem && !f.IsFocused() {
 				cmds = append(cmds, f.Focus())
@@ -580,7 +581,7 @@ func (l *list[T]) blurSelectedItem() tea.Cmd {
 		return nil
 	}
 	var cmds []tea.Cmd
-	for _, item := range l.items.Slice() {
+	for _, item := range slices.Collect(l.items.Seq()) {
 		if f, ok := any(item).(layout.Focusable); ok {
 			if item.ID() == l.selectedItem && f.IsFocused() {
 				cmds = append(cmds, f.Blur())
@@ -655,7 +656,7 @@ func (l *list[T]) AppendItem(item T) tea.Cmd {
 
 	l.items.Append(item)
 	l.indexMap = csync.NewMap[string, int]()
-	for inx, item := range l.items.Slice() {
+	for inx, item := range slices.Collect(l.items.Seq()) {
 		l.indexMap.Set(item.ID(), inx)
 	}
 	if l.width > 0 && l.height > 0 {
@@ -702,7 +703,7 @@ func (l *list[T]) DeleteItem(id string) tea.Cmd {
 	}
 	l.items.Delete(inx)
 	l.renderedItems.Del(id)
-	for inx, item := range l.items.Slice() {
+	for inx, item := range slices.Collect(l.items.Seq()) {
 		l.indexMap.Set(item.ID(), inx)
 	}
 
@@ -767,7 +768,7 @@ func (l *list[T]) IsFocused() bool {
 
 // Items implements List.
 func (l *list[T]) Items() []T {
-	return l.items.Slice()
+	return slices.Collect(l.items.Seq())
 }
 
 func (l *list[T]) incrementOffset(n int) {
@@ -822,7 +823,7 @@ func (l *list[T]) PrependItem(item T) tea.Cmd {
 	}
 	l.items.Prepend(item)
 	l.indexMap = csync.NewMap[string, int]()
-	for inx, item := range l.items.Slice() {
+	for inx, item := range slices.Collect(l.items.Seq()) {
 		l.indexMap.Set(item.ID(), inx)
 	}
 	if l.width > 0 && l.height > 0 {
@@ -926,7 +927,7 @@ func (l *list[T]) SelectedItem() *T {
 func (l *list[T]) SetItems(items []T) tea.Cmd {
 	l.items.SetSlice(items)
 	var cmds []tea.Cmd
-	for inx, item := range l.items.Slice() {
+	for inx, item := range slices.Collect(l.items.Seq()) {
 		if i, ok := any(item).(Indexable); ok {
 			i.SetIndex(inx)
 		}
@@ -949,7 +950,7 @@ func (l *list[T]) reset(selectedItem string) tea.Cmd {
 	l.selectedItem = selectedItem
 	l.indexMap = csync.NewMap[string, int]()
 	l.renderedItems = csync.NewMap[string, renderedItem]()
-	for inx, item := range l.items.Slice() {
+	for inx, item := range slices.Collect(l.items.Seq()) {
 		l.indexMap.Set(item.ID(), inx)
 		if l.width > 0 && l.height > 0 {
 			cmds = append(cmds, item.SetSize(l.width, l.height))