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/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/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; +} 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()