From 523f55f9708c563c75d7e3327a860af31fc94130 Mon Sep 17 00:00:00 2001 From: Charlie Tonneslan Date: Tue, 12 May 2026 09:20:30 -0400 Subject: [PATCH] list: fix RemoveItem using wrong index when a filter is active When a filter is active, the filtered list and the global list have different item orderings. RemoveItem was using the caller's index for both, which meant it removed the wrong item from the global list. For example, with items [foo, bar, baz] and a filter showing [bar, baz], calling RemoveItem(0) would remove foo (global index 0) instead of bar (filtered index 0, global index 1). Fix: when filtering is active, use the global index stored in filteredItems[index].index to remove the correct item from m.items. Also decrement the stored global indices of any remaining filtered items that came after the removed item, keeping them consistent. Fixes #632. --- list/list.go | 18 ++++++++++++++++- list/list_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/list/list.go b/list/list.go index da73d0fd6..4d176219e 100644 --- a/list/list.go +++ b/list/list.go @@ -438,13 +438,29 @@ func (m *Model) InsertItem(index int, item Item) tea.Cmd { // RemoveItem removes an item at the given index. If the index is out of bounds // this will be a no-op. O(n) complexity, which probably won't matter in the // case of a TUI. +// +// When a filter is active, index refers to the position in the filtered list. +// The corresponding item is removed from both the filtered list and the +// underlying unfiltered list. func (m *Model) RemoveItem(index int) { - m.items = removeItemFromSlice(m.items, index) if m.filterState != Unfiltered { + if index >= len(m.filteredItems) { + return // noop + } + globalIndex := m.filteredItems[index].index + m.items = removeItemFromSlice(m.items, globalIndex) m.filteredItems = removeFilterMatchFromSlice(m.filteredItems, index) + // Keep stored global indices consistent after the removal. + for i := range m.filteredItems { + if m.filteredItems[i].index > globalIndex { + m.filteredItems[i].index-- + } + } if len(m.filteredItems) == 0 { m.resetFiltering() } + } else { + m.items = removeItemFromSlice(m.items, index) } m.updatePagination() } diff --git a/list/list_test.go b/list/list_test.go index 13be41af9..69988a6a4 100644 --- a/list/list_test.go +++ b/list/list_test.go @@ -135,3 +135,54 @@ func TestSetFilterState(t *testing.T) { t.Fatalf("Error: expected view to contain '%s'", expected) } } + +func TestRemoveItemWhileFiltered(t *testing.T) { + // items: foo(0), bar(1), baz(2) + // filter "ba" → filtered: bar(global 1), baz(global 2) + items := []Item{item("foo"), item("bar"), item("baz")} + list := New(items, itemDelegate{}, 10, 10) + list.SetFilterText("ba") + + // Sanity: filtered list is [bar, baz] + if got := list.VisibleItems(); !reflect.DeepEqual(got, []Item{item("bar"), item("baz")}) { + t.Fatalf("setup: expected filtered [bar baz], got %v", got) + } + + // RemoveItem(0) should remove "bar" (filtered index 0 → global index 1) + list.RemoveItem(0) + + // Filtered list should now be [baz] + if got := list.VisibleItems(); !reflect.DeepEqual(got, []Item{item("baz")}) { + t.Fatalf("filtered view after remove: expected [baz], got %v", got) + } + + // Reset filter — global list should be [foo, baz] + list.ResetFilter() + if got := list.Items(); !reflect.DeepEqual(got, []Item{item("foo"), item("baz")}) { + t.Fatalf("global list after remove+reset: expected [foo baz], got %v", got) + } +} + +func TestRemoveItemWhileFilteredUpdatesIndices(t *testing.T) { + // items: foo(0), bar(1), baz(2) + // filter "ba" → filtered: bar(global 1), baz(global 2) + // Remove bar (filtered index 0, global index 1). + // After removal baz should still be reachable (global index now 1). + items := []Item{item("foo"), item("bar"), item("baz")} + list := New(items, itemDelegate{}, 10, 10) + list.SetFilterText("ba") + list.RemoveItem(0) + + // baz is the only remaining filtered item; remove it too + list.RemoveItem(0) + + // filter should have been reset (no filtered items left) + if list.FilterState() != Unfiltered { + t.Fatalf("expected filter to reset when no items remain, got state %v", list.FilterState()) + } + + // Only "foo" should remain + if got := list.Items(); !reflect.DeepEqual(got, []Item{item("foo")}) { + t.Fatalf("global list after removing all filtered items: expected [foo], got %v", got) + } +}