review fixes 1

Author: dcog989

README.md

@@ -18,7 +18,7 @@ ContextPacker is a desktop application designed to scrape websites, clone Git re
   * **Smart Filtering:** Automatically respects **`.gitignore`** rules and allows hiding common binary and image files.
 * **Customisability:**
   * **Customisable Settings:** Configure scraping options (depth, paths, speed) and file exclusions.
-  * **External Configuration:** Key settings (`user_agents`, `default_local_excludes`, `binary_file_patterns`) can be modified in a **`config.json`** file created on first run.
+  * **External Configuration:** Key settings (`user_agents`, `default_local_excludes`, `binary_file_patterns`) can be modified in a **`settings.json`** file created on first run.
 * **Cross-Platform:** Supports Light and Dark themes (detects system theme on Windows, macOS, and Linux).
 
 -----
@@ -46,6 +46,25 @@ The application operates in two main modes, selected via radio buttons:
 
 -----
 
+## 🔧 Advanced Configuration (`settings.json`)
+
+The application creates a `settings.json` file on first run in the application data directory (e.g., `%APPDATA%\ContextPacker` on Windows). This file contains settings that are only read once on startup and are intended to be user-managed.
+
+| Key | Description | Default Value | Notes |
+| :--- | :--- | :--- | :--- |
+| `logging_level` | Sets the verbosity of the internal log output. | `"INFO"` | Options: `"DEBUG"`, `"INFO"`, `"WARNING"`, `"ERROR"`, `"CRITICAL"`. |
+| `log_max_size_mb` | Maximum size (in megabytes) of the `app.log` file before log rotation occurs. | `3` | |
+| `log_backup_count` | Number of backup log files to keep during rotation. | `5` | |
+| `user_agents` | A list of strings used by the web crawler to identify itself. | `[...]` | The application cycles through these. |
+| `default_output_format` | The default file extension selected in the Output panel. | `".md"` | Options: `".md"`, `".txt"`, `".xml"`. |
+| `default_local_excludes` | A list of global `fnmatch` patterns automatically applied to local directory scans. | `[".archive/", ".git/", ...]` | These are visible and editable in the 'Excludes' text area. |
+| `binary_file_patterns` | A list of `fnmatch` patterns that are considered binary/image files and can be toggled via the 'Hide Images + Binaries' checkbox. | `[*.png, *.jpg, ...]` | |
+| `max_age_cache_days` | The number of days after which old, temporary session and cache directories are automatically deleted on startup. | `7` | Set to a high number to keep all cache files indefinitely. |
+
+The file also contains window-state keys (`window_size`, `h_sash_state`, etc.) which are managed automatically by the application on close.
+
+-----
+
 ## ⚙️ Installation & Setup
 
 ### Requirements

core/actions.py

@@ -59,8 +59,10 @@ def clone_repo_worker(url, path, message_queue: queue.Queue, cancel_event: threa
 
     try:
         resolved_path = Path(path).resolve()
-        if not any(parent.name == "cache" for parent in resolved_path.parents):
-            message_queue.put(StatusMessage(status=StatusType.ERROR, message="Invalid clone path detected."))
+        app_cache_dir = Path(get_app_data_dir()) / "cache"
+        # Ensure the resolved path is a sub-path of the application's cache directory
+        if not resolved_path.is_relative_to(app_cache_dir):
+            message_queue.put(StatusMessage(status=StatusType.ERROR, message="Invalid clone path detected: Path is outside the application cache directory."))
             return
     except Exception:
         message_queue.put(StatusMessage(status=StatusType.ERROR, message="Invalid path provided."))
@@ -116,6 +118,8 @@ def clone_repo_worker(url, path, message_queue: queue.Queue, cancel_event: threa
         if process:
             if reader_thread and reader_thread.is_alive():
                 reader_thread.join(timeout=GIT_READER_THREAD_JOIN_TIMEOUT_SECONDS)
+                if reader_thread.is_alive():
+                    error_handler.log_message("Warning: Git output reader thread did not terminate in time.")
             error_handler.handle_process_cleanup(process)
             error_handler.handle_stream_cleanup(process)
 
@@ -125,16 +129,17 @@ def packaging_worker(source_dir, output_path, repomix_style, exclude_patterns, t
     logging.debug(f"Packaging worker started. Source: {source_dir}, Output: {output_path}")
 
     class RepomixProgressHandler(logging.Handler):
-        def __init__(self, msg_queue, total_files_count):
+        def __init__(self, msg_queue, total_files_count, cancel_event: threading.Event):
             super().__init__()
             self.msg_queue = msg_queue
             self.total_files = total_files_count
             self.processed_count = 0
+            self.cancel_event = cancel_event
             self.batch_size = REPOMIX_PROGRESS_UPDATE_BATCH_SIZE
             self.last_progress_value = -1
 
         def emit(self, record):
-            if cancel_event.is_set():
+            if self.cancel_event.is_set():
                 return
 
             msg = self.format(record)
@@ -150,10 +155,11 @@ def emit(self, record):
 
     repomix_logger = logging.getLogger("repomix")
     original_level = repomix_logger.level
-    progress_handler = RepomixProgressHandler(message_queue, total_files)
+    progress_handler = None
 
     try:
         repomix_logger.setLevel(logging.INFO)
+        progress_handler = RepomixProgressHandler(message_queue, total_files, cancel_event)
         repomix_logger.addHandler(progress_handler)
         run_repomix(
             source_dir,
@@ -164,7 +170,8 @@ def emit(self, record):
             exclude_patterns=exclude_patterns,
         )
     finally:
-        repomix_logger.removeHandler(progress_handler)
+        if progress_handler:
+            repomix_logger.removeHandler(progress_handler)
         repomix_logger.setLevel(original_level)
         logging.debug(f"Packaging worker finished for source: {source_dir}")
 

core/config_service.py

@@ -1,6 +1,7 @@
 import json
 import sys
 from pathlib import Path
+from typing import List, Optional
 
 
 class ConfigService:
@@ -66,6 +67,14 @@ def __init__(self):
         }
         self._config_dir = self._get_config_dir()
         self._config_path = self._config_dir / self._config_filename
+
+        # Separately store the mutable keys that the app changes
+        self._mutable_keys = ["window_size", "window_pos", "h_sash_state", "v_sash_state"]
+        # Separately store the user-editable keys that are not changed by the app at runtime
+        self._static_keys = list(set(self._default_config.keys()) - set(self._mutable_keys))
+
+        # Read/Write config. All keys are initialized from defaults, then overwritten by file.
+        # This dict holds the runtime values.
         self.config = self._load_config()
 
     def _get_config_dir(self):
@@ -78,35 +87,69 @@ def _get_config_dir(self):
 
     def _load_config(self):
         """Loads settings.json, creating a default one if it doesn't exist."""
+        # Use a copy of defaults for a foundation
+        config = self._default_config.copy()
+
         if not self._config_path.exists():
             try:
+                # Write only the static keys to the initial file, letting mutable keys use defaults
+                initial_config_content = {k: v for k, v in self._default_config.items() if k in self._static_keys}
                 with open(self._config_path, "w", encoding="utf-8") as f:
-                    json.dump(self._default_config, f, indent=4)
-                return self._default_config.copy()
+                    json.dump(initial_config_content, f, indent=4)
+                # The runtime config remains the full default set (config.copy())
+                return config
             except IOError as e:
                 print(f"Warning: Could not create default config file: {e}")
-                return self._default_config.copy()
+                return config.copy()
 
         try:
             with open(self._config_path, "r", encoding="utf-8") as f:
                 loaded_config = json.load(f)
-            # Merge loaded config with defaults to ensure all keys are present
-            config = self._default_config.copy()
+            # Merge loaded config with defaults.
+            # This ensures all keys are present and respects user's static config.
             config.update(loaded_config)
             return config
         except (json.JSONDecodeError, IOError) as e:
             print(f"Warning: Could not load config.json, using defaults: {e}")
-            return self._default_config.copy()
+            return config.copy()
 
     def get(self, key, default=None):
         """Gets a configuration value by key."""
         return self.config.get(key, default)
 
-    def save_config(self):
-        """Saves the current configuration dictionary to settings.json."""
+    def save_config(self, save_static=False):
+        """
+        Saves the current configuration dictionary to settings.json.
+
+        Args:
+            save_static (bool): If True, saves ALL keys (static and mutable).
+                                If False (default), only saves the mutable keys,
+                                merging them back into the existing file's content.
+        """
+        # 1. Read the existing file content to keep static settings unless told to overwrite
+        current_file_config = {}
+        if self._config_path.exists():
+            try:
+                with open(self._config_path, "r", encoding="utf-8") as f:
+                    current_file_config = json.load(f)
+            except (json.JSONDecodeError, IOError):
+                pass  # Use empty dict if file is corrupt/missing
+
+        # 2. Determine what to save
+        if save_static:
+            config_to_save = self.config.copy()
+        else:
+            # Only save the mutable keys from runtime config,
+            # and merge with the current file content to keep static keys.
+            config_to_save = current_file_config.copy()
+            for key in self._mutable_keys:
+                if key in self.config:
+                    config_to_save[key] = self.config[key]
+
+        # 3. Write the new content
         try:
             with open(self._config_path, "w", encoding="utf-8") as f:
-                json.dump(self.config, f, indent=4)
+                json.dump(config_to_save, f, indent=4)
         except IOError as e:
             print(f"Error: Could not save config file: {e}")
 
@@ -118,4 +161,6 @@ def save_window_state(self, size, pos, h_splitter_state, v_splitter_state):
         v_sash_qba = v_splitter_state.toBase64()
         self.config["h_sash_state"] = bytes(h_sash_qba.data()).decode("utf-8")
         self.config["v_sash_state"] = bytes(v_sash_qba.data()).decode("utf-8")
-        self.save_config()
+
+        # Only save mutable keys (window state)
+        self.save_config(save_static=False)

core/error_handling.py

@@ -63,10 +63,15 @@ def handle_process_cleanup(self, process, timeout: int = PROCESS_CLEANUP_TIMEOUT
                     self.log_message("Process terminated gracefully.")
                     return True
                 except subprocess.TimeoutExpired:
+                    # Graceful terminate failed, force kill
                     process.kill()
-                    process.wait(timeout=PROCESS_FORCE_KILL_WAIT_SECONDS)
-                    self.log_message("Process killed forcefully.")
-                    return True
+                    try:
+                        process.wait(timeout=PROCESS_FORCE_KILL_WAIT_SECONDS)
+                        self.log_message("Process killed forcefully.")
+                        return True
+                    except subprocess.TimeoutExpired:
+                        self.log_message("Process force-kill timed out. Process may be a zombie.")
+                        return False  # Indicate that cleanup failed
         except Exception as e:
             self.log_message(f"Warning during process cleanup: {e}")
             return False

ui/main_window.py

@@ -62,6 +62,7 @@ def __init__(self, config_service):
         self.hide_binaries_check: QCheckBox
         self.dir_level_ctrl: QSpinBox
         self.list_group: QGroupBox
+        self.list_stack_layout: QVBoxLayout
         self.standard_log_list: QTableWidget
         self.local_file_list: QTableWidget
         self.progress_gauge: QProgressBar