One merge window, not fifteen
show_conflict_summary() built a fresh ConflictSummaryDialog every time and only reassigned self._conflict_dialog — the old dialog was still a child of the window, so it stayed on screen. check_for_external_changes() runs resolve_conflicts() on every Syncthing watch tick, so that was one window per merge, forever. Fifteen had piled up on trav's desktop. The dialog is now a session-long log: add_event() folds each merge in as its own timestamped entry, newest first, and the restore button names the merge it would undo. MainWindow reuses the open dialog and clears its reference on finished, so closing it lets the next merge open a fresh one instead of touching a deleted C++ object. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8Mzze7shr5pZoEgKQU5NW
This commit is contained in:
@@ -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).
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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]):
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user