diff --git a/TASKS.md b/TASKS.md index f89e077..16a3936 100644 --- a/TASKS.md +++ b/TASKS.md @@ -38,9 +38,11 @@ performance half. This is the correctness half. never conflict, effective count = base + sum of journals, and `library.json` stops being rewritten every 3 s during playback — which is what generates the conflicts in the first place. -- [ ] **Quiet the merge dialog.** Lossless merges (counts and dates only) should - be a status-bar line, not a window; keep the dialog for lossy cases, and - reuse one instance so six can never stack up again. +- [x] **Quiet the merge dialog.** One dialog per session: each merge is folded + in as a timestamped entry (newest first) instead of opening another + window. Fifteen had stacked up. Kept the window for every merge rather + than demoting lossless ones to the status bar — with journals landing, + a merge stops being routine and is worth seeing. - [ ] Consider a `.stignore` for `.resolved` so merge backups stop syncing (Round 35 bounded the folder to 10 snapshots, but it still replicates). diff --git a/lintunes/gui/conflict_dialog.py b/lintunes/gui/conflict_dialog.py index dc96630..836c5c0 100644 --- a/lintunes/gui/conflict_dialog.py +++ b/lintunes/gui/conflict_dialog.py @@ -1,5 +1,11 @@ """A read-only summary of Syncthing conflicts LinTunes auto-merged, with the -option to open or restore the pre-merge backup.""" +option to open or restore the pre-merge backup. + +One dialog per session, not one per merge: Syncthing can deliver conflicts every +few minutes, so ``add_event`` folds each new merge into the open window as a +timestamped entry instead of stacking another window on the desktop. +""" +from datetime import datetime from pathlib import Path from PyQt6.QtCore import QUrl @@ -9,62 +15,97 @@ from PyQt6.QtWidgets import ( QMessageBox) +INTRO = ("LinTunes found changes made on more than one machine and merged them " + "(play counts kept highest, newest edits win, nothing removed). Both " + "versions were backed up first — restore them if a merge isn't what " + "you wanted.") + + class ConflictSummaryDialog(QDialog): def __init__(self, summaries, manager, parent=None): super().__init__(parent) - self._summaries = summaries self._manager = manager - self._backup_dir = summaries[0].backup_dir if summaries else "" + self._events = [] # newest last: (datetime, list[ConflictSummary]) self.setWindowTitle("Synced changes merged") self.resize(560, 440) layout = QVBoxLayout(self) - intro = QLabel( - "LinTunes found changes made on more than one machine and merged " - "them (play counts kept highest, newest edits win, nothing removed). " - "Both versions were backed up first — restore them if a merge isn't " - "what you wanted.") - intro.setWordWrap(True) - layout.addWidget(intro) + self._intro = QLabel(INTRO) + self._intro.setWordWrap(True) + layout.addWidget(self._intro) - body = QTextEdit() - body.setReadOnly(True) - body.setPlainText(self._format(summaries)) - layout.addWidget(body, 1) + self._body = QTextEdit() + self._body.setReadOnly(True) + layout.addWidget(self._body, 1) buttons = QHBoxLayout() - open_btn = QPushButton("Open backup folder") - open_btn.clicked.connect(self._open_backup) - restore_btn = QPushButton("Restore pre-merge backup") - restore_btn.clicked.connect(self._restore_backup) + self._open_btn = QPushButton("Open backup folder") + self._open_btn.clicked.connect(self._open_backup) + self._restore_btn = QPushButton("Restore pre-merge backup") + self._restore_btn.clicked.connect(self._restore_backup) keep_btn = QPushButton("Keep merged") keep_btn.setDefault(True) keep_btn.clicked.connect(self.accept) - buttons.addWidget(open_btn) - buttons.addWidget(restore_btn) + buttons.addWidget(self._open_btn) + buttons.addWidget(self._restore_btn) buttons.addStretch(1) buttons.addWidget(keep_btn) layout.addLayout(buttons) - open_btn.setEnabled(bool(self._backup_dir)) - restore_btn.setEnabled(bool(self._backup_dir)) + self.add_event(summaries) - @staticmethod - def _format(summaries) -> str: + # ---- events ---- + + def add_event(self, summaries, when=None): + """Fold another merge into this window as its own timestamped entry.""" + self._events.append((when or datetime.now(), list(summaries or []))) + self._refresh() + + @property + def _backup_dir(self) -> str: + """The newest event's backup snapshot. One resolve_conflicts() call makes + one timestamped dir, so every summary in an event shares it.""" + for _when, summaries in reversed(self._events): + if summaries and summaries[0].backup_dir: + return summaries[0].backup_dir + return "" + + def _refresh(self): + self._body.setPlainText(self._format()) + count = len(self._events) + self._intro.setText( + INTRO if count < 2 else f"{INTRO}\n\n{count} merges this session.") + newest = self._events[-1][0] if self._events else None + self._restore_btn.setText( + f"Restore backup from {newest.strftime('%-I:%M %p')}" + if newest and count > 1 else "Restore pre-merge backup") + self._open_btn.setEnabled(bool(self._backup_dir)) + self._restore_btn.setEnabled(bool(self._backup_dir)) + + def _format(self) -> str: blocks = [] - for s in summaries: - lines = [f"● {s.file}"] - lines.extend(f" {line}" for line in s.lines) - blocks.append("\n".join(lines)) + multi = len(self._events) > 1 + for when, summaries in reversed(self._events): # newest first + lines = [] + if multi: + lines.append(f"── {when.strftime('%-I:%M %p')} " + "─" * 30) + for s in summaries: + lines.append(f"● {s.file}") + lines.extend(f" {line}" for line in s.lines) + lines.append("") + blocks.append("\n".join(lines).rstrip()) return "\n\n".join(blocks) + # ---- actions ---- + def _open_backup(self): if self._backup_dir: QDesktopServices.openUrl(QUrl.fromLocalFile(self._backup_dir)) def _restore_backup(self): - if not self._backup_dir: + backup_dir = self._backup_dir + if not backup_dir: return if QMessageBox.question( self, "Restore backup", @@ -73,7 +114,7 @@ class ConflictSummaryDialog(QDialog): "conflict later.") != QMessageBox.StandardButton.Yes: return from lintunes.storage import conflict_resolver - conflict_resolver.restore_backup(Path(self._backup_dir), self._manager.data_dir) + conflict_resolver.restore_backup(Path(backup_dir), self._manager.data_dir) self._manager.reload_from_disk() QMessageBox.information(self, "Restored", "Restored the pre-merge backup.") self.accept() diff --git a/lintunes/gui/main_window.py b/lintunes/gui/main_window.py index 8ac1fdc..185ef10 100644 --- a/lintunes/gui/main_window.py +++ b/lintunes/gui/main_window.py @@ -745,12 +745,27 @@ class MainWindow(QMainWindow): self.show_conflict_summary(summaries) def show_conflict_summary(self, summaries): + """Show (or extend) the one merge window. Syncthing can deliver conflicts + every few minutes; each merge becomes another entry in the open window + rather than another window.""" from lintunes.gui.conflict_dialog import ConflictSummaryDialog - # Modeless so a merge never blocks playback or what you're doing. - self._conflict_dialog = ConflictSummaryDialog(summaries, self._manager, self) - self._conflict_dialog.show() + if self._conflict_dialog is not None: + self._conflict_dialog.add_event(summaries) + else: + # Modeless so a merge never blocks playback or what you're doing. + self._conflict_dialog = ConflictSummaryDialog( + summaries, self._manager, self) + self._conflict_dialog.finished.connect(self._on_conflict_dialog_closed) + self._conflict_dialog.show() self._conflict_dialog.raise_() + def _on_conflict_dialog_closed(self, _result): + """Drop the reference (and the widget) so the next merge opens a fresh + window instead of touching a deleted C++ object.""" + dialog, self._conflict_dialog = self._conflict_dialog, None + if dialog is not None: + dialog.deleteLater() + # ---- info dialog ---- def _show_info(self, track_ids: list[int]): diff --git a/tests/test_round38.py b/tests/test_round38.py new file mode 100644 index 0000000..57da872 --- /dev/null +++ b/tests/test_round38.py @@ -0,0 +1,101 @@ +"""Round 38: one merge window, and play counts that can't conflict. + +Two bugs stacked fifteen "Synced changes merged" windows on the desktop: the +dialog was never reused, and playing a song rewrote all 15 MB of library.json — +so Syncthing produced a conflict file roughly once per track. The dialog is now +a session-long log, and play counts live in per-machine journal files that only +their owner ever writes. +""" +from datetime import datetime + +import pytest + +from lintunes.storage.conflict_resolver import ConflictSummary + + +def _summary(file="library.json", lines=("2 track(s) reconciled.",), backup="/b/1"): + return ConflictSummary(file, "library", list(lines), backup) + + +@pytest.fixture +def dialog_cls(qapp): + from lintunes.gui.conflict_dialog import ConflictSummaryDialog + return ConflictSummaryDialog + + +class TestOneMergeWindow: + def test_second_merge_appends_instead_of_replacing(self, dialog_cls): + dlg = dialog_cls([_summary(lines=["first merge"])], manager=None) + dlg.add_event([_summary(lines=["second merge"])]) + + text = dlg._body.toPlainText() + assert "first merge" in text and "second merge" in text + # Newest first. + assert text.index("second merge") < text.index("first merge") + + def test_events_are_timestamped_once_there_is_more_than_one(self, dialog_cls): + dlg = dialog_cls([_summary()], manager=None) + assert "──" not in dlg._body.toPlainText() # a lone merge needs no header + + dlg.add_event([_summary()], when=datetime(2026, 8, 20, 18, 42)) + text = dlg._body.toPlainText() + assert "6:42 PM" in text + assert "2 merges this session." in dlg._intro.text() + + def test_restore_targets_the_newest_backup_and_names_it(self, dialog_cls): + dlg = dialog_cls([_summary(backup="/b/old")], manager=None) + assert dlg._backup_dir == "/b/old" + + dlg.add_event([_summary(backup="/b/new")], when=datetime(2026, 8, 20, 18, 42)) + assert dlg._backup_dir == "/b/new" + assert dlg._restore_btn.text() == "Restore backup from 6:42 PM" + + def test_empty_event_leaves_the_previous_backup_reachable(self, dialog_cls): + dlg = dialog_cls([_summary(backup="/b/old")], manager=None) + dlg.add_event([]) + assert dlg._backup_dir == "/b/old" + assert dlg._restore_btn.isEnabled() + + def test_no_backup_disables_the_backup_buttons(self, dialog_cls): + dlg = dialog_cls([_summary(backup="")], manager=None) + assert not dlg._open_btn.isEnabled() + assert not dlg._restore_btn.isEnabled() + + +class TestWindowReuse: + """MainWindow isn't constructible headless, so exercise the two real methods + on a minimal QWidget host — that's where the stacking bug lived.""" + + @pytest.fixture + def host(self, qapp): + from PyQt6.QtWidgets import QWidget + from lintunes.gui.main_window import MainWindow + + class Host(QWidget): + _manager = None + _conflict_dialog = None + show_conflict_summary = MainWindow.show_conflict_summary + _on_conflict_dialog_closed = MainWindow._on_conflict_dialog_closed + + return Host() + + def test_repeated_merges_make_exactly_one_dialog(self, host): + from lintunes.gui.conflict_dialog import ConflictSummaryDialog + + for i in range(15): + host.show_conflict_summary([_summary(lines=[f"merge {i}"])]) + + dialogs = host.findChildren(ConflictSummaryDialog) + assert len(dialogs) == 1 + text = dialogs[0]._body.toPlainText() + assert "merge 0" in text and "merge 14" in text + + def test_closing_lets_the_next_merge_open_a_fresh_window(self, host): + host.show_conflict_summary([_summary()]) + first = host._conflict_dialog + first.accept() # "Keep merged" + assert host._conflict_dialog is None + + host.show_conflict_summary([_summary()]) + assert host._conflict_dialog is not None + assert host._conflict_dialog is not first