Repository: Textualize/textual

Issue:
Calling `remove_items` or `pop` on a ListView does not update index or highlighting
It seems that calling `pop` (or `remove_items`, though that's not what I'm using for my use case) does not correctly update highlighting (presumably because calling `pop` does not update the index).

A small reproduction is as follows:

```python
from textual.app import App, ComposeResult
from textual.widgets import ListItem, ListView, Footer


class TestApp(App):
    BINDINGS = [
        ('a', 'add', 'Adds a blank item'),
        ('d', 'delete_highlighted', 'Deletes the highlighted cell'),
    ]

    _listview: ListView

    def compose(self) -> ComposeResult:
        self._listview = ListView(ListItem())

        yield Footer()
        yield self._listview

    def action_add(self):
        self._listview.append(ListItem())

    def action_delete_highlighted(self):
        index = self._listview.index
        self._listview.pop(index)


if __name__ == '__main__':
    TestApp().run()
```

Spawning a few items and trying to delete them does not set any highlighting, only manually updating the index will do that.


# Textual Diagnostics

## Versions

| Name    | Value  |
|---------|--------|
| Textual | 0.83.0 |
| Rich    | 13.9.2 |

## Python

| Name           | Value                                       |
|----------------|---------------------------------------------|
| Version        | 3.12.1                                      |
| Implementation | CPython                                     |
| Compiler       | Clang 17.0.6                                |
| Executable     | /home/king/Projects/private/.venv/bin/python |

## Operating System

| Name    | Value                                                |
|---------|------------------------------------------------------|
| System  | Linux                                                |
| Release | 6.10.11-amd64                                        |
| Version | #1 SMP PREEMPT_DYNAMIC Debian 6.10.11-1 (2024-09-22) |

## Terminal

| Name                 | Value          |
|----------------------|----------------|
| Terminal Application | *Unknown*      |
| TERM                 | xterm-256color |
| COLORTERM            | truecolor      |
| FORCE_COLOR          | *Not set*      |
| NO_COLOR             | *Not set*      |

## Rich Console options

| Name           | Value                |
|----------------|----------------------|
| size           | width=223, height=76 |
| legacy_windows | False                |
| min_width      | 1                    |
| max_width      | 223                  |
| is_terminal    | True                 |
| encoding       | utf-8                |
| max_height     | 76                   |
| justify        | None                 |
| overflow       | None                 |
| no_wrap        | False                |
| highlight      | None                 |
| markup         | None                 |
| height         | None                 |


Discussion:
TomJGooding: I've now also updated the `remove_items` method, so hopefully I was on the right track!
TomJGooding: This now also reinstates `always_update` to the `index` reactive that was removed in recent changes. When items are removed, the `index` may not change but the highlighted item should be updated.
willmcgugan: I'd recommend against the `always_update`. Since it calls watchers you can get unintended side effects, such as re-sending messages.

Better to factor out the specific functionality you need and call it directly.

TBH I regret adding `always_update`.  I know we've recommended it in the past, but this will rightly confuse anything not intimately familiar with reactives:

```python
self.index = self.index
```
willmcgugan: Hi Tom, is this ready to go?
darrenburns: @willmcgugan Was going to merge, but it's blocked until you approve.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c2d544bd..6de8ba68f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,13 +5,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-### Unreleased
+## Unreleased
 
 ### Fixed
 
 - Fixed infinite loop in `Widget.anchor` https://github.com/Textualize/textual/pull/5290
 - Restores the ability to supply console markup to command list https://github.com/Textualize/textual/pull/5294
 - Fixed delayed App Resize event https://github.com/Textualize/textual/pull/5296
+- Fixed `ListView` not updating its index or highlighting after removing items https://github.com/Textualize/textual/issues/5114
+
+### Changed
+
+- `ListView.pop` now returns `AwaitComplete` rather than `AwaitRemove` https://github.com/Textualize/textual/pull/5135
+- `ListView.remove_items` now returns `AwaitComplete` rather than `AwaitRemove` https://github.com/Textualize/textual/pull/5135
 - Fixed ListView focus styling rule being too broad https://github.com/Textualize/textual/pull/5304
 - Fixed issue with auto-generated tab IDs https://github.com/Textualize/textual/pull/5298
 
@@ -46,6 +52,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed a glitch with the scrollbar that occurs when you hold `a` to add stopwatches in the tutorial app https://github.com/Textualize/textual/pull/5257
 
+
 ## [0.86.2] - 2024-11-18
 
 ### Fixed
diff --git a/src/textual/widgets/_list_view.py b/src/textual/widgets/_list_view.py
index a5879b7c4..b8c36bdd4 100644
--- a/src/textual/widgets/_list_view.py
+++ b/src/textual/widgets/_list_view.py
@@ -5,6 +5,7 @@ from typing import ClassVar, Iterable, Optional
 from typing_extensions import TypeGuard
 
 from textual._loop import loop_from_index
+from textual.await_complete import AwaitComplete
 from textual.await_remove import AwaitRemove
 from textual.binding import Binding, BindingType
 from textual.containers import VerticalScroll
@@ -280,7 +281,7 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
         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
 
@@ -291,13 +292,31 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
             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
-
-    def remove_items(self, indices: Iterable[int]) -> AwaitRemove:
+        if len(self) == 0:
+            raise IndexError("pop from empty list")
+
+        index = index if index is not None else -1
+        item_to_remove = self.query("ListItem")[index]
+        normalized_index = index if index >= 0 else index + len(self)
+
+        async def do_pop() -> None:
+            """Remove the item and update the highlighted index."""
+            await item_to_remove.remove()
+            if self.index is not None:
+                if normalized_index < self.index:
+                    self.index -= 1
+                elif normalized_index == self.index:
+                    old_index = self.index
+                    # Force a re-validation of the index
+                    self.index = self.index
+                    # If the index hasn't changed, the watcher won't be called
+                    # but we need to update the highlighted item
+                    if old_index == self.index:
+                        self.watch_index(old_index, self.index)
+
+        return AwaitComplete(do_pop())
+
+    def remove_items(self, indices: Iterable[int]) -> AwaitComplete:
         """Remove ListItems from ListView by indices
 
         Args:
@@ -308,8 +327,29 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
         """
         items = self.query("ListItem")
         items_to_remove = [items[index] for index in indices]
-        await_remove = self.remove_children(items_to_remove)
-        return await_remove
+        normalized_indices = set(
+            index if index >= 0 else index + len(self) for index in indices
+        )
+
+        async def do_remove_items() -> None:
+            """Remove the items and update the highlighted index."""
+            await self.remove_children(items_to_remove)
+            if self.index is not None:
+                removed_before_highlighted = sum(
+                    1 for index in normalized_indices if index < self.index
+                )
+                if removed_before_highlighted:
+                    self.index -= removed_before_highlighted
+                elif self.index in normalized_indices:
+                    old_index = self.index
+                    # Force a re-validation of the index
+                    self.index = self.index
+                    # If the index hasn't changed, the watcher won't be called
+                    # but we need to update the highlighted item
+                    if old_index == self.index:
+                        self.watch_index(old_index, self.index)
+
+        return AwaitComplete(do_remove_items())
 
     def action_select_cursor(self) -> None:
         """Select the current item in the list."""


Test patch:
diff --git a/tests/listview/test_listview_remove_items.py b/tests/listview/test_listview_remove_items.py
index 43501cdf1..48a03633d 100644
--- a/tests/listview/test_listview_remove_items.py
+++ b/tests/listview/test_listview_remove_items.py
@@ -1,8 +1,31 @@
+from __future__ import annotations
+
+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 +37,15 @@ class ListViewApp(App[None]):
             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 +57,60 @@ 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
+
+
+@pytest.mark.parametrize(
+    "initial_index, remove_indices, expected_new_index, expected_highlighted",
+    [
+        (2, [2], 2, ["2", "3"]),  # Remove highlighted item
+        (0, [0], 0, ["0", "1"]),  # Remove first item when highlighted
+        (8, [-1], 7, ["8", "7"]),  # Remove last item when highlighted
+        (4, [2, 1], 2, ["4", "4"]),  # Remove items before the highlighted index
+        (4, [-2, 5], 4, ["4"]),  # Remove items after the highlighted index
+        (4, range(0, 9), None, ["4", None]),  # Remove all items
+    ],
+)
+async def test_listview_remove_items_updates_index_and_highlighting(
+    initial_index, remove_indices, 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.remove_items(remove_indices)
+        await pilot.pause()
+
+        assert listview.index == expected_new_index
+        if expected_new_index is not None:
+            assert listview._nodes[expected_new_index].highlighted is True
+        assert app.highlighted == expected_highlighted
+
+
 if __name__ == "__main__":
     app = ListViewApp()
     app.run()