From 0ae918f9abe867a54a38b3d5137f282c9707cdf6 Mon Sep 17 00:00:00 2001 From: Li <175231748+linkmodo@users.noreply.github.com> Date: Tue, 7 Jul 2026 19:19:57 -0400 Subject: [PATCH 1/2] Prompt before overwriting existing files on export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Export (single) and Export All silently overwrote files that already existed at the destination, with no warning. Now the "Overwrite existing files" option gates this, and the default is safe: - Unchecked (new default): if a batch would clobber existing files, the export is gated on a prompt — Overwrite / Rename / Cancel — listing the conflicting names. Rename falls back to the worker's numbered-suffix auto-rename; Cancel aborts the whole export. Same behavior for single Export and Export All (all route through _run_export_tasks). - Checked: overwrite silently, no prompt (for both Export and Export All). - The prompt carries an "Always overwrite without asking" checkbox that, when ticked with Overwrite, persists the preference (turns the Export-panel option on and the sticky default), so it stays on until turned off. The overwrite default flips True -> False so the warning is on by default; a saved "checked" preference keeps overwriting silently as before. Implementation: - Factor the worker's output-path logic into shared, testable helpers (resolve_export_dir / resolve_export_naming / resolve_export_target_path / find_export_conflicts) so conflict detection resolves the exact path the worker writes, and refactor run_batch to reuse them. - AppController._resolve_export_conflicts runs before dispatch; _set_overwrite_ preference persists the remembered choice without touching edit history. Contact-sheet export is unchanged (already auto-renames, no fixed per-frame names). Tests: path resolution + conflict detection, and controller wiring for the silent preference, overwrite / rename / cancel, remember-persist, and no-conflict pass. Co-Authored-By: Claude Opus 4.8 --- negpy/desktop/controller.py | 85 ++++++++++++++++++- .../view/widgets/export_settings_form.py | 4 + negpy/desktop/workers/export.py | 66 +++++++++----- negpy/domain/models.py | 6 +- tests/test_controller.py | 78 ++++++++++++++++- tests/test_export_overwrite.py | 68 +++++++++++++++ 6 files changed, 279 insertions(+), 28 deletions(-) create mode 100644 tests/test_export_overwrite.py diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 337cc539..5b2f23b5 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -6,11 +6,11 @@ import numpy as np from PyQt6.QtCore import Q_ARG, QMetaObject, QObject, Qt, QThread, QTimer, pyqtSignal from PyQt6.QtGui import QIcon, QPixmap -from PyQt6.QtWidgets import QMessageBox +from PyQt6.QtWidgets import QCheckBox, QMessageBox from negpy.desktop.converters import ImageConverter from negpy.desktop.session import AppState, DesktopSessionManager, ToolMode, resolve_asset_rgbscan -from negpy.desktop.workers.export import ExportTask, ExportWorker +from negpy.desktop.workers.export import ExportTask, ExportWorker, find_export_conflicts from negpy.desktop.workers.render import ( AssetDiscoveryTask, AssetDiscoveryWorker, @@ -1798,6 +1798,7 @@ def export_edit_sidecars(self) -> None: self.set_status(f"Wrote {written} edit sidecar(s)", 4000) def _run_export_tasks(self, tasks: List[ExportTask]) -> None: + # Reject unencodable format/colour-space pairings before anything else. blocked = [t for t in tasks if export_blocked(t.export_settings.export_fmt, t.export_settings.export_color_space)] if blocked: names = ", ".join(sorted({t.file_info.get("name", "?") for t in blocked})[:5]) @@ -1809,6 +1810,11 @@ def _run_export_tasks(self, tasks: List[ExportTask]) -> None: ) return + # Then confirm any overwrites before dispatching to the worker. + tasks = self._resolve_export_conflicts(tasks) + if not tasks: + return + self._export_start_time = time.time() self._export_failures = 0 self._begin_batch("Exporting", abortable=True) @@ -1819,6 +1825,81 @@ def _run_export_tasks(self, tasks: List[ExportTask]) -> None: Q_ARG(list, tasks), ) + def _resolve_export_conflicts(self, tasks: List[ExportTask]) -> Optional[List[ExportTask]]: + """Decide how to handle existing destination files before dispatching an export. + + If the "Overwrite existing files" preference is on, overwrite silently (no prompt) + — for single Export and Export All alike. Otherwise, if the batch would clobber + existing files, prompt (Overwrite / Rename / Cancel); the dialog's "always + overwrite without asking" toggle persists the preference. Returns the tasks to run + (overwrite flag set to the chosen action) or None to cancel the whole export.""" + if not tasks: + return tasks + + if self.state.config.export.overwrite: + return [replace(t, export_settings=replace(t.export_settings, overwrite=True)) for t in tasks] + + conflicts = find_export_conflicts(tasks) + if not conflicts: + return tasks + + choice, remember = self._prompt_overwrite_conflicts(conflicts) + if choice is None: + return None + if remember and choice: + self._set_overwrite_preference(True) + return [replace(t, export_settings=replace(t.export_settings, overwrite=choice)) for t in tasks] + + def _set_overwrite_preference(self, value: bool) -> None: + """Persist the global 'Overwrite existing files' preference (syncs the Export tab + checkbox and the sticky default) without touching edit history or re-rendering.""" + cfg = self.state.config + if bool(cfg.export.overwrite) == value: + return + new_config = replace(cfg, export=replace(cfg.export, overwrite=value)) + self.session.update_config(new_config, persist=True, render=True, record_history=False) + + @staticmethod + def _prompt_overwrite_conflicts(conflicts: List[str]) -> tuple[Optional[bool], bool]: + """Ask how to handle existing destination files. Returns (choice, remember): + choice is True (overwrite), False (rename with a numbered suffix) or None (cancel); + remember is whether the user asked to always overwrite without being asked again.""" + n = len(conflicts) + names = "\n".join(" • " + os.path.basename(p) for p in conflicts[:8]) + if n > 8: + names += f"\n … and {n - 8} more" + + box = QMessageBox() + box.setIcon(QMessageBox.Icon.Warning) + if n == 1: + box.setWindowTitle("File already exists") + box.setText(f"“{os.path.basename(conflicts[0])}” already exists in the export folder.") + else: + box.setWindowTitle("Files already exist") + box.setText(f"{n} files already exist in the export destination.") + box.setInformativeText(f"{names}\n\nOverwrite, save with a new name, or cancel?") + + remember_check = QCheckBox("Always overwrite without asking") + remember_check.setToolTip("Turns on the Export panel's “Overwrite existing files” option; stays on until you turn it off.") + box.setCheckBox(remember_check) + + overwrite_label = "Overwrite" if n == 1 else "Overwrite All" + rename_label = "Rename" if n == 1 else "Rename All" + overwrite_btn = box.addButton(overwrite_label, QMessageBox.ButtonRole.DestructiveRole) + rename_btn = box.addButton(rename_label, QMessageBox.ButtonRole.AcceptRole) + cancel_btn = box.addButton(QMessageBox.StandardButton.Cancel) + box.setDefaultButton(rename_btn) + box.setEscapeButton(cancel_btn) + box.exec() + + clicked = box.clickedButton() + remember = remember_check.isChecked() + if clicked is overwrite_btn: + return True, remember + if clicked is rename_btn: + return False, remember + return None, False + def _on_render_finished(self, _result: Any, metrics: Dict[str, Any]) -> None: self._is_rendering = False diff --git a/negpy/desktop/view/widgets/export_settings_form.py b/negpy/desktop/view/widgets/export_settings_form.py index 59e8d90a..8d3a5724 100644 --- a/negpy/desktop/view/widgets/export_settings_form.py +++ b/negpy/desktop/view/widgets/export_settings_form.py @@ -324,6 +324,10 @@ def _build_destination(self, root: QVBoxLayout) -> None: root.addLayout(filename_row) self.overwrite_check = QCheckBox("Overwrite existing files") + self.overwrite_check.setToolTip( + "Checked: replace files that already exist, without asking. " + "Unchecked: ask before overwriting (Overwrite / Rename / Cancel) when a file already exists." + ) self.overwrite_check.stateChanged.connect(self._on_changed) root.addWidget(self.overwrite_check) diff --git a/negpy/desktop/workers/export.py b/negpy/desktop/workers/export.py index 7f21520c..62563337 100644 --- a/negpy/desktop/workers/export.py +++ b/negpy/desktop/workers/export.py @@ -39,6 +39,48 @@ def _same_decode_source(a: ExportTask, b: ExportTask) -> bool: ) +_EXT = { + ExportFormat.JPEG: "jpg", + ExportFormat.TIFF: "tiff", + ExportFormat.PNG: "png", + ExportFormat.DNG: "dng", + ExportFormat.JXL: "jxl", + ExportFormat.WEBP: "webp", +} + + +def resolve_export_dir(task: ExportTask) -> str: + """Destination folder for a task, per its output-mode rule.""" + source_dir = os.path.dirname(task.file_info["path"]) + output_mode = task.export_settings.output_mode + if output_mode == ExportPresetOutputMode.SUBFOLDER_OF_SOURCE: + subfolder = task.export_settings.output_subfolder or "" + return os.path.join(source_dir, subfolder) if subfolder else source_dir + if output_mode == ExportPresetOutputMode.ABSOLUTE: + return task.export_settings.output_path or source_dir + return source_dir + + +def resolve_export_naming(task: ExportTask) -> tuple[str, str, str]: + """(out_dir, filename-stem, extension) for a task — the shared source of truth for + both conflict detection and the actual write, so they can never disagree.""" + out_dir = resolve_export_dir(task) + ext = _EXT.get(task.export_settings.export_fmt, "jpg") + filename = render_export_filename(task.file_info["path"], task.export_settings, border_size=task.params.finish.border_size) + return out_dir, filename, ext + + +def resolve_export_target_path(task: ExportTask) -> str: + """The path a task writes to before any overwrite/rename resolution.""" + out_dir, filename, ext = resolve_export_naming(task) + return os.path.join(out_dir, f"{filename}.{ext}") + + +def find_export_conflicts(tasks: List[ExportTask]) -> List[str]: + """Target paths for the batch that already exist on disk (would be overwritten).""" + return [path for task in tasks if os.path.exists(path := resolve_export_target_path(task))] + + class ExportWorker(QObject): """ Background batch export orchestrator. @@ -101,30 +143,8 @@ def run_batch(self, tasks: List[ExportTask]) -> None: ): bits = embed_metadata(bits, task.metadata_config, task.source_exif) - source_dir = os.path.dirname(task.file_info["path"]) - output_mode = task.export_settings.output_mode - if output_mode == ExportPresetOutputMode.SUBFOLDER_OF_SOURCE: - subfolder = task.export_settings.output_subfolder or "" - out_dir = os.path.join(source_dir, subfolder) if subfolder else source_dir - elif output_mode == ExportPresetOutputMode.ABSOLUTE: - out_dir = task.export_settings.output_path or source_dir - else: - out_dir = source_dir + out_dir, filename, ext = resolve_export_naming(task) os.makedirs(out_dir, exist_ok=True) - - _EXT = { - ExportFormat.JPEG: "jpg", - ExportFormat.TIFF: "tiff", - ExportFormat.PNG: "png", - ExportFormat.DNG: "dng", - ExportFormat.JXL: "jxl", - ExportFormat.WEBP: "webp", - } - ext = _EXT.get(task.export_settings.export_fmt, "jpg") - - filename = render_export_filename( - task.file_info["path"], task.export_settings, border_size=task.params.finish.border_size - ) path = os.path.join(out_dir, f"{filename}.{ext}") if not task.export_settings.overwrite: diff --git a/negpy/domain/models.py b/negpy/domain/models.py index 851feb19..b7214ebd 100644 --- a/negpy/domain/models.py +++ b/negpy/domain/models.py @@ -136,7 +136,9 @@ class ExportConfig: export_resolution_mode: str = ExportResolutionMode.ORIGINAL.value export_target_long_edge_px: int = 2000 filename_pattern: str = "{{ original_name }}" - overwrite: bool = True + # When True, exports silently overwrite existing files; when False, the export + # prompts (Overwrite / Rename / Cancel) before clobbering anything. + overwrite: bool = False output_mode: str = ExportPresetOutputMode.ABSOLUTE output_subfolder: str = "" icc_input_path: Optional[str] = None @@ -189,7 +191,7 @@ class ExportPreset: output_mode: str = ExportPresetOutputMode.SAME_AS_SOURCE output_subfolder: str = "" output_path: str = "" - overwrite: bool = True + overwrite: bool = False filename_pattern: str = "{{ original_name }}" # Color diff --git a/tests/test_controller.py b/tests/test_controller.py index b3be54d0..7b3e187c 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -1,4 +1,6 @@ +import os import sys +import tempfile import unittest from unittest.mock import MagicMock, patch from dataclasses import replace @@ -7,7 +9,8 @@ from negpy.desktop.controller import AppController from negpy.desktop.session import DesktopSessionManager, AppState, ToolMode -from negpy.domain.models import ExportConfig, ExportFormat, ExportPreset, ExportPresetOutputMode +from negpy.desktop.workers.export import ExportTask, resolve_export_target_path +from negpy.domain.models import ExportConfig, ExportFormat, ExportPreset, ExportPresetOutputMode, WorkspaceConfig from negpy.services.rendering.preview_manager import PreviewManager if not QApplication.instance(): @@ -198,6 +201,79 @@ def test_reset_crop_disables_auto_crop_and_clears_manual_rect(self): self.assertIsNone(saved_config.geometry.manual_crop_rect) self.controller.request_render.assert_called_once_with() + def _export_task(self, path, overwrite=False): + preset = ExportPreset( + name="t", + output_mode=ExportPresetOutputMode.ABSOLUTE, + output_path=os.path.dirname(path), + overwrite=overwrite, + ) + return ExportTask( + file_info={"name": os.path.basename(path), "path": path, "hash": "h"}, + params=WorkspaceConfig(), + export_settings=preset, + ) + + def _set_export_overwrite(self, value): + cfg = self.controller.state.config + self.controller.state.config = replace(cfg, export=replace(cfg.export, overwrite=value)) + + def test_export_overwrite_pref_on_skips_prompt_and_overwrites(self): + self._set_export_overwrite(True) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) + open(resolve_export_target_path(task), "wb").close() + self.controller._prompt_overwrite_conflicts = MagicMock() + out = self.controller._resolve_export_conflicts([task]) + self.assertTrue(out[0].export_settings.overwrite) + self.controller._prompt_overwrite_conflicts.assert_not_called() + + def test_export_conflict_overwrite_sets_flag_true(self): + self._set_export_overwrite(False) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) + open(resolve_export_target_path(task), "wb").close() + self.controller._prompt_overwrite_conflicts = MagicMock(return_value=(True, False)) + out = self.controller._resolve_export_conflicts([task]) + self.assertEqual(len(out), 1) + self.assertTrue(out[0].export_settings.overwrite) + + def test_export_conflict_rename_sets_flag_false(self): + self._set_export_overwrite(False) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) + open(resolve_export_target_path(task), "wb").close() + self.controller._prompt_overwrite_conflicts = MagicMock(return_value=(False, False)) + out = self.controller._resolve_export_conflicts([task]) + self.assertFalse(out[0].export_settings.overwrite) + + def test_export_conflict_cancel_returns_none(self): + self._set_export_overwrite(False) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) + open(resolve_export_target_path(task), "wb").close() + self.controller._prompt_overwrite_conflicts = MagicMock(return_value=(None, False)) + self.assertIsNone(self.controller._resolve_export_conflicts([task])) + + def test_export_conflict_remember_persists_preference(self): + self._set_export_overwrite(False) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) + open(resolve_export_target_path(task), "wb").close() + self.controller._prompt_overwrite_conflicts = MagicMock(return_value=(True, True)) + self.controller._set_overwrite_preference = MagicMock() + self.controller._resolve_export_conflicts([task]) + self.controller._set_overwrite_preference.assert_called_once_with(True) + + def test_export_no_conflict_passes_through_without_prompt(self): + self._set_export_overwrite(False) + with tempfile.TemporaryDirectory() as d: + task = self._export_task(os.path.join(d, "A.RAF")) # target not created + self.controller._prompt_overwrite_conflicts = MagicMock() + out = self.controller._resolve_export_conflicts([task]) + self.assertEqual(out, [task]) + self.controller._prompt_overwrite_conflicts.assert_not_called() + def test_manual_crop_rect_changed_disables_auto_crop(self): geometry = replace(self.controller.state.config.geometry, auto_crop_enabled=True) self.controller.state.config = replace(self.controller.state.config, geometry=geometry) diff --git a/tests/test_export_overwrite.py b/tests/test_export_overwrite.py new file mode 100644 index 00000000..27770589 --- /dev/null +++ b/tests/test_export_overwrite.py @@ -0,0 +1,68 @@ +import os +import tempfile +import unittest + +from negpy.desktop.workers.export import ( + ExportTask, + find_export_conflicts, + resolve_export_dir, + resolve_export_naming, + resolve_export_target_path, +) +from negpy.domain.models import ExportFormat, ExportPreset, ExportPresetOutputMode, WorkspaceConfig + + +def _task(src_path, *, out_mode, out_path="", subfolder="", fmt=ExportFormat.JPEG, overwrite=True): + preset = ExportPreset( + name="t", + export_fmt=fmt, + output_mode=out_mode, + output_path=out_path, + output_subfolder=subfolder, + overwrite=overwrite, + ) + return ExportTask( + file_info={"name": os.path.basename(src_path), "path": src_path, "hash": "h"}, + params=WorkspaceConfig(), + export_settings=preset, + ) + + +class TestExportPathResolution(unittest.TestCase): + def test_dir_same_as_source(self): + t = _task(os.path.join("photos", "roll", "IMG_1.RAF"), out_mode=ExportPresetOutputMode.SAME_AS_SOURCE) + self.assertEqual(resolve_export_dir(t), os.path.join("photos", "roll")) + + def test_dir_subfolder_of_source(self): + t = _task(os.path.join("photos", "roll", "IMG_1.RAF"), out_mode=ExportPresetOutputMode.SUBFOLDER_OF_SOURCE, subfolder="JPEG") + self.assertEqual(resolve_export_dir(t), os.path.join("photos", "roll", "JPEG")) + + def test_dir_absolute(self): + t = _task(os.path.join("photos", "roll", "IMG_1.RAF"), out_mode=ExportPresetOutputMode.ABSOLUTE, out_path="out") + self.assertEqual(resolve_export_dir(t), "out") + + def test_naming_uses_format_ext_and_original_name(self): + t = _task(os.path.join("d", "IMG_1.RAF"), out_mode=ExportPresetOutputMode.ABSOLUTE, out_path="out", fmt=ExportFormat.TIFF) + out_dir, filename, ext = resolve_export_naming(t) + self.assertEqual((out_dir, filename, ext), ("out", "IMG_1", "tiff")) + self.assertEqual(resolve_export_target_path(t), os.path.join("out", "IMG_1.tiff")) + + +class TestFindExportConflicts(unittest.TestCase): + def test_detects_only_existing_targets(self): + with tempfile.TemporaryDirectory() as d: + t1 = _task(os.path.join(d, "A.RAF"), out_mode=ExportPresetOutputMode.ABSOLUTE, out_path=d) + t2 = _task(os.path.join(d, "B.RAF"), out_mode=ExportPresetOutputMode.ABSOLUTE, out_path=d) + # Pre-create only t1's target on disk. + with open(resolve_export_target_path(t1), "wb"): + pass + self.assertEqual(find_export_conflicts([t1, t2]), [resolve_export_target_path(t1)]) + + def test_no_conflicts_when_none_exist(self): + with tempfile.TemporaryDirectory() as d: + t = _task(os.path.join(d, "A.RAF"), out_mode=ExportPresetOutputMode.ABSOLUTE, out_path=d) + self.assertEqual(find_export_conflicts([t]), []) + + +if __name__ == "__main__": + unittest.main() From e2815b22a6e79b61ad28a5ef3df02f3dc9ff03ae Mon Sep 17 00:00:00 2001 From: Li <175231748+linkmodo@users.noreply.github.com> Date: Tue, 7 Jul 2026 19:51:51 -0400 Subject: [PATCH 2/2] Make checkbox indicators visible on the dark theme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fusion's default checkbox indicator is nearly invisible against the app's #0D0D0D background — the unchecked "Overwrite existing files" box (and every other checkbox) blended in. Add a QCheckBox::indicator rule: a clear border and subtle fill when unchecked (brighter on hover), and an accent box with a white checkmark when checked. The checkmark is an SVG (media/icons/checkbox_check.svg); its absolute path is baked into the stylesheet at load time because QSS url() can't resolve relative paths reliably across dev and frozen runs. media/ is already bundled by build.py. Co-Authored-By: Claude Opus 4.8 --- media/icons/checkbox_check.svg | 3 ++ negpy/desktop/main.py | 7 ++++- negpy/desktop/view/styles/modern_dark.qss | 37 +++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 media/icons/checkbox_check.svg diff --git a/media/icons/checkbox_check.svg b/media/icons/checkbox_check.svg new file mode 100644 index 00000000..15d55070 --- /dev/null +++ b/media/icons/checkbox_check.svg @@ -0,0 +1,3 @@ + + + diff --git a/negpy/desktop/main.py b/negpy/desktop/main.py index 9d7d4e6c..0a2d66d9 100644 --- a/negpy/desktop/main.py +++ b/negpy/desktop/main.py @@ -107,7 +107,12 @@ def main() -> None: qss_path = get_resource_path("negpy/desktop/view/styles/modern_dark.qss") if os.path.exists(qss_path): with open(qss_path, "r", encoding="utf-8") as f: - app.setStyleSheet(f.read()) + qss = f.read() + # QSS url() can't resolve relative paths reliably across dev/frozen runs, + # so bake in the absolute icon path (forward slashes, Qt-friendly). + check_icon = get_resource_path("media/icons/checkbox_check.svg").replace("\\", "/") + qss = qss.replace("__CHECKBOX_CHECK_ICON__", check_icon) + app.setStyleSheet(qss) session_manager = DesktopSessionManager(repo) controller = AppController(session_manager) diff --git a/negpy/desktop/view/styles/modern_dark.qss b/negpy/desktop/view/styles/modern_dark.qss index c1bacf67..a77db807 100644 --- a/negpy/desktop/view/styles/modern_dark.qss +++ b/negpy/desktop/view/styles/modern_dark.qss @@ -335,3 +335,40 @@ QToolButton#split_menu_btn::menu-arrow { width: 0px; height: 0px; } + +/* Checkboxes — Fusion's default indicator is almost invisible on this dark + background. Give the unchecked box a clear border + subtle fill, and a filled + accent when checked, so its state reads at a glance. */ +QCheckBox { + spacing: 6px; +} + +QCheckBox::indicator { + width: 15px; + height: 15px; + border-radius: 3px; + border: 1px solid #5A5A5A; + background-color: #1A1A1A; +} + +QCheckBox::indicator:hover { + border: 1px solid #8A8A8A; + background-color: #202020; +} + +QCheckBox::indicator:checked { + background-color: #B71C1C; + border: 1px solid #C62828; + image: url("__CHECKBOX_CHECK_ICON__"); +} + +QCheckBox::indicator:checked:hover { + background-color: #C62828; + border: 1px solid #C62828; + image: url("__CHECKBOX_CHECK_ICON__"); +} + +QCheckBox::indicator:disabled { + border: 1px solid #333333; + background-color: #141414; +}