From a646b9d13aa24762702f678e9b9771e051f10d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A9sir=C3=A9=20Werner=20Menrath?= Date: Tue, 9 Sep 2025 02:39:16 +0200 Subject: [PATCH] fix(settings): Improve settings logic and fix UI bugs This commit addresses several bugs and improves the logic of the settings panels. - fix(settings): The "Reset to default settings" action no longer deletes the user-defined file/folder exclusion list. - fix(settings): Corrected a bug in the "Add to exclude list" function where new entries were not being written correctly. The logic is now more robust and prevents duplicate entries. - fix(ui): Fixed a TclError crash in Advanced Settings caused by mixing `grid` and `pack` geometry managers. - feat(settings): Implemented mutual exclusivity for the "trash bin" and "compression/encryption" checkboxes to prevent invalid configurations. - i18n: Improved and clarified the English text for the trash bin feature descriptions to enable better translation. --- core/pbp_app_config.py | 11 ++-- pyimage_ui/actions.py | 5 +- pyimage_ui/advanced_settings_frame.py | 79 ++++++++++++++++----------- pyimage_ui/settings_frame.py | 20 +++++-- 4 files changed, 68 insertions(+), 47 deletions(-) diff --git a/core/pbp_app_config.py b/core/pbp_app_config.py index 5f8f063..cca6b51 100644 --- a/core/pbp_app_config.py +++ b/core/pbp_app_config.py @@ -352,11 +352,12 @@ class Msg: "force_incremental_backup": _("Always force incremental backup (except first)"), "force_compression": _("Always compress backup"), "force_encryption": _("Always encrypt backup"), - "use_trash_bin": _("Papierkorb verwenden"), - "no_trash_bin": _("Kein Papierkorb (reine Synchronisierung)"), - "sync_mode_pure_sync": _("Synchronisierungsmodus: Reine Synchronisierung (Dateien werden gelöscht)"), - "sync_mode_trash_bin": _("Synchronisierungsmodus: Papierkorb verwenden (Gelöschte Dateien werden verschoben)"), - "sync_mode_no_delete": _("Synchronisierungsmodus: Keine Löschung (Dateien bleiben im Ziel)"), + "use_trash_bin": _("Archive outdated files in a trash bin"), + "no_trash_bin": _("Permanently delete outdated files (true sync)"), + "trash_bin_explanation": _("This setting only applies to User Backups, not System Backups. It controls how files that are deleted from your source (e.g., your Documents folder) are handled in the destination."), + "sync_mode_pure_sync": _("Sync Mode: Mirror. Files deleted from the source will also be permanently deleted from the backup."), + "sync_mode_trash_bin": _("Sync Mode: Archive. Files deleted from the source will be moved to a trash folder in the backup."), + "sync_mode_no_delete": _("Sync Mode: Additive. Files deleted from the source will be kept in the backup."), "encryption_note_system_backup": _("Note: For system backups, encryption only applies to files directly within the /home directory. Folders are not automatically encrypted unless explicitly included in the backup."), "keyfile_settings": _("Keyfile Settings"), # New "backup_defaults_title": _("Backup Defaults"), # New diff --git a/pyimage_ui/actions.py b/pyimage_ui/actions.py index 4e468c7..dd03f62 100644 --- a/pyimage_ui/actions.py +++ b/pyimage_ui/actions.py @@ -405,10 +405,7 @@ class Actions: title=Msg.STR["error"], text=Msg.STR["path_not_found"].format(path=path)).show() def reset_to_default_settings(self): - try: - AppConfig.create_default_user_excludes() - except OSError as e: - app_logger.log(f"Error creating default user exclude list: {e}") + self.app.config_manager.set_setting("backup_destination_path", None) self.app.config_manager.set_setting("restore_source_path", None) diff --git a/pyimage_ui/advanced_settings_frame.py b/pyimage_ui/advanced_settings_frame.py index 4cca5b1..50a02da 100644 --- a/pyimage_ui/advanced_settings_frame.py +++ b/pyimage_ui/advanced_settings_frame.py @@ -29,12 +29,11 @@ class AdvancedSettingsFrame(ttk.Frame): self.nav_buttons_defs = [ (Msg.STR["system_excludes"], lambda: self._switch_view(0)), (Msg.STR["manual_excludes"], lambda: self._switch_view(1)), - # New button for Keyfile/Automation (Msg.STR["keyfile_settings"], lambda: self._switch_view(2)), (Msg.STR["animation_settings_title"], - lambda: self._switch_view(3)), # Animation settings + lambda: self._switch_view(3)), (Msg.STR["backup_defaults_title"], - lambda: self._switch_view(4)), # Backup Defaults + lambda: self._switch_view(4)), ] self.nav_buttons = [] @@ -130,23 +129,27 @@ class AdvancedSettingsFrame(ttk.Frame): self.force_full_var, self.force_incremental_var, self.force_full_var.get())).pack(anchor=tk.W) ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["force_incremental_backup"], variable=self.force_incremental_var, command=lambda: enforce_backup_type_exclusivity( self.force_incremental_var, self.force_full_var, self.force_incremental_var.get())).pack(anchor=tk.W) - ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["force_compression"], - variable=self.force_compression_var).pack(anchor=tk.W) - ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["force_encryption"], - variable=self.force_encryption_var).pack(anchor=tk.W) - ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["use_trash_bin"], - variable=self.use_trash_bin_var).pack(anchor=tk.W) - ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["no_trash_bin"], - variable=self.no_trash_bin_var).pack(anchor=tk.W) + ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["force_compression"], variable=self.force_compression_var, command=lambda: enforce_backup_type_exclusivity( + self.force_compression_var, self.force_encryption_var, self.force_compression_var.get())).pack(anchor=tk.W) + ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["force_encryption"], variable=self.force_encryption_var, command=lambda: enforce_backup_type_exclusivity( + self.force_encryption_var, self.force_compression_var, self.force_encryption_var.get())).pack(anchor=tk.W) + + ttk.Separator(self.backup_defaults_frame, orient=tk.HORIZONTAL).pack(fill=tk.X, pady=10) - ttk.Separator(self.backup_defaults_frame, orient=tk.HORIZONTAL).pack( - fill=tk.X, pady=5) + trash_info_label = ttk.Label(self.backup_defaults_frame, text=Msg.STR["trash_bin_explanation"], wraplength=750, justify="left") + trash_info_label.pack(anchor=tk.W, pady=5) + + ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["use_trash_bin"], variable=self.use_trash_bin_var, command=lambda: self._handle_trash_checkbox_click( + self.use_trash_bin_var, self.no_trash_bin_var)).pack(anchor=tk.W) + ttk.Checkbutton(self.backup_defaults_frame, text=Msg.STR["no_trash_bin"], variable=self.no_trash_bin_var, command=lambda: self._handle_trash_checkbox_click( + self.no_trash_bin_var, self.use_trash_bin_var)).pack(anchor=tk.W) + + ttk.Separator(self.backup_defaults_frame, orient=tk.HORIZONTAL).pack(fill=tk.X, pady=10) encryption_note = ttk.Label( self.backup_defaults_frame, text=Msg.STR["encryption_note_system_backup"], wraplength=750, justify="left") encryption_note.pack(anchor=tk.W, pady=5) - # --- Keyfile/Automation Settings --- self.keyfile_settings_frame = ttk.LabelFrame( view_container, text=Msg.STR["automation_settings_title"], padding=10) @@ -162,7 +165,6 @@ class AdvancedSettingsFrame(ttk.Frame): sudoers_info_text = (f"To run automated backups, an administrator must create a file in /etc/sudoers.d/\n" f"with the following content (replace 'punix' with the correct username):\n" - # Path needs to be updated f"punix ALL=(ALL) NOPASSWD: /path/to/pybackup-cli.py") sudoers_info_label = ttk.Label( self.keyfile_settings_frame, text=sudoers_info_text, justify="left") @@ -172,7 +174,6 @@ class AdvancedSettingsFrame(ttk.Frame): self.keyfile_settings_frame.columnconfigure(1, weight=1) - # --- Action Buttons --- button_frame = ttk.Frame(self) button_frame.pack(pady=10) @@ -181,16 +182,10 @@ class AdvancedSettingsFrame(ttk.Frame): ttk.Button(button_frame, text=Msg.STR["cancel"], command=self._cancel_changes).pack( side=tk.LEFT, padx=5) - # Initial packing of frames (all hidden except the first one by _switch_view) - # Initially packed, then hidden by _switch_view self.tree_frame.pack(fill=tk.BOTH, expand=True) - # Initially packed, then hidden by _switch_view self.manual_excludes_frame.pack(fill=tk.BOTH, expand=True) - # Initially packed, then hidden by _switch_view self.keyfile_settings_frame.pack(fill=tk.BOTH, expand=True) - # Initially packed, then hidden by _switch_view self.animation_settings_frame.pack(fill=tk.BOTH, expand=True) - # Initially packed, then hidden by _switch_view self.backup_defaults_frame.pack(fill=tk.BOTH, expand=True) self._load_system_folders() @@ -201,6 +196,27 @@ class AdvancedSettingsFrame(ttk.Frame): self._switch_view(self.current_view_index) + def _handle_trash_checkbox_click(self, changed_var, other_var): + enforce_backup_type_exclusivity(changed_var, other_var, changed_var.get()) + self._on_trash_setting_change() + + def _on_trash_setting_change(self): + use_trash = self.use_trash_bin_var.get() + no_trash = self.no_trash_bin_var.get() + + if no_trash: + self.app_instance.sync_mode_label.config( + text=Msg.STR["sync_mode_pure_sync"], foreground="red") + elif use_trash: + self.app_instance.sync_mode_label.config( + text=Msg.STR["sync_mode_trash_bin"], foreground="orange") + else: + self.app_instance.sync_mode_label.config( + text=Msg.STR["sync_mode_no_delete"], foreground="green") + + self.config_manager.set_setting("use_trash_bin", use_trash) + self.config_manager.set_setting("no_trash_bin", no_trash) + def _create_key_file(self): if not self.app_instance.destination_path: MessageDialog(self, message_type="error", title="Error", @@ -215,13 +231,12 @@ class AdvancedSettingsFrame(ttk.Frame): text="No encrypted container found at the destination.") return - # Prompt for the existing password to authorize adding a new key password_dialog = PasswordDialog( self, title="Enter Existing Password", confirm=False) password, _ = password_dialog.get_password() if not password: - return # User cancelled + return key_file_path = self.app_instance.backup_manager.encryption_manager.create_and_add_key_file( self.app_instance.destination_path, password) @@ -253,14 +268,12 @@ class AdvancedSettingsFrame(ttk.Frame): self.current_view_index = index self.update_nav_buttons(index) - # Hide all frames first self.tree_frame.pack_forget() self.manual_excludes_frame.pack_forget() self.keyfile_settings_frame.pack_forget() self.animation_settings_frame.pack_forget() self.backup_defaults_frame.pack_forget() - # Show the selected frame and update info label if index == 0: self.tree_frame.pack(fill=tk.BOTH, expand=True) self.info_label.config(text=Msg.STR["advanced_settings_warning"]) @@ -269,7 +282,6 @@ class AdvancedSettingsFrame(ttk.Frame): self.info_label.config(text=Msg.STR["manual_excludes_info"]) elif index == 2: self.keyfile_settings_frame.pack(fill=tk.BOTH, expand=True) - # Use automation title for info self.info_label.config(text="") elif index == 3: self.animation_settings_frame.pack(fill=tk.BOTH, expand=True) @@ -325,6 +337,10 @@ class AdvancedSettingsFrame(ttk.Frame): self.config_manager.get_setting("force_compression", False)) self.force_encryption_var.set( self.config_manager.get_setting("force_encryption", False)) + self.use_trash_bin_var.set( + self.config_manager.get_setting("use_trash_bin", False)) + self.no_trash_bin_var.set( + self.config_manager.get_setting("no_trash_bin", False)) def _load_animation_settings(self): backup_anim = self.config_manager.get_setting( @@ -416,6 +432,8 @@ class AdvancedSettingsFrame(ttk.Frame): "force_compression", self.force_compression_var.get()) self.config_manager.set_setting( "force_encryption", self.force_encryption_var.get()) + self.config_manager.set_setting("use_trash_bin", self.use_trash_bin_var.get()) + self.config_manager.set_setting("no_trash_bin", self.no_trash_bin_var.get()) if self.app_instance: self.app_instance.update_backup_options_from_config() @@ -431,8 +449,7 @@ class AdvancedSettingsFrame(ttk.Frame): self.app_instance.animated_icon = AnimatedIcon( self.app_instance.action_frame, width=20, height=20, use_pillow=True, bg=bg_color, animation_type=initial_animation_type) - self.app_instance.animated_icon.pack( - side=tk.LEFT, padx=5, before=self.app_instance.task_progress) + self.app_instance.animated_icon.grid(row=0, column=0, rowspan=2, padx=5) self.app_instance.animated_icon.stop("DISABLE") self.app_instance.animated_icon.animation_type = backup_animation_type @@ -479,7 +496,6 @@ class AdvancedSettingsFrame(ttk.Frame): for item in self.manual_excludes_listbox.get(0, tk.END): f.write(f"{item}\n") - # Instead of destroying the Toplevel, hide this frame and show main settings self.pack_forget() if self.show_main_settings_callback: self.show_main_settings_callback() @@ -491,7 +507,6 @@ class AdvancedSettingsFrame(ttk.Frame): current_source) def _cancel_changes(self): - # Hide this frame and show main settings without applying changes self.pack_forget() if self.show_main_settings_callback: self.show_main_settings_callback() @@ -514,4 +529,4 @@ class AdvancedSettingsFrame(ttk.Frame): user_patterns.extend( [line.strip() for line in f if line.strip() and not line.startswith('#')]) - return generated_patterns, user_patterns + return generated_patterns, user_patterns \ No newline at end of file diff --git a/pyimage_ui/settings_frame.py b/pyimage_ui/settings_frame.py index f15d7b6..55302bb 100644 --- a/pyimage_ui/settings_frame.py +++ b/pyimage_ui/settings_frame.py @@ -117,14 +117,22 @@ class SettingsFrame(ttk.Frame): dialog.destroy() if path: - with open(AppConfig.MANUAL_EXCLUDE_LIST_PATH, 'a') as f: - if os.path.isdir(path): - f.write(f"\n{path}/*") - else: - f.write(f"\n{path}") + try: + with open(AppConfig.MANUAL_EXCLUDE_LIST_PATH, 'r') as f: + lines = {line.strip() for line in f if line.strip()} + except FileNotFoundError: + lines = set() + + new_entry = f"{path}/*" if os.path.isdir(path) else path + lines.add(new_entry) + + with open(AppConfig.MANUAL_EXCLUDE_LIST_PATH, 'w') as f: + for line in sorted(list(lines)): + f.write(f"{line}\n") self.load_and_display_excludes() - self._load_hidden_files() + if hasattr(self, 'advanced_settings_frame_instance') and self.advanced_settings_frame_instance: + self.advanced_settings_frame_instance._load_manual_excludes() def show(self): self.grid(row=2, column=0, sticky="nsew")