diff --git a/memory-bank/activeContext.md b/memory-bank/activeContext.md index 35b0636..e85656d 100644 --- a/memory-bank/activeContext.md +++ b/memory-bank/activeContext.md @@ -2,10 +2,19 @@ ## Current Work Focus -**Phase 3 Complete - Edit Mode Foundation**: The hosts TUI application now has a complete edit mode foundation with permission management, entry manipulation, and safe file operations. All keyboard shortcuts are implemented and tested. The application is ready for Phase 4 advanced edit features. +**Phase 3 Complete with Save Confirmation Enhancement**: The hosts TUI application now has a complete edit mode foundation with permission management, entry manipulation, safe file operations, and professional save confirmation functionality. All keyboard shortcuts are implemented and tested. The application is ready for Phase 4 advanced edit features. ## Recent Changes +### Phase 3 Save Confirmation Enhancement ✅ COMPLETE +- ✅ **Save confirmation modal**: Professional modal dialog asking to save, discard, or cancel when exiting edit entry mode +- ✅ **Change detection system**: Intelligent tracking of original entry values vs. current form values +- ✅ **No auto-save behavior**: Changes are only saved when explicitly confirmed by the user +- ✅ **Graceful exit handling**: ESC key in edit entry mode now triggers save confirmation instead of auto-exiting +- ✅ **Validation integration**: Full validation before saving with clear error messages for invalid data +- ✅ **Comprehensive testing**: 13 new tests for save confirmation functionality (161 total tests) +- ✅ **Modal keyboard shortcuts**: Save (S), Discard (D), Cancel (ESC) with intuitive button labels + ### Phase 2 Implementation Complete - ✅ **Advanced configuration system**: Complete Config class with JSON persistence to ~/.config/hosts-manager/ - ✅ **Professional configuration modal**: Modal dialog with keyboard bindings for settings management diff --git a/memory-bank/progress.md b/memory-bank/progress.md index a5b38cd..455215d 100644 --- a/memory-bank/progress.md +++ b/memory-bank/progress.md @@ -64,7 +64,9 @@ - ✅ **Live testing**: Manual testing confirms all functionality works correctly - ✅ **Human-readable formatting**: Tab-based column alignment with proper spacing - ✅ **Management header**: Automatic addition of management header to hosts files -- ✅ **Auto-save functionality**: Immediate saving of changes when entries are edited +- ✅ **Save confirmation modal**: Professional modal dialog for save/discard/cancel when exiting edit entry mode +- ✅ **Change detection**: Intelligent tracking of original vs. current entry values +- ✅ **No auto-save**: Changes saved only when explicitly confirmed by user ### Phase 4: Advanced Edit Features - ❌ **Add new entries**: Create new host entries @@ -89,8 +91,8 @@ ## Current Status ### Development Stage -**Stage**: Phase 3 Complete - Moving to Phase 4 -**Progress**: 75% (Complete edit mode foundation with permission management) +**Stage**: Phase 3 Complete with Save Confirmation Enhancement - Ready for Phase 4 +**Progress**: 78% (Complete edit mode foundation with professional save confirmation) **Next Milestone**: Advanced edit features (add/delete entries, bulk operations) ### Phase 2 Final Achievements @@ -112,7 +114,10 @@ 6. ✅ **Manager module**: Complete HostsManager class for all edit operations 7. ✅ **Safe file operations**: Atomic file writing with rollback capability 8. ✅ **Enhanced error messages**: Professional read-only mode error messages with clear instructions -9. ✅ **Comprehensive testing**: 38 new tests for manager module (135 total tests) +9. ✅ **Comprehensive testing**: 38 new tests for manager module (148 total tests) +10. ✅ **Save confirmation modal**: Professional save/discard/cancel dialog when exiting edit entry mode +11. ✅ **Change detection system**: Intelligent tracking of original vs. current entry values +12. ✅ **No auto-save behavior**: User-controlled saving with explicit confirmation ### Recent Major Accomplishments - ✅ **Complete Phase 3 implementation**: Full edit mode foundation with permission management @@ -121,7 +126,8 @@ - ✅ **Permission system**: Robust sudo request, validation, and release functionality - ✅ **File backup system**: Automatic backup creation with timestamp naming - ✅ **Entry manipulation**: Toggle active/inactive and reorder entries safely -- ✅ **Comprehensive testing**: 135 total tests with 100% pass rate including edit operations +- ✅ **Save confirmation enhancement**: Professional modal dialog system for editing workflow +- ✅ **Comprehensive testing**: 161 total tests with 100% pass rate including save confirmation - ✅ **Error handling**: Graceful handling of permission errors and file operations - ✅ **Keyboard shortcuts**: Complete set of edit mode bindings (e, space, Ctrl+Up/Down, Ctrl+S) diff --git a/src/hosts/main.py b/src/hosts/main.py index d2f3856..8754a72 100644 --- a/src/hosts/main.py +++ b/src/hosts/main.py @@ -18,16 +18,17 @@ from .core.models import HostsFile from .core.config import Config from .core.manager import HostsManager from .tui.config_modal import ConfigModal +from .tui.save_confirmation_modal import SaveConfirmationModal class HostsManagerApp(App): """ Main application class for the hosts TUI manager. - + Provides a two-pane interface for managing hosts file entries with read-only mode by default and explicit edit mode. """ - + CSS = """ .hosts-container { height: 1fr; @@ -115,7 +116,7 @@ class HostsManagerApp(App): margin-bottom: 1; } """ - + BINDINGS = [ Binding("q", "quit", "Quit"), Binding("r", "reload", "Reload"), @@ -134,7 +135,7 @@ class HostsManagerApp(App): Binding("shift+tab", "prev_field", "Prev Field", show=False), ("ctrl+c", "quit", "Quit"), ] - + # Reactive attributes hosts_file: reactive[HostsFile] = reactive(HostsFile()) selected_entry_index: reactive[int] = reactive(0) @@ -142,7 +143,7 @@ class HostsManagerApp(App): entry_edit_mode: reactive[bool] = reactive(False) sort_column: reactive[str] = reactive("") # "ip" or "hostname" sort_ascending: reactive[bool] = reactive(True) - + def __init__(self): super().__init__() self.parser = HostsParser() @@ -150,11 +151,14 @@ class HostsManagerApp(App): self.manager = HostsManager() self.title = "Hosts Manager" self.sub_title = "Read-only mode" - + + # Track original entry values for change detection + self.original_entry_values = None + def compose(self) -> ComposeResult: """Create the application layout.""" yield Header() - + with Vertical(): with Horizontal(classes="hosts-container"): left_pane = Vertical(classes="left-pane") @@ -162,7 +166,7 @@ class HostsManagerApp(App): with left_pane: yield DataTable(id="entries-table") yield left_pane - + right_pane = Vertical(classes="right-pane") right_pane.border_title = "Entry Details" with right_pane: @@ -173,34 +177,38 @@ class HostsManagerApp(App): yield Label("Hostname:") yield Input(id="hostname-input", placeholder="Enter hostname") yield Label("Comment:") - yield Input(id="comment-input", placeholder="Enter comment (optional)") + yield Input( + id="comment-input", placeholder="Enter comment (optional)" + ) yield Label("Active:") yield Checkbox(id="active-checkbox", value=True) yield right_pane - + yield Static("", classes="status-bar", id="status") - + yield Footer() - + def on_ready(self) -> None: """Initialize the application when ready.""" self.load_hosts_file() self.update_status() - + def load_hosts_file(self) -> None: """Load the hosts file and populate the interface.""" # Remember current selection for restoration current_entry = None - if self.hosts_file.entries and self.selected_entry_index < len(self.hosts_file.entries): + if self.hosts_file.entries and self.selected_entry_index < len( + self.hosts_file.entries + ): current_entry = self.hosts_file.entries[self.selected_entry_index] - + try: self.hosts_file = self.parser.parse() self.populate_entries_table() - + # Restore cursor position with a timer to ensure ListView is fully rendered self.set_timer(0.1, lambda: self.restore_cursor_position(current_entry)) - + self.update_entry_details() self.log(f"Loaded {len(self.hosts_file.entries)} entries from hosts file") except FileNotFoundError: @@ -212,80 +220,84 @@ class HostsManagerApp(App): except Exception as e: self.log(f"Error loading hosts file: {e}") self.update_status(f"Error: {e}") - + def get_visible_entries(self) -> list: """Get the list of entries that are visible in the table (after filtering).""" show_defaults = self.config.should_show_default_entries() visible_entries = [] - + for entry in self.hosts_file.entries: canonical_hostname = entry.hostnames[0] if entry.hostnames else "" # Skip default entries if configured to hide them - if not show_defaults and self.config.is_default_entry(entry.ip_address, canonical_hostname): + if not show_defaults and self.config.is_default_entry( + entry.ip_address, canonical_hostname + ): continue visible_entries.append(entry) - + return visible_entries - + def get_first_visible_entry_index(self) -> int: """Get the index of the first visible entry in the hosts file.""" show_defaults = self.config.should_show_default_entries() - + for i, entry in enumerate(self.hosts_file.entries): canonical_hostname = entry.hostnames[0] if entry.hostnames else "" # Skip default entries if configured to hide them - if not show_defaults and self.config.is_default_entry(entry.ip_address, canonical_hostname): + if not show_defaults and self.config.is_default_entry( + entry.ip_address, canonical_hostname + ): continue return i - + # If no visible entries found, return 0 return 0 - + def display_index_to_actual_index(self, display_index: int) -> int: """Convert a display table index to the actual hosts file entry index.""" visible_entries = self.get_visible_entries() if display_index >= len(visible_entries): return 0 - + target_entry = visible_entries[display_index] - + # Find this entry in the full hosts file for i, entry in enumerate(self.hosts_file.entries): if entry is target_entry: return i - + return 0 - + def actual_index_to_display_index(self, actual_index: int) -> int: """Convert an actual hosts file entry index to a display table index.""" if actual_index >= len(self.hosts_file.entries): return 0 - + target_entry = self.hosts_file.entries[actual_index] visible_entries = self.get_visible_entries() - + # Find this entry in the visible entries for i, entry in enumerate(visible_entries): if entry is target_entry: return i - + return 0 - + def populate_entries_table(self) -> None: """Populate the left pane with hosts entries using DataTable.""" table = self.query_one("#entries-table", DataTable) table.clear(columns=True) # Clear both rows and columns - + # Configure DataTable properties table.zebra_stripes = True table.cursor_type = "row" table.show_header = True - + # Create column labels with sort indicators active_label = "Active" ip_label = "IP Address" hostname_label = "Canonical Hostname" - + # Add sort indicators if self.sort_column == "ip": arrow = "↑" if self.sort_ascending else "↓" @@ -293,21 +305,21 @@ class HostsManagerApp(App): elif self.sort_column == "hostname": arrow = "↑" if self.sort_ascending else "↓" hostname_label = f"{arrow} Canonical Hostname" - + # Add columns with proper labels (Active column first) table.add_columns(active_label, ip_label, hostname_label) - + # Get visible entries (after filtering) visible_entries = self.get_visible_entries() - + # Add rows for entry in visible_entries: # Get the canonical hostname (first hostname) canonical_hostname = entry.hostnames[0] if entry.hostnames else "" - + # Check if this is a default system entry is_default = entry.is_default_entry() - + # Add row with styling based on active status and default entry status if is_default: # Default entries are always shown in dim grey regardless of active status @@ -327,28 +339,30 @@ class HostsManagerApp(App): ip_text = Text(entry.ip_address, style="dim yellow italic") hostname_text = Text(canonical_hostname, style="dim yellow italic") table.add_row(active_text, ip_text, hostname_text) - + def restore_cursor_position(self, previous_entry) -> None: """Restore cursor position after reload, maintaining selection if possible.""" if not self.hosts_file.entries: self.selected_entry_index = 0 return - + if previous_entry is None: # No previous selection, start at first visible entry self.selected_entry_index = self.get_first_visible_entry_index() else: # Try to find the same entry in the reloaded file for i, entry in enumerate(self.hosts_file.entries): - if (entry.ip_address == previous_entry.ip_address and - entry.hostnames == previous_entry.hostnames and - entry.comment == previous_entry.comment): + if ( + entry.ip_address == previous_entry.ip_address + and entry.hostnames == previous_entry.hostnames + and entry.comment == previous_entry.comment + ): self.selected_entry_index = i break else: # Entry not found, default to first visible entry self.selected_entry_index = self.get_first_visible_entry_index() - + # Update the DataTable cursor position using display index table = self.query_one("#entries-table", DataTable) display_index = self.actual_index_to_display_index(self.selected_entry_index) @@ -358,40 +372,44 @@ class HostsManagerApp(App): table.focus() # Update the details pane to match the selection self.update_entry_details() - + def update_entry_details(self) -> None: """Update the right pane with selected entry details.""" if self.entry_edit_mode: self.update_edit_form() else: self.update_details_display() - + def update_details_display(self) -> None: """Update the static details display.""" details_widget = self.query_one("#entry-details", Static) edit_form = self.query_one("#entry-edit-form") - + # Show details, hide edit form details_widget.remove_class("hidden") edit_form.add_class("hidden") - + if not self.hosts_file.entries: details_widget.update("No entries loaded") return - + # Get visible entries to check if we need to adjust selection visible_entries = self.get_visible_entries() if not visible_entries: details_widget.update("No visible entries") return - + # If default entries are hidden and selected_entry_index points to a hidden entry, # we need to find the corresponding visible entry show_defaults = self.config.should_show_default_entries() if not show_defaults: # Check if the currently selected entry is a default entry (hidden) - if (self.selected_entry_index < len(self.hosts_file.entries) and - self.hosts_file.entries[self.selected_entry_index].is_default_entry()): + if ( + self.selected_entry_index < len(self.hosts_file.entries) + and self.hosts_file.entries[ + self.selected_entry_index + ].is_default_entry() + ): # The selected entry is hidden, so we should show the first visible entry instead if visible_entries: # Find the first visible entry in the hosts file @@ -399,61 +417,65 @@ class HostsManagerApp(App): if not entry.is_default_entry(): self.selected_entry_index = i break - + if self.selected_entry_index >= len(self.hosts_file.entries): self.selected_entry_index = 0 - + entry = self.hosts_file.entries[self.selected_entry_index] - + details_lines = [ f"IP Address: {entry.ip_address}", f"Hostnames: {', '.join(entry.hostnames)}", f"Status: {'Active' if entry.is_active else 'Inactive'}", ] - + # Add notice for default system entries if entry.is_default_entry(): details_lines.append("") details_lines.append("⚠️ SYSTEM DEFAULT ENTRY") - details_lines.append("This is a default system entry and cannot be modified.") - + details_lines.append( + "This is a default system entry and cannot be modified." + ) + if entry.comment: details_lines.append(f"Comment: {entry.comment}") - + if entry.dns_name: details_lines.append(f"DNS Name: {entry.dns_name}") - + details_widget.update("\n".join(details_lines)) - + def update_edit_form(self) -> None: """Update the edit form with current entry values.""" details_widget = self.query_one("#entry-details", Static) edit_form = self.query_one("#entry-edit-form") - + # Hide details, show edit form details_widget.add_class("hidden") edit_form.remove_class("hidden") - - if not self.hosts_file.entries or self.selected_entry_index >= len(self.hosts_file.entries): + + if not self.hosts_file.entries or self.selected_entry_index >= len( + self.hosts_file.entries + ): return - + entry = self.hosts_file.entries[self.selected_entry_index] - + # Update form fields with current entry values ip_input = self.query_one("#ip-input", Input) hostname_input = self.query_one("#hostname-input", Input) comment_input = self.query_one("#comment-input", Input) active_checkbox = self.query_one("#active-checkbox", Checkbox) - + ip_input.value = entry.ip_address - hostname_input.value = ', '.join(entry.hostnames) + hostname_input.value = ", ".join(entry.hostnames) comment_input.value = entry.comment or "" active_checkbox.value = entry.is_active - + def update_status(self, message: str = "") -> None: """Update the status bar.""" status_widget = self.query_one("#status", Static) - + if message: # Check if this is an error message (starts with ❌) if message.startswith("❌"): @@ -474,34 +496,38 @@ class HostsManagerApp(App): # Reset to normal status display status_widget.remove_class("status-error") status_widget.add_class("status-bar") - + mode = "Edit mode" if self.edit_mode else "Read-only mode" entry_count = len(self.hosts_file.entries) active_count = len(self.hosts_file.get_active_entries()) - + status_text = f"{mode} | {entry_count} entries ({active_count} active)" - + # Add file info file_info = self.parser.get_file_info() - if file_info['exists']: + if file_info["exists"]: status_text += f" | {file_info['path']}" - + status_widget.update(status_text) - + def on_data_table_row_highlighted(self, event: DataTable.RowHighlighted) -> None: """Handle row highlighting (cursor movement) in the DataTable.""" if event.data_table.id == "entries-table": # Convert display index to actual index - self.selected_entry_index = self.display_index_to_actual_index(event.cursor_row) + self.selected_entry_index = self.display_index_to_actual_index( + event.cursor_row + ) self.update_entry_details() - + def on_data_table_row_selected(self, event: DataTable.RowSelected) -> None: """Handle row selection in the DataTable.""" if event.data_table.id == "entries-table": # Convert display index to actual index - self.selected_entry_index = self.display_index_to_actual_index(event.cursor_row) + self.selected_entry_index = self.display_index_to_actual_index( + event.cursor_row + ) self.update_entry_details() - + def action_reload(self) -> None: """Reload the hosts file.""" # Reset sort state on reload @@ -509,22 +535,25 @@ class HostsManagerApp(App): self.sort_ascending = True self.load_hosts_file() self.update_status("Hosts file reloaded") - + def action_help(self) -> None: """Show help information.""" # For now, just update the status with help info - self.update_status("Help: ↑/↓ Navigate, r Reload, q Quit, h Help, c Config, e Edit") - + self.update_status( + "Help: ↑/↓ Navigate, r Reload, q Quit, h Help, c Config, e Edit" + ) + def action_config(self) -> None: """Show configuration modal.""" + def handle_config_result(config_changed: bool) -> None: if config_changed: # Reload the table to apply new filtering self.populate_entries_table() self.update_status("Configuration saved") - + self.push_screen(ConfigModal(self.config), handle_config_result) - + def action_sort_by_ip(self) -> None: """Sort entries by IP address, toggle ascending/descending.""" # Toggle sort direction if already sorting by IP @@ -533,14 +562,14 @@ class HostsManagerApp(App): else: self.sort_column = "ip" self.sort_ascending = True - + # Sort the entries using the new method that keeps defaults on top self.hosts_file.sort_by_ip(self.sort_ascending) self.populate_entries_table() - + direction = "ascending" if self.sort_ascending else "descending" self.update_status(f"Sorted by IP address ({direction})") - + def action_sort_by_hostname(self) -> None: """Sort entries by canonical hostname, toggle ascending/descending.""" # Toggle sort direction if already sorting by hostname @@ -549,14 +578,14 @@ class HostsManagerApp(App): else: self.sort_column = "hostname" self.sort_ascending = True - + # Sort the entries using the new method that keeps defaults on top self.hosts_file.sort_by_hostname(self.sort_ascending) self.populate_entries_table() - + direction = "ascending" if self.sort_ascending else "descending" self.update_status(f"Sorted by hostname ({direction})") - + def on_data_table_header_selected(self, event: DataTable.HeaderSelected) -> None: """Handle column header clicks for sorting.""" if event.data_table.id == "entries-table": @@ -565,7 +594,7 @@ class HostsManagerApp(App): self.action_sort_by_ip() elif "Canonical Hostname" in str(event.column_key): self.action_sort_by_hostname() - + def action_toggle_edit_mode(self) -> None: """Toggle between read-only and edit mode.""" if self.edit_mode: @@ -586,87 +615,228 @@ class HostsManagerApp(App): self.update_status(message) else: self.update_status(f"Error entering edit mode: {message}") - + def action_edit_entry(self) -> None: """Enter edit mode for the selected entry.""" if not self.edit_mode: - self.update_status("❌ Cannot edit entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode.") + self.update_status( + "❌ Cannot edit entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode." + ) return - + if not self.hosts_file.entries: self.update_status("No entries to edit") return - + if self.selected_entry_index >= len(self.hosts_file.entries): self.update_status("Invalid entry selected") return - + entry = self.hosts_file.entries[self.selected_entry_index] if entry.is_default_entry(): self.update_status("❌ Cannot edit system default entry") return - + + # Store original values for change detection + self.original_entry_values = { + "ip_address": entry.ip_address, + "hostnames": entry.hostnames.copy(), + "comment": entry.comment, + "is_active": entry.is_active, + } + self.entry_edit_mode = True self.update_entry_details() - + # Focus on the IP address input field ip_input = self.query_one("#ip-input", Input) ip_input.focus() - + self.update_status("Editing entry - Use Tab/Shift+Tab to navigate, ESC to exit") - + + def has_entry_changes(self) -> bool: + """Check if the current entry has been modified from its original values.""" + if not self.original_entry_values or not self.entry_edit_mode: + return False + + # Get current values from form fields + ip_input = self.query_one("#ip-input", Input) + hostname_input = self.query_one("#hostname-input", Input) + comment_input = self.query_one("#comment-input", Input) + active_checkbox = self.query_one("#active-checkbox", Checkbox) + + current_hostnames = [ + h.strip() for h in hostname_input.value.split(",") if h.strip() + ] + current_comment = comment_input.value.strip() or None + + # Compare with original values + return ( + ip_input.value.strip() != self.original_entry_values["ip_address"] + or current_hostnames != self.original_entry_values["hostnames"] + or current_comment != self.original_entry_values["comment"] + or active_checkbox.value != self.original_entry_values["is_active"] + ) + def action_exit_edit_entry(self) -> None: """Exit entry edit mode and return focus to the entries table.""" - if self.entry_edit_mode: - self.entry_edit_mode = False - self.update_entry_details() - - # Return focus to the entries table + if not self.entry_edit_mode: + return + + # Check if there are unsaved changes + if self.has_entry_changes(): + # Show save confirmation modal + def handle_save_confirmation(result): + if result == "save": + # Validate and save changes + if self.validate_and_save_entry_changes(): + self.exit_edit_entry_mode() + elif result == "discard": + # Restore original values and exit + self.restore_original_entry_values() + self.exit_edit_entry_mode() + elif result == "cancel": + # Do nothing, stay in edit mode + pass + + self.push_screen(SaveConfirmationModal(), handle_save_confirmation) + else: + # No changes, exit directly + self.exit_edit_entry_mode() + + def exit_edit_entry_mode(self) -> None: + """Helper method to exit entry edit mode and clean up.""" + self.entry_edit_mode = False + self.original_entry_values = None + self.update_entry_details() + + # Return focus to the entries table + table = self.query_one("#entries-table", DataTable) + table.focus() + + self.update_status("Exited entry edit mode") + + def restore_original_entry_values(self) -> None: + """Restore the original values to the form fields.""" + if not self.original_entry_values: + return + + # Update form fields with original values + ip_input = self.query_one("#ip-input", Input) + hostname_input = self.query_one("#hostname-input", Input) + comment_input = self.query_one("#comment-input", Input) + active_checkbox = self.query_one("#active-checkbox", Checkbox) + + ip_input.value = self.original_entry_values["ip_address"] + hostname_input.value = ", ".join(self.original_entry_values["hostnames"]) + comment_input.value = self.original_entry_values["comment"] or "" + active_checkbox.value = self.original_entry_values["is_active"] + + def validate_and_save_entry_changes(self) -> bool: + """Validate current entry values and save if valid.""" + if not self.hosts_file.entries or self.selected_entry_index >= len( + self.hosts_file.entries + ): + return False + + entry = self.hosts_file.entries[self.selected_entry_index] + + # Get values from form fields + ip_input = self.query_one("#ip-input", Input) + hostname_input = self.query_one("#hostname-input", Input) + comment_input = self.query_one("#comment-input", Input) + active_checkbox = self.query_one("#active-checkbox", Checkbox) + + # Validate IP address + try: + ipaddress.ip_address(ip_input.value.strip()) + except ValueError: + self.update_status("❌ Invalid IP address - changes not saved") + return False + + # Validate hostname(s) + hostnames = [h.strip() for h in hostname_input.value.split(",") if h.strip()] + if not hostnames: + self.update_status( + "❌ At least one hostname is required - changes not saved" + ) + return False + + hostname_pattern = re.compile( + r"^[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)*$" + ) + + for hostname in hostnames: + if not hostname_pattern.match(hostname): + self.update_status( + f"❌ Invalid hostname: {hostname} - changes not saved" + ) + return False + + # Update the entry + entry.ip_address = ip_input.value.strip() + entry.hostnames = hostnames + entry.comment = comment_input.value.strip() or None + entry.is_active = active_checkbox.value + + # Save to file + success, message = self.manager.save_hosts_file(self.hosts_file) + if success: + # Update the table display + self.populate_entries_table() + # Restore cursor position table = self.query_one("#entries-table", DataTable) - table.focus() - - self.update_status("Exited entry edit mode") - + display_index = self.actual_index_to_display_index( + self.selected_entry_index + ) + if table.row_count > 0 and display_index < table.row_count: + table.move_cursor(row=display_index) + self.update_status("Entry saved successfully") + return True + else: + self.update_status(f"❌ Error saving entry: {message}") + return False + def action_next_field(self) -> None: """Move to the next field in edit mode.""" if not self.entry_edit_mode: return - + # Get all input fields in order fields = [ self.query_one("#ip-input", Input), self.query_one("#hostname-input", Input), self.query_one("#comment-input", Input), - self.query_one("#active-checkbox", Checkbox) + self.query_one("#active-checkbox", Checkbox), ] - + # Find currently focused field and move to next for i, field in enumerate(fields): if field.has_focus: next_field = fields[(i + 1) % len(fields)] next_field.focus() break - + def action_prev_field(self) -> None: """Move to the previous field in edit mode.""" if not self.entry_edit_mode: return - + # Get all input fields in order fields = [ self.query_one("#ip-input", Input), self.query_one("#hostname-input", Input), self.query_one("#comment-input", Input), - self.query_one("#active-checkbox", Checkbox) + self.query_one("#active-checkbox", Checkbox), ] - + # Find currently focused field and move to previous for i, field in enumerate(fields): if field.has_focus: prev_field = fields[(i - 1) % len(fields)] prev_field.focus() break - + def on_key(self, event) -> None: """Handle key events to override default tab behavior in edit mode.""" if self.entry_edit_mode and event.key == "tab": @@ -675,94 +845,39 @@ class HostsManagerApp(App): self.action_next_field() elif self.entry_edit_mode and event.key == "shift+tab": # Prevent default shift+tab behavior and use our custom navigation - event.prevent_default() + event.prevent_default() self.action_prev_field() - + def on_input_changed(self, event: Input.Changed) -> None: - """Handle input field changes and auto-save.""" - if not self.entry_edit_mode or not self.edit_mode: - return - - if event.input.id in ["ip-input", "hostname-input", "comment-input"]: - self.save_entry_changes() - + """Handle input field changes (no auto-save - changes saved on exit).""" + # Input changes are tracked but not automatically saved + # Changes will be validated and saved when exiting edit mode + pass + def on_checkbox_changed(self, event: Checkbox.Changed) -> None: - """Handle checkbox changes and auto-save.""" - if not self.entry_edit_mode or not self.edit_mode: - return - - if event.checkbox.id == "active-checkbox": - self.save_entry_changes() - - def save_entry_changes(self) -> None: - """Save the current entry changes.""" - if not self.hosts_file.entries or self.selected_entry_index >= len(self.hosts_file.entries): - return - - entry = self.hosts_file.entries[self.selected_entry_index] - - # Get values from form fields - ip_input = self.query_one("#ip-input", Input) - hostname_input = self.query_one("#hostname-input", Input) - comment_input = self.query_one("#comment-input", Input) - active_checkbox = self.query_one("#active-checkbox", Checkbox) - - # Validate IP address - try: - ipaddress.ip_address(ip_input.value.strip()) - except ValueError: - self.update_status("❌ Invalid IP address") - return - - # Validate hostname(s) - hostnames = [h.strip() for h in hostname_input.value.split(',') if h.strip()] - if not hostnames: - self.update_status("❌ At least one hostname is required") - return - - hostname_pattern = re.compile( - r'^[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)*$' - ) - - for hostname in hostnames: - if not hostname_pattern.match(hostname): - self.update_status(f"❌ Invalid hostname: {hostname}") - return - - # Update the entry - entry.ip_address = ip_input.value.strip() - entry.hostnames = hostnames - entry.comment = comment_input.value.strip() or None - entry.is_active = active_checkbox.value - - # Save to file - success, message = self.manager.save_hosts_file(self.hosts_file) - if success: - # Update the table display - self.populate_entries_table() - # Restore cursor position - table = self.query_one("#entries-table", DataTable) - display_index = self.actual_index_to_display_index(self.selected_entry_index) - if table.row_count > 0 and display_index < table.row_count: - table.move_cursor(row=display_index) - self.update_status("Entry saved successfully") - else: - self.update_status(f"❌ Error saving entry: {message}") - + """Handle checkbox changes (no auto-save - changes saved on exit).""" + # Checkbox changes are tracked but not automatically saved + # Changes will be validated and saved when exiting edit mode + pass + def action_toggle_entry(self) -> None: """Toggle the active state of the selected entry.""" if not self.edit_mode: - self.update_status("❌ Cannot toggle entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode.") + self.update_status( + "❌ Cannot toggle entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode." + ) return - + if not self.hosts_file.entries: self.update_status("No entries to toggle") return - + # Remember current entry for cursor position restoration current_entry = self.hosts_file.entries[self.selected_entry_index] - - success, message = self.manager.toggle_entry(self.hosts_file, self.selected_entry_index) + + success, message = self.manager.toggle_entry( + self.hosts_file, self.selected_entry_index + ) if success: # Auto-save the changes immediately save_success, save_message = self.manager.save_hosts_file(self.hosts_file) @@ -776,18 +891,22 @@ class HostsManagerApp(App): self.update_status(f"Entry toggled but save failed: {save_message}") else: self.update_status(f"Error toggling entry: {message}") - + def action_move_entry_up(self) -> None: """Move the selected entry up in the list.""" if not self.edit_mode: - self.update_status("❌ Cannot move entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode.") + self.update_status( + "❌ Cannot move entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode." + ) return - + if not self.hosts_file.entries: self.update_status("No entries to move") return - - success, message = self.manager.move_entry_up(self.hosts_file, self.selected_entry_index) + + success, message = self.manager.move_entry_up( + self.hosts_file, self.selected_entry_index + ) if success: # Auto-save the changes immediately save_success, save_message = self.manager.save_hosts_file(self.hosts_file) @@ -798,7 +917,9 @@ class HostsManagerApp(App): self.populate_entries_table() # Update the DataTable cursor position to follow the moved entry table = self.query_one("#entries-table", DataTable) - display_index = self.actual_index_to_display_index(self.selected_entry_index) + display_index = self.actual_index_to_display_index( + self.selected_entry_index + ) if table.row_count > 0 and display_index < table.row_count: table.move_cursor(row=display_index) self.update_entry_details() @@ -807,18 +928,22 @@ class HostsManagerApp(App): self.update_status(f"Entry moved but save failed: {save_message}") else: self.update_status(f"Error moving entry: {message}") - + def action_move_entry_down(self) -> None: """Move the selected entry down in the list.""" if not self.edit_mode: - self.update_status("❌ Cannot move entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode.") + self.update_status( + "❌ Cannot move entry: Application is in read-only mode. Press 'Ctrl+E' to enable edit mode." + ) return - + if not self.hosts_file.entries: self.update_status("No entries to move") return - - success, message = self.manager.move_entry_down(self.hosts_file, self.selected_entry_index) + + success, message = self.manager.move_entry_down( + self.hosts_file, self.selected_entry_index + ) if success: # Auto-save the changes immediately save_success, save_message = self.manager.save_hosts_file(self.hosts_file) @@ -829,7 +954,9 @@ class HostsManagerApp(App): self.populate_entries_table() # Update the DataTable cursor position to follow the moved entry table = self.query_one("#entries-table", DataTable) - display_index = self.actual_index_to_display_index(self.selected_entry_index) + display_index = self.actual_index_to_display_index( + self.selected_entry_index + ) if table.row_count > 0 and display_index < table.row_count: table.move_cursor(row=display_index) self.update_entry_details() @@ -838,25 +965,27 @@ class HostsManagerApp(App): self.update_status(f"Entry moved but save failed: {save_message}") else: self.update_status(f"Error moving entry: {message}") - + def action_save_file(self) -> None: """Save the hosts file to disk.""" if not self.edit_mode: - self.update_status("❌ Cannot save: Application is in read-only mode. No changes to save.") + self.update_status( + "❌ Cannot save: Application is in read-only mode. No changes to save." + ) return - + success, message = self.manager.save_hosts_file(self.hosts_file) if success: self.update_status(message) else: self.update_status(f"Error saving file: {message}") - + def action_quit(self) -> None: """Quit the application.""" # If in entry edit mode, exit it first if self.entry_edit_mode: self.action_exit_edit_entry() - + # If in edit mode, exit it first if self.edit_mode: self.manager.exit_edit_mode() diff --git a/src/hosts/tui/save_confirmation_modal.py b/src/hosts/tui/save_confirmation_modal.py new file mode 100644 index 0000000..8de6e48 --- /dev/null +++ b/src/hosts/tui/save_confirmation_modal.py @@ -0,0 +1,112 @@ +""" +Save confirmation modal for the hosts TUI application. + +This module provides a modal dialog to confirm saving changes when exiting edit entry mode. +""" + +from textual.app import ComposeResult +from textual.containers import Vertical, Horizontal +from textual.widgets import Static, Button, Label +from textual.screen import ModalScreen +from textual.binding import Binding + + +class SaveConfirmationModal(ModalScreen): + """ + Modal screen for save confirmation when exiting edit entry mode. + + Provides a confirmation dialog asking whether to save or discard changes. + """ + + CSS = """ + SaveConfirmationModal { + align: center middle; + } + + .save-confirmation-container { + width: 60; + height: 12; + background: $surface; + border: thick $primary; + padding: 1; + } + + .save-confirmation-title { + text-align: center; + text-style: bold; + color: $primary; + margin-bottom: 1; + } + + .save-confirmation-message { + text-align: center; + margin-bottom: 2; + color: $text; + } + + .button-row { + align: center middle; + } + + .save-confirmation-button { + margin: 0 1; + min-width: 12; + } + """ + + BINDINGS = [ + Binding("escape", "cancel", "Cancel"), + Binding("enter", "save", "Save"), + Binding("s", "save", "Save"), + Binding("d", "discard", "Discard"), + ] + + def compose(self) -> ComposeResult: + """Create the save confirmation modal layout.""" + with Vertical(classes="save-confirmation-container"): + yield Static("Save Changes?", classes="save-confirmation-title") + yield Label( + "You have made changes to this entry.\nDo you want to save or discard them?", + classes="save-confirmation-message", + ) + + with Horizontal(classes="button-row"): + yield Button( + "Save (S)", + variant="primary", + id="save-button", + classes="save-confirmation-button", + ) + yield Button( + "Discard (D)", + variant="default", + id="discard-button", + classes="save-confirmation-button", + ) + yield Button( + "Cancel (ESC)", + variant="default", + id="cancel-button", + classes="save-confirmation-button", + ) + + def on_button_pressed(self, event: Button.Pressed) -> None: + """Handle button presses.""" + if event.button.id == "save-button": + self.action_save() + elif event.button.id == "discard-button": + self.action_discard() + elif event.button.id == "cancel-button": + self.action_cancel() + + def action_save(self) -> None: + """Save changes and close modal.""" + self.dismiss("save") + + def action_discard(self) -> None: + """Discard changes and close modal.""" + self.dismiss("discard") + + def action_cancel(self) -> None: + """Cancel operation and close modal.""" + self.dismiss("cancel") diff --git a/tests/test_save_confirmation_modal.py b/tests/test_save_confirmation_modal.py new file mode 100644 index 0000000..0cff6a5 --- /dev/null +++ b/tests/test_save_confirmation_modal.py @@ -0,0 +1,284 @@ +""" +Tests for the save confirmation modal. + +This module tests the save confirmation functionality when exiting edit entry mode. +""" + +import pytest +from unittest.mock import Mock, patch +from textual.widgets import Input, Checkbox + +from hosts.main import HostsManagerApp +from hosts.core.models import HostsFile, HostEntry +from hosts.tui.save_confirmation_modal import SaveConfirmationModal + + +class TestSaveConfirmationModal: + """Test cases for the SaveConfirmationModal class.""" + + def test_modal_creation(self): + """Test that the modal can be created.""" + modal = SaveConfirmationModal() + assert modal is not None + + def test_modal_compose(self): + """Test that the modal composes correctly.""" + # Note: Cannot test compose() directly without app context + # This is a basic existence check for the modal + modal = SaveConfirmationModal() + assert hasattr(modal, "compose") + assert callable(modal.compose) + + def test_action_save(self): + """Test save action dismisses with 'save'.""" + modal = SaveConfirmationModal() + modal.dismiss = Mock() + + modal.action_save() + + modal.dismiss.assert_called_once_with("save") + + def test_action_discard(self): + """Test discard action dismisses with 'discard'.""" + modal = SaveConfirmationModal() + modal.dismiss = Mock() + + modal.action_discard() + + modal.dismiss.assert_called_once_with("discard") + + def test_action_cancel(self): + """Test cancel action dismisses with 'cancel'.""" + modal = SaveConfirmationModal() + modal.dismiss = Mock() + + modal.action_cancel() + + modal.dismiss.assert_called_once_with("cancel") + + +class TestSaveConfirmationIntegration: + """Test cases for save confirmation integration with the main app.""" + + @pytest.fixture + def app(self): + """Create a test app instance.""" + return HostsManagerApp() + + def test_has_entry_changes_no_original_values(self, app): + """Test has_entry_changes returns False when no original values stored.""" + app.original_entry_values = None + app.entry_edit_mode = True + + assert not app.has_entry_changes() + + def test_has_entry_changes_not_in_edit_mode(self, app): + """Test has_entry_changes returns False when not in edit mode.""" + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = False + + assert not app.has_entry_changes() + + @patch.object(HostsManagerApp, "query_one") + def test_has_entry_changes_no_changes(self, mock_query_one, app): + """Test has_entry_changes returns False when no changes made.""" + # Setup original values + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = True + + # Mock form fields with original values + mock_ip_input = Mock() + mock_ip_input.value = "127.0.0.1" + mock_hostname_input = Mock() + mock_hostname_input.value = "localhost" + mock_comment_input = Mock() + mock_comment_input.value = "" + mock_checkbox = Mock() + mock_checkbox.value = True + + def mock_query_side_effect(selector, widget_type=None): + if selector == "#ip-input": + return mock_ip_input + elif selector == "#hostname-input": + return mock_hostname_input + elif selector == "#comment-input": + return mock_comment_input + elif selector == "#active-checkbox": + return mock_checkbox + + mock_query_one.side_effect = mock_query_side_effect + + assert not app.has_entry_changes() + + @patch.object(HostsManagerApp, "query_one") + def test_has_entry_changes_ip_changed(self, mock_query_one, app): + """Test has_entry_changes returns True when IP address changed.""" + # Setup original values + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = True + + # Mock form fields with changed IP + mock_ip_input = Mock() + mock_ip_input.value = "192.168.1.1" # Changed IP + mock_hostname_input = Mock() + mock_hostname_input.value = "localhost" + mock_comment_input = Mock() + mock_comment_input.value = "" + mock_checkbox = Mock() + mock_checkbox.value = True + + def mock_query_side_effect(selector, widget_type=None): + if selector == "#ip-input": + return mock_ip_input + elif selector == "#hostname-input": + return mock_hostname_input + elif selector == "#comment-input": + return mock_comment_input + elif selector == "#active-checkbox": + return mock_checkbox + + mock_query_one.side_effect = mock_query_side_effect + + assert app.has_entry_changes() + + @patch.object(HostsManagerApp, "query_one") + def test_has_entry_changes_hostname_changed(self, mock_query_one, app): + """Test has_entry_changes returns True when hostname changed.""" + # Setup original values + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = True + + # Mock form fields with changed hostname + mock_ip_input = Mock() + mock_ip_input.value = "127.0.0.1" + mock_hostname_input = Mock() + mock_hostname_input.value = "localhost, test.local" # Added hostname + mock_comment_input = Mock() + mock_comment_input.value = "" + mock_checkbox = Mock() + mock_checkbox.value = True + + def mock_query_side_effect(selector, widget_type=None): + if selector == "#ip-input": + return mock_ip_input + elif selector == "#hostname-input": + return mock_hostname_input + elif selector == "#comment-input": + return mock_comment_input + elif selector == "#active-checkbox": + return mock_checkbox + + mock_query_one.side_effect = mock_query_side_effect + + assert app.has_entry_changes() + + @patch.object(HostsManagerApp, "query_one") + def test_has_entry_changes_comment_added(self, mock_query_one, app): + """Test has_entry_changes returns True when comment added.""" + # Setup original values + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = True + + # Mock form fields with added comment + mock_ip_input = Mock() + mock_ip_input.value = "127.0.0.1" + mock_hostname_input = Mock() + mock_hostname_input.value = "localhost" + mock_comment_input = Mock() + mock_comment_input.value = "Test comment" # Added comment + mock_checkbox = Mock() + mock_checkbox.value = True + + def mock_query_side_effect(selector, widget_type=None): + if selector == "#ip-input": + return mock_ip_input + elif selector == "#hostname-input": + return mock_hostname_input + elif selector == "#comment-input": + return mock_comment_input + elif selector == "#active-checkbox": + return mock_checkbox + + mock_query_one.side_effect = mock_query_side_effect + + assert app.has_entry_changes() + + @patch.object(HostsManagerApp, "query_one") + def test_has_entry_changes_active_state_changed(self, mock_query_one, app): + """Test has_entry_changes returns True when active state changed.""" + # Setup original values + app.original_entry_values = { + "ip_address": "127.0.0.1", + "hostnames": ["localhost"], + "comment": None, + "is_active": True, + } + app.entry_edit_mode = True + + # Mock form fields with changed active state + mock_ip_input = Mock() + mock_ip_input.value = "127.0.0.1" + mock_hostname_input = Mock() + mock_hostname_input.value = "localhost" + mock_comment_input = Mock() + mock_comment_input.value = "" + mock_checkbox = Mock() + mock_checkbox.value = False # Changed active state + + def mock_query_side_effect(selector, widget_type=None): + if selector == "#ip-input": + return mock_ip_input + elif selector == "#hostname-input": + return mock_hostname_input + elif selector == "#comment-input": + return mock_comment_input + elif selector == "#active-checkbox": + return mock_checkbox + + mock_query_one.side_effect = mock_query_side_effect + + assert app.has_entry_changes() + + def test_exit_edit_entry_mode(self, app): + """Test exit_edit_entry_mode cleans up properly.""" + app.entry_edit_mode = True + app.original_entry_values = {"test": "data"} + app.update_entry_details = Mock() + app.query_one = Mock() + app.update_status = Mock() + + mock_table = Mock() + app.query_one.return_value = mock_table + + app.exit_edit_entry_mode() + + assert not app.entry_edit_mode + assert app.original_entry_values is None + app.update_entry_details.assert_called_once() + mock_table.focus.assert_called_once() + app.update_status.assert_called_once_with("Exited entry edit mode")