From 1a686bd0ff6a0bab6028f1d4c74a012b1228b315 Mon Sep 17 00:00:00 2001 From: phg Date: Fri, 4 Sep 2026 07:32:16 +0200 Subject: [PATCH] Make sorting view-only (#11) --- README.md | 2 - src/hosts/tui/table_handler.py | 26 +++---- tests/test_main.py | 130 ++++++++++++++++++++++++++++----- 3 files changed, 124 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 22b7004..17c8cdc 100644 --- a/README.md +++ b/README.md @@ -75,8 +75,6 @@ sorting, DNS refresh, movement, undo, and redo. ## Safety and known limitations - Reloading discards unsaved in-memory state. -- Sorting currently reorders the in-memory model; a later save can write that - order to `/etc/hosts`. Treat sorting as unsafe before another mutation. - A failed save can leave the interface changed while `/etc/hosts` remains unchanged. - Pre-edit Backups are not listed or restored by the TUI and have no retention diff --git a/src/hosts/tui/table_handler.py b/src/hosts/tui/table_handler.py index e7b0683..b3c2d5c 100644 --- a/src/hosts/tui/table_handler.py +++ b/src/hosts/tui/table_handler.py @@ -9,7 +9,7 @@ from rich.text import Text from textual.widgets import DataTable from typing import List -from ..core.models import HostEntry +from ..core.models import HostEntry, HostsFile class TableHandler: @@ -45,7 +45,18 @@ class TableHandler: # Fallback to legacy search filtering for backward compatibility filtered_entries = self._apply_legacy_search_filter(all_entries) - return filtered_entries + return self._get_sorted_display_entries(filtered_entries) + + def _get_sorted_display_entries(self, entries: List[HostEntry]) -> List[HostEntry]: + """Return entries in the current display order without mutating the Hosts File.""" + display_file = HostsFile(entries=list(entries)) + + if self.app.sort_column == "ip": + display_file.sort_by_ip(ascending=self.app.sort_ascending) + elif self.app.sort_column == "hostname": + display_file.sort_by_hostname(ascending=self.app.sort_ascending) + + return display_file.entries def _apply_legacy_search_filter(self, entries: List[HostEntry]) -> List[HostEntry]: """Apply legacy search filter for backward compatibility.""" @@ -238,11 +249,6 @@ class TableHandler: ): current_entry = self.app.hosts_file.entries[self.app.selected_entry_index] - # Sort the entries - self.app.hosts_file.entries.sort( - key=lambda entry: entry.ip_address, reverse=not self.app.sort_ascending - ) - # Refresh the table and restore cursor position self.populate_entries_table() self.restore_cursor_position(current_entry) @@ -300,12 +306,6 @@ class TableHandler: ): current_entry = self.app.hosts_file.entries[self.app.selected_entry_index] - # Sort the entries - self.app.hosts_file.entries.sort( - key=lambda entry: entry.hostnames[0].lower() if entry.hostnames else "", - reverse=not self.app.sort_ascending, - ) - # Refresh the table and restore cursor position self.populate_entries_table() self.restore_cursor_position(current_entry) diff --git a/tests/test_main.py b/tests/test_main.py index 2843fcb..4c95e5d 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -12,6 +12,7 @@ from hosts.tui.app import HostsManagerApp from hosts.core.models import HostEntry, HostsFile from hosts.core.parser import HostsParser from hosts.core.config import Config +from hosts.core.manager import HostsManager class TestHostsManagerApp: @@ -394,8 +395,8 @@ class TestHostsManagerApp: args = app.push_screen.call_args[0] assert len(args) >= 1 # ConfigModal instance - def test_action_sort_by_ip_ascending(self): - """Test sorting by IP address in ascending order.""" + def test_action_sort_by_ip_orders_display_without_changing_file_order(self): + """Sorting by IP changes the display order without changing file order.""" mock_parser = Mock(spec=HostsParser) mock_config = Mock(spec=Config) @@ -405,17 +406,26 @@ class TestHostsManagerApp: ): app = HostsManagerApp() - # Add test entries in reverse order + mock_config.should_show_default_entries.return_value = True + + # Keep default entries deliberately out of their fixed display order. app.hosts_file = HostsFile() app.hosts_file.add_entry( HostEntry(ip_address="192.168.1.1", hostnames=["router"]) ) app.hosts_file.add_entry( - HostEntry(ip_address="127.0.0.1", hostnames=["localhost"]) + HostEntry(ip_address="::1", hostnames=["localhost"]) ) app.hosts_file.add_entry( HostEntry(ip_address="10.0.0.1", hostnames=["test"]) ) + app.hosts_file.add_entry( + HostEntry(ip_address="127.0.0.1", hostnames=["localhost"]) + ) + app.hosts_file.add_entry( + HostEntry(ip_address="255.255.255.255", hostnames=["broadcasthost"]) + ) + original_order = [entry.ip_address for entry in app.hosts_file.entries] # Mock the table_handler methods to avoid UI queries app.table_handler.populate_entries_table = Mock() @@ -424,17 +434,30 @@ class TestHostsManagerApp: app.action_sort_by_ip() - # Check that entries are sorted by IP address - assert app.hosts_file.entries[0].ip_address == "10.0.0.1" # Sorted by IP - assert app.hosts_file.entries[1].ip_address == "127.0.0.1" - assert app.hosts_file.entries[2].ip_address == "192.168.1.1" + assert [ + entry.ip_address for entry in app.table_handler.get_visible_entries() + ] == ["127.0.0.1", "255.255.255.255", "::1", "10.0.0.1", "192.168.1.1"] + assert [ + entry.ip_address for entry in app.hosts_file.entries + ] == original_order assert app.sort_column == "ip" assert app.sort_ascending is True app.table_handler.populate_entries_table.assert_called_once() - def test_action_sort_by_hostname_ascending(self): - """Test sorting by hostname in ascending order.""" + app.action_sort_by_ip() + + assert [ + entry.ip_address for entry in app.table_handler.get_visible_entries() + ] == ["127.0.0.1", "255.255.255.255", "::1", "192.168.1.1", "10.0.0.1"] + assert [ + entry.ip_address for entry in app.hosts_file.entries + ] == original_order + + def test_action_sort_by_hostname_keeps_default_entries_first_in_both_directions( + self, + ): + """Sorting by hostname leaves Default Entries first when toggled.""" mock_parser = Mock(spec=HostsParser) mock_config = Mock(spec=Config) @@ -444,17 +467,25 @@ class TestHostsManagerApp: ): app = HostsManagerApp() - # Add test entries in reverse alphabetical order + mock_config.should_show_default_entries.return_value = True + app.hosts_file = HostsFile() - app.hosts_file.add_entry( - HostEntry(ip_address="127.0.0.1", hostnames=["zebra"]) - ) app.hosts_file.add_entry( HostEntry(ip_address="192.168.1.1", hostnames=["alpha"]) ) app.hosts_file.add_entry( - HostEntry(ip_address="10.0.0.1", hostnames=["beta"]) + HostEntry(ip_address="::1", hostnames=["localhost"]) ) + app.hosts_file.add_entry( + HostEntry(ip_address="10.0.0.1", hostnames=["zebra"]) + ) + app.hosts_file.add_entry( + HostEntry(ip_address="127.0.0.1", hostnames=["localhost"]) + ) + app.hosts_file.add_entry( + HostEntry(ip_address="255.255.255.255", hostnames=["broadcasthost"]) + ) + original_order = [entry.hostnames[0] for entry in app.hosts_file.entries] # Mock the table_handler methods to avoid UI queries app.table_handler.populate_entries_table = Mock() @@ -463,15 +494,76 @@ class TestHostsManagerApp: app.action_sort_by_hostname() - # Check that entries are sorted alphabetically - assert app.hosts_file.entries[0].hostnames[0] == "alpha" - assert app.hosts_file.entries[1].hostnames[0] == "beta" - assert app.hosts_file.entries[2].hostnames[0] == "zebra" + assert [ + entry.ip_address for entry in app.table_handler.get_visible_entries() + ] == ["127.0.0.1", "255.255.255.255", "::1", "192.168.1.1", "10.0.0.1"] assert app.sort_column == "hostname" assert app.sort_ascending is True app.table_handler.populate_entries_table.assert_called_once() + app.action_sort_by_hostname() + + assert [ + entry.ip_address for entry in app.table_handler.get_visible_entries() + ] == ["127.0.0.1", "255.255.255.255", "::1", "10.0.0.1", "192.168.1.1"] + assert [ + entry.hostnames[0] for entry in app.hosts_file.entries + ] == original_order + + def test_sorting_in_read_only_mode_preserves_file_order_when_mutation_autosaves( + self, + ): + """A privileged mutation saves file order after sorting in Read-only Mode.""" + mock_parser = Mock(spec=HostsParser) + mock_config = Mock(spec=Config) + mock_config.should_show_default_entries.return_value = True + + with ( + patch("hosts.tui.app.HostsParser", return_value=mock_parser), + patch("hosts.tui.app.Config", return_value=mock_config), + ): + app = HostsManagerApp() + app.hosts_file = HostsFile( + entries=[ + HostEntry(ip_address="192.168.1.1", hostnames=["router"]), + HostEntry(ip_address="10.0.0.1", hostnames=["test"]), + ] + ) + app.table_handler.populate_entries_table = Mock() + app.table_handler.restore_cursor_position = Mock() + app.details_handler.update_entry_details = Mock() + app.set_timer = Mock() + app.update_status = Mock() + + serialized_files = [] + app.manager = Mock(spec=HostsManager) + app.manager.enter_edit_mode.return_value = (True, "Edit mode enabled") + + def toggle_entry(hosts_file, index): + hosts_file.toggle_entry(index) + return Mock(success=True, message="Entry toggled") + + def save_hosts_file(hosts_file): + serialized_files.append(HostsParser().serialize(hosts_file)) + return True, "Hosts file saved successfully" + + app.manager.execute_toggle_command.side_effect = toggle_entry + app.manager.save_hosts_file.side_effect = save_hosts_file + + app.action_sort_by_ip() + assert app.edit_mode is False + + app.action_toggle_edit_mode() + app.action_toggle_entry() + + assert app.edit_mode is True + assert len(serialized_files) == 1 + + assert serialized_files[0].index("router") < serialized_files[0].index( + "test" + ) + def test_data_table_row_highlighted_event(self): """Test DataTable row highlighting event handling.""" mock_parser = Mock(spec=HostsParser)