From 4bd7edfdc5489ef8e9d65d8ec94af2d43468c57d Mon Sep 17 00:00:00 2001
From: TomJGooding <101601846+TomJGooding@users.noreply.github.com>
Date: Thu, 17 Oct 2024 08:55:56 +0100
Subject: [PATCH] fix(listview): update index after pop

---
 src/textual/widgets/_list_view.py            | 25 ++++++---
 tests/listview/test_listview_remove_items.py | 56 +++++++++++++++++++-
 2 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/src/textual/widgets/_list_view.py b/src/textual/widgets/_list_view.py
index a60ddf8468..c452ef4a79 100644
--- a/src/textual/widgets/_list_view.py
+++ b/src/textual/widgets/_list_view.py
@@ -5,6 +5,7 @@
 from typing_extensions import TypeGuard
 
 from textual import _widget_navigation
+from textual.await_complete import AwaitComplete
 from textual.await_remove import AwaitRemove
 from textual.binding import Binding, BindingType
 from textual.containers import VerticalScroll
@@ -231,7 +232,7 @@ def insert(self, index: int, items: Iterable[ListItem]) -> AwaitMount:
         await_mount = self.mount(*items, before=index)
         return await_mount
 
-    def pop(self, index: Optional[int] = None) -> AwaitRemove:
+    def pop(self, index: Optional[int] = None) -> AwaitComplete:
         """Remove last ListItem from ListView or
            Remove ListItem from ListView by index
 
@@ -242,11 +243,23 @@ def pop(self, index: Optional[int] = None) -> AwaitRemove:
             An awaitable that yields control to the event loop until
                 the DOM has been updated to reflect item being removed.
         """
-        if index is None:
-            await_remove = self.query("ListItem").last().remove()
-        else:
-            await_remove = self.query("ListItem")[index].remove()
-        return await_remove
+        if len(self) == 0:
+            raise IndexError("pop from empty list")
+
+        index = index if index is not None else -1
+        if index < 0:
+            index += len(self)
+        item_to_remove = self.query("ListItem")[index]
+
+        async def do_pop():
+            await item_to_remove.remove()
+            if self.index is not None:
+                if index == self.index:
+                    self.index = self.index
+                elif index < self.index:
+                    self.index = self.index - 1
+
+        return AwaitComplete(do_pop())
 
     def remove_items(self, indices: Iterable[int]) -> AwaitRemove:
         """Remove ListItems from ListView by indices
diff --git a/tests/listview/test_listview_remove_items.py b/tests/listview/test_listview_remove_items.py
index 43501cdf19..a8f7f6cd93 100644
--- a/tests/listview/test_listview_remove_items.py
+++ b/tests/listview/test_listview_remove_items.py
@@ -1,8 +1,29 @@
+import pytest
+
 from textual.app import App, ComposeResult
-from textual.widgets import ListView, ListItem, Label
+from textual.widgets import Label, ListItem, ListView
+
+
+class EmptyListViewApp(App[None]):
+    def compose(self) -> ComposeResult:
+        yield ListView()
+
+
+async def test_listview_pop_empty_raises_index_error():
+    app = EmptyListViewApp()
+    async with app.run_test() as pilot:
+        listview = pilot.app.query_one(ListView)
+        with pytest.raises(IndexError) as excinfo:
+            listview.pop()
+        assert "pop from empty list" in str(excinfo.value)
 
 
 class ListViewApp(App[None]):
+    def __init__(self, initial_index: int | None = None):
+        super().__init__()
+        self.initial_index = initial_index
+        self.highlighted = []
+
     def compose(self) -> ComposeResult:
         yield ListView(
             ListItem(Label("0")),
@@ -14,8 +35,15 @@ def compose(self) -> ComposeResult:
             ListItem(Label("6")),
             ListItem(Label("7")),
             ListItem(Label("8")),
+            initial_index=self.initial_index,
         )
 
+    def _on_list_view_highlighted(self, message: ListView.Highlighted) -> None:
+        if message.item is None:
+            self.highlighted.append(None)
+        else:
+            self.highlighted.append(str(message.item.children[0].renderable))
+
 
 async def test_listview_remove_items() -> None:
     """Regression test for https://github.com/Textualize/textual/issues/4735"""
@@ -27,6 +55,32 @@ async def test_listview_remove_items() -> None:
         assert len(listview) == 4
 
 
+@pytest.mark.parametrize(
+    "initial_index, pop_index, expected_new_index, expected_highlighted",
+    [
+        (2, 2, 2, ["2", "3"]),  # Remove highlighted item
+        (0, 0, 0, ["0", "1"]),  # Remove first item when highlighted
+        (8, None, 7, ["8", "7"]),  # Remove last item when highlighted
+        (4, 2, 3, ["4", "4"]),  # Remove item before the highlighted index
+        (4, -2, 4, ["4"]),  # Remove item after the highlighted index
+    ],
+)
+async def test_listview_pop_updates_index_and_highlighting(
+    initial_index, pop_index, expected_new_index, expected_highlighted
+) -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/5114"""
+    app = ListViewApp(initial_index)
+    async with app.run_test() as pilot:
+        listview = pilot.app.query_one(ListView)
+
+        await listview.pop(pop_index)
+        await pilot.pause()
+
+        assert listview.index == expected_new_index
+        assert listview._nodes[expected_new_index].highlighted is True
+        assert app.highlighted == expected_highlighted
+
+
 if __name__ == "__main__":
     app = ListViewApp()
     app.run()