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.
2025-09-09 02:39:16 +02:00
parent f11f30ba74
commit a646b9d13a
4 changed files with 68 additions and 47 deletions
--- 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
--- 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)
--- 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
--- 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")