From 89b37239b2c3175049f78ab7102116c7e11c9b29 Mon Sep 17 00:00:00 2001 From: phg Date: Thu, 3 Sep 2026 19:39:12 +0200 Subject: [PATCH] Fix #7: persist undo and redo results immediately --- README.md | 2 - docs/user-guide.md | 35 +++++++-------- src/hosts/tui/app.py | 32 ++++++++++++++ tests/test_main.py | 102 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 50c4444..22b7004 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,6 @@ sorting, DNS refresh, movement, undo, and redo. ## Safety and known limitations -- Most successful mutations save immediately, but undo and redo currently - change only the in-memory state until another save occurs. - 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. diff --git a/docs/user-guide.md b/docs/user-guide.md index 1e33f6b..e98a92b 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -6,7 +6,7 @@ authorization before it can change `/etc/hosts`. > [!WARNING] > `hosts` is alpha software. Read [Persistence and recovery](#persistence-and-recovery) -> before entering Privileged Mode. In particular, undo, redo, sorting, and save +> before entering Privileged Mode. In particular, sorting and save > failures can currently leave the interface and `/etc/hosts` out of sync. ## Before you begin @@ -74,7 +74,7 @@ additional hostnames are Aliases. Default Entries represent protected baseline localhost or broadcasthost mappings. They cannot be changed, deleted, activated, deactivated, or moved. -Most successful mutations are saved to `/etc/hosts` immediately. Read the +Successful mutations are saved to `/etc/hosts` immediately. Read the status message after every action. A message that reports a save failure means the interface may have changed while the system file did not. @@ -99,9 +99,8 @@ stored mapping. ## Verify what is on disk -The details shown by the application normally match the last successful save, -except for the known persistence cases below. To discard in-memory state and -read `/etc/hosts` again, press `Ctrl+R`. +The details shown by the application normally match the last successful save. +To discard in-memory state and read `/etc/hosts` again, press `Ctrl+R`. You can also inspect the file from another terminal: @@ -117,19 +116,15 @@ a management header while retaining the modeled Host Entries and comments. ### What saves immediately -Adding, editing, deleting, moving, activating, deactivating, and successfully -refreshing DNS Entries normally save immediately. `Ctrl+S` explicitly saves the -entire current in-memory Hosts File. +Adding, editing, deleting, moving, activating, deactivating, undoing, redoing, +and successfully refreshing DNS Entries save immediately. `Ctrl+S` explicitly +saves the entire current in-memory Hosts File. -### What does not save immediately +If saving an undo or redo result fails, the application restores the prior +in-memory state and undo/redo history. Reload with `Ctrl+R` to verify the +on-disk state before attempting another change. -Undo and redo currently update the in-memory state without saving it. After -`Ctrl+Z` or `Ctrl+Y`, use `Ctrl+S` if you want `/etc/hosts` to match the display. -Reloading or quitting discards that unsaved in-memory result. Leaving -Privileged Mode does not save it: the result remains displayed, but its -undo/redo history is cleared and `/etc/hosts` still differs from the display. - -A save failure can also leave the interface changed while the file on disk is +Other save failures can leave the interface changed while the file on disk is unchanged. Reload with `Ctrl+R` to return the interface to the on-disk state before attempting another change. @@ -206,9 +201,9 @@ specific failure. ### The display differs from `/etc/hosts` -This can occur after undo, redo, a failed save, or sorting followed by other -actions. Press `Ctrl+R` to discard in-memory state and reload the file. If an -unwanted change reached disk, follow the manual restoration procedure above. +This can occur after a failed save or sorting followed by other actions. Press +`Ctrl+R` to discard in-memory state and reload the file. If an unwanted change +reached disk, follow the manual restoration procedure above. ### DNS resolution fails @@ -245,7 +240,7 @@ continually retrying it. | `Shift+Up` / `Shift+Down` | Move the selected Host Entry | | `r` | Refresh the selected DNS Entry | | `Shift+R` | Refresh all DNS Entries | -| `Ctrl+Z` / `Ctrl+Y` | Undo or redo in memory | +| `Ctrl+Z` / `Ctrl+Y` | Undo or redo and save the result | | `Ctrl+S` | Save the current in-memory state | Within the Entry Editor, use `Tab` and `Shift+Tab` to move between fields and diff --git a/src/hosts/tui/app.py b/src/hosts/tui/app.py index 80e388a..599c6e5 100644 --- a/src/hosts/tui/app.py +++ b/src/hosts/tui/app.py @@ -5,6 +5,8 @@ This module contains the main application class that orchestrates all the handlers and provides the primary user interface. """ +from collections.abc import Callable + from textual.app import App, ComposeResult, SuspendNotSupported from textual.containers import Horizontal, Vertical from textual.widgets import ( @@ -20,6 +22,7 @@ from textual.reactive import reactive from ..core.parser import HostsParser from ..core.models import HostsFile +from ..core.commands import OperationResult from ..core.config import Config from ..core.manager import HostsManager from ..core.dns import DNSService @@ -712,6 +715,25 @@ class HostsManagerApp(App): """Quit the application.""" self.navigation_handler.quit_application() + def _save_history_operation( + self, action: str, rollback: Callable[[HostsFile], OperationResult] + ) -> bool: + """Save a history change or restore the previous state after a failure.""" + save_success, save_message = self.manager.save_hosts_file(self.hosts_file) + if save_success: + return True + + rollback_result = rollback(self.hosts_file) + if rollback_result.success: + self.update_status( + f"❌ {action} save failed; previous state restored: {save_message}" + ) + else: + self.update_status( + f"❌ {action} save failed and could not restore the previous state: {save_message}; {rollback_result.message}. Reload the Hosts File." + ) + return False + def action_undo(self) -> None: """Undo the last operation.""" if not self.edit_mode: @@ -728,6 +750,11 @@ class HostsManagerApp(App): # Perform undo result = self.manager.undo_last_operation(self.hosts_file) if result.success: + if not self._save_history_operation( + "Undo", self.manager.redo_last_operation + ): + return + # Refresh the table and update UI self.table_handler.populate_entries_table() self.details_handler.update_entry_details() @@ -751,6 +778,11 @@ class HostsManagerApp(App): # Perform redo result = self.manager.redo_last_operation(self.hosts_file) if result.success: + if not self._save_history_operation( + "Redo", self.manager.undo_last_operation + ): + return + # Refresh the table and update UI self.table_handler.populate_entries_table() self.details_handler.update_entry_details() diff --git a/tests/test_main.py b/tests/test_main.py index be80570..2843fcb 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -986,6 +986,108 @@ class TestHostsManagerApp: # Should start DNS resolution app.run_worker.assert_called() + def test_undo_saves_the_reverted_hosts_file(self): + """Undo persists its result before refreshing the interface.""" + with patch("hosts.tui.app.HostsParser"), patch("hosts.tui.app.Config"): + app = HostsManagerApp() + app.edit_mode = True + app.manager.edit_mode = True + app.hosts_file = HostsFile() + entry = HostEntry(ip_address="192.168.1.1", hostnames=["test.local"]) + app.hosts_file.add_entry(entry) + app.manager.execute_toggle_command(app.hosts_file, 0) + app.manager.save_hosts_file = Mock(return_value=(True, "Hosts file saved")) + app.table_handler.populate_entries_table = Mock() + app.details_handler.update_entry_details = Mock() + app.update_status = Mock() + + app.action_undo() + + assert entry.is_active is True + app.manager.save_hosts_file.assert_called_once_with(app.hosts_file) + app.table_handler.populate_entries_table.assert_called_once() + app.details_handler.update_entry_details.assert_called_once() + + def test_undo_save_failure_restores_the_persisted_state(self): + """Undo failure leaves the display and undo history at the saved state.""" + with patch("hosts.tui.app.HostsParser"), patch("hosts.tui.app.Config"): + app = HostsManagerApp() + app.edit_mode = True + app.manager.edit_mode = True + app.hosts_file = HostsFile() + entry = HostEntry(ip_address="192.168.1.1", hostnames=["test.local"]) + app.hosts_file.add_entry(entry) + app.manager.execute_toggle_command(app.hosts_file, 0) + app.manager.save_hosts_file = Mock( + return_value=(False, "Permission denied") + ) + app.table_handler.populate_entries_table = Mock() + app.details_handler.update_entry_details = Mock() + app.update_status = Mock() + + app.action_undo() + + assert entry.is_active is False + assert app.manager.can_undo() + assert not app.manager.can_redo() + app.table_handler.populate_entries_table.assert_not_called() + app.details_handler.update_entry_details.assert_not_called() + app.update_status.assert_called_once_with( + "❌ Undo save failed; previous state restored: Permission denied" + ) + + def test_redo_saves_the_reapplied_hosts_file(self): + """Redo persists its result before refreshing the interface.""" + with patch("hosts.tui.app.HostsParser"), patch("hosts.tui.app.Config"): + app = HostsManagerApp() + app.edit_mode = True + app.manager.edit_mode = True + app.hosts_file = HostsFile() + entry = HostEntry(ip_address="192.168.1.1", hostnames=["test.local"]) + app.hosts_file.add_entry(entry) + app.manager.execute_toggle_command(app.hosts_file, 0) + app.manager.undo_last_operation(app.hosts_file) + app.manager.save_hosts_file = Mock(return_value=(True, "Hosts file saved")) + app.table_handler.populate_entries_table = Mock() + app.details_handler.update_entry_details = Mock() + app.update_status = Mock() + + app.action_redo() + + assert entry.is_active is False + app.manager.save_hosts_file.assert_called_once_with(app.hosts_file) + app.table_handler.populate_entries_table.assert_called_once() + app.details_handler.update_entry_details.assert_called_once() + + def test_redo_save_failure_restores_the_persisted_state(self): + """Redo failure leaves the display and redo history at the saved state.""" + with patch("hosts.tui.app.HostsParser"), patch("hosts.tui.app.Config"): + app = HostsManagerApp() + app.edit_mode = True + app.manager.edit_mode = True + app.hosts_file = HostsFile() + entry = HostEntry(ip_address="192.168.1.1", hostnames=["test.local"]) + app.hosts_file.add_entry(entry) + app.manager.execute_toggle_command(app.hosts_file, 0) + app.manager.undo_last_operation(app.hosts_file) + app.manager.save_hosts_file = Mock( + return_value=(False, "Permission denied") + ) + app.table_handler.populate_entries_table = Mock() + app.details_handler.update_entry_details = Mock() + app.update_status = Mock() + + app.action_redo() + + assert entry.is_active is True + assert not app.manager.can_undo() + assert app.manager.can_redo() + app.table_handler.populate_entries_table.assert_not_called() + app.details_handler.update_entry_details.assert_not_called() + app.update_status.assert_called_once_with( + "❌ Redo save failed; previous state restored: Permission denied" + ) + def test_main_function(self): """Test main entry point function.""" with patch("hosts.main.HostsManagerApp") as mock_app_class: