4b0f84f · crush · git.iko.soy

git.iko.soy/crush

chore: address review requests

- 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>

commit4b0f84f8c6a9027bd536628f68ac1cb123cff91e authorKujtim Hoxha <kujtimii.h@gmail.com> dateSat 26 Jul 2025, 12:17 (1 year ago) parentee45939

6 files changed+26−30

compare with main

unifiedsplit

internal/csync/slices.go+5−14 internal/csync/slices_test.go+4−3 internal/tui/exp/list/filterable.go+1−1 internal/tui/exp/list/filterable_group.go+2−1 internal/tui/exp/list/grouped.go+3−1 internal/tui/exp/list/list.go+11−10

internal/csync/slices.go+5−14

@@ -112,15 +112,6 @@ func (s *Slice[T]) Len() int {

112112 return len(s.inner)

113113}

114114

115−// Slice returns a copy of the underlying slice.

116−func (s *Slice[T]) Slice() []T {

117− s.mu.RLock()

118− defer s.mu.RUnlock()

119− result := make([]T, len(s.inner))

120− copy(result, s.inner)

121− return result

122−}

123−

124115// SetSlice replaces the entire slice with a new one.

125116func (s *Slice[T]) SetSlice(items []T) {

126117 s.mu.Lock()

@@ -138,10 +129,8 @@ func (s *Slice[T]) Clear() {

138129

139130// Seq returns an iterator that yields elements from the slice.

140131func (s *Slice[T]) Seq() iter.Seq[T] {

141− // Take a snapshot to avoid holding the lock during iteration

142− items := s.Slice()

143132 return func(yield func(T) bool) {

144− for _, v := range items {

133+ for _, v := range s.Seq2() {

145134 if !yield(v) {

146135 return

147136 }

@@ -151,8 +140,10 @@ func (s *Slice[T]) Seq() iter.Seq[T] {

151140

152141// Seq2 returns an iterator that yields index-value pairs from the slice.

153142func (s *Slice[T]) Seq2() iter.Seq2[int, T] {

154− // Take a snapshot to avoid holding the lock during iteration

155− items := s.Slice()

143+ s.mu.RLock()

144+ items := make([]T, len(s.inner))

145+ copy(items, s.inner)

146+ s.mu.RUnlock()

156147 return func(yield func(int, T) bool) {

157148 for i, v := range items {

158149 if !yield(i, v) {

internal/csync/slices_test.go+4−3

@@ -1,6 +1,7 @@

11package csync

33import (

4+ "slices"

45 "sync"

56 "sync/atomic"

67 "testing"

@@ -145,7 +146,7 @@ func TestSlice(t *testing.T) {

145146 assert.Equal(t, 4, s.Len())

146147

147148 expected := []int{1, 2, 4, 5}

148− actual := s.Slice()

149+ actual := slices.Collect(s.Seq())

149150 assert.Equal(t, expected, actual)

150151

151152 // Delete out of bounds

@@ -203,7 +204,7 @@ func TestSlice(t *testing.T) {

203204 s.SetSlice(newItems)

204205

205206 assert.Equal(t, 3, s.Len())

206− assert.Equal(t, newItems, s.Slice())

207+ assert.Equal(t, newItems, slices.Collect(s.Seq()))

207208

208209 // Verify it's a copy

209210 newItems[0] = 999

@@ -224,7 +225,7 @@ func TestSlice(t *testing.T) {

224225 original := []int{1, 2, 3}

225226 s := NewSliceFrom(original)

226227

227− copy := s.Slice()

228+ copy := slices.Collect(s.Seq())

228229 assert.Equal(t, original, copy)

229230

230231 // Verify it's a copy

internal/tui/exp/list/filterable.go+1−1

@@ -97,7 +97,7 @@ func NewFilterableList[T FilterableItem](items []T, opts ...filterableListOption

9797 f.list = New[T](items, f.listOptions...).(*list[T])

9898

9999 f.updateKeyMaps()

100− f.items = f.list.items.Slice()

100+ f.items = slices.Collect(f.list.items.Seq())

101101

102102 if f.inputHidden {

103103 return f

internal/tui/exp/list/filterable_group.go+2−1

@@ -2,6 +2,7 @@ package list

33import (

44 "regexp"

5+ "slices"

56 "sort"

67 "strings"

@@ -179,7 +180,7 @@ func (f *filterableGroupList[T]) inputHeight() int {

179180

180181func (f *filterableGroupList[T]) Filter(query string) tea.Cmd {

181182 var cmds []tea.Cmd

182− for _, item := range f.items.Slice() {

183+ for _, item := range slices.Collect(f.items.Seq()) {

183184 if i, ok := any(item).(layout.Focusable); ok {

184185 cmds = append(cmds, i.Blur())

185186 }

internal/tui/exp/list/grouped.go+3−1

@@ -1,6 +1,8 @@

11package list

33import (

4+ "slices"

46 tea "github.com/charmbracelet/bubbletea/v2"

57 "github.com/charmbracelet/crush/internal/csync"

68 "github.com/charmbracelet/crush/internal/tui/components/core/layout"

@@ -89,7 +91,7 @@ func (g *groupedList[T]) convertItems() {

8991func (g *groupedList[T]) SetGroups(groups []Group[T]) tea.Cmd {

9092 g.groups = groups

9193 g.convertItems()

92− return g.SetItems(g.items.Slice())

94+ return g.SetItems(slices.Collect(g.items.Seq()))

9395}

9496

9597func (g *groupedList[T]) Groups() []Group[T] {

internal/tui/exp/list/list.go+11−10

@@ -1,6 +1,7 @@

11package list

33import (

4+ "slices"

45 "strings"

67 "github.com/charmbracelet/bubbles/v2/key"

@@ -201,7 +202,7 @@ func (l *list[T]) Update(msg tea.Msg) (tea.Model, tea.Cmd) {

201202 return l, nil

202203 case anim.StepMsg:

203204 var cmds []tea.Cmd

204− for _, item := range l.items.Slice() {

205+ for _, item := range slices.Collect(l.items.Seq()) {

205206 if i, ok := any(item).(HasAnim); ok && i.Spinning() {

206207 updated, cmd := i.Update(msg)

207208 cmds = append(cmds, cmd)

@@ -288,7 +289,7 @@ func (l *list[T]) viewPosition() (int, int) {

288289

289290func (l *list[T]) recalculateItemPositions() {

290291 currentContentHeight := 0

291− for _, item := range l.items.Slice() {

292+ for _, item := range slices.Collect(l.items.Seq()) {

292293 rItem, ok := l.renderedItems.Get(item.ID())

293294 if !ok {

294295 continue

@@ -561,7 +562,7 @@ func (l *list[T]) focusSelectedItem() tea.Cmd {

561562 return nil

562563 }

563564 var cmds []tea.Cmd

564− for _, item := range l.items.Slice() {

565+ for _, item := range slices.Collect(l.items.Seq()) {

565566 if f, ok := any(item).(layout.Focusable); ok {

566567 if item.ID() == l.selectedItem && !f.IsFocused() {

567568 cmds = append(cmds, f.Focus())

@@ -580,7 +581,7 @@ func (l *list[T]) blurSelectedItem() tea.Cmd {

580581 return nil

581582 }

582583 var cmds []tea.Cmd

583− for _, item := range l.items.Slice() {

584+ for _, item := range slices.Collect(l.items.Seq()) {

584585 if f, ok := any(item).(layout.Focusable); ok {

585586 if item.ID() == l.selectedItem && f.IsFocused() {

586587 cmds = append(cmds, f.Blur())

@@ -655,7 +656,7 @@ func (l *list[T]) AppendItem(item T) tea.Cmd {

655656

656657 l.items.Append(item)

657658 l.indexMap = csync.NewMap[string, int]()

658− for inx, item := range l.items.Slice() {

659+ for inx, item := range slices.Collect(l.items.Seq()) {

659660 l.indexMap.Set(item.ID(), inx)

660661 }

661662 if l.width > 0 && l.height > 0 {

@@ -702,7 +703,7 @@ func (l *list[T]) DeleteItem(id string) tea.Cmd {

702703 }

703704 l.items.Delete(inx)

704705 l.renderedItems.Del(id)

705− for inx, item := range l.items.Slice() {

706+ for inx, item := range slices.Collect(l.items.Seq()) {

706707 l.indexMap.Set(item.ID(), inx)

707708 }

708709

@@ -767,7 +768,7 @@ func (l *list[T]) IsFocused() bool {

767768

768769// Items implements List.

769770func (l *list[T]) Items() []T {

770− return l.items.Slice()

771+ return slices.Collect(l.items.Seq())

771772}

772773

773774func (l *list[T]) incrementOffset(n int) {

@@ -822,7 +823,7 @@ func (l *list[T]) PrependItem(item T) tea.Cmd {

822823 }

823824 l.items.Prepend(item)

824825 l.indexMap = csync.NewMap[string, int]()

825− for inx, item := range l.items.Slice() {

826+ for inx, item := range slices.Collect(l.items.Seq()) {

826827 l.indexMap.Set(item.ID(), inx)

827828 }

828829 if l.width > 0 && l.height > 0 {

@@ -926,7 +927,7 @@ func (l *list[T]) SelectedItem() *T {

926927func (l *list[T]) SetItems(items []T) tea.Cmd {

927928 l.items.SetSlice(items)

928929 var cmds []tea.Cmd

929− for inx, item := range l.items.Slice() {

930+ for inx, item := range slices.Collect(l.items.Seq()) {

930931 if i, ok := any(item).(Indexable); ok {

931932 i.SetIndex(inx)

932933 }

@@ -949,7 +950,7 @@ func (l *list[T]) reset(selectedItem string) tea.Cmd {

949950 l.selectedItem = selectedItem

950951 l.indexMap = csync.NewMap[string, int]()

951952 l.renderedItems = csync.NewMap[string, renderedItem]()

952− for inx, item := range l.items.Slice() {

953+ for inx, item := range slices.Collect(l.items.Seq()) {

953954 l.indexMap.Set(item.ID(), inx)

954955 if l.width > 0 && l.height > 0 {

955956 cmds = append(cmds, item.SetSize(l.width, l.height))