v0.9.1: playlist merges stop losing your track order
What scrambled `a nissa one` took three defects at once. Reorder it on machine A; open it on machine B and resize the window; B's file is now newer, so the merge takes B's order — the old one — wholesale. _merge_playlist was a 2-way union with no common ancestor: one side's order wholesale, the other side's extras appended at the tail, so a track inserted in the middle on one machine arrived at the bottom on the other. merge_track_order re-inserts each side-only track after the nearest track both copies share instead. _reconcile_playlist (the live-reload path) uses the same helper. The union invariant is unchanged and property-tested over 300 random pairs — a merge never drops a track. Whose order wins is now Playlist.date_modified, bumped only in _set_track_ids (add / remove / reorder / undo) and deliberately not by mark_playlist_settings_dirty, with a file-mtime fallback for playlists written before the field existed. The file's mtime was a lie: the last column is stretch-sized, so Qt re-fires sectionResized whenever the viewport width changes, and a window resize or splitter drag rewrote the open playlist's JSON — moving its mtime and handing Syncthing another conflict — for a width apply_settings overrides on load anyway. _on_section_resized now skips that section; genuine drags on every other column still persist. Replayed on a copy of the real playlists: `a nissa one` keeps its reorder against a newer-by-mtime opponent, and `a nissa ideas` takes the other machine's two mid-list inserts at 5 and 12 rather than 26 and 27. Claude-Session: https://claude.ai/code/session_01M8Mzze7shr5pZoEgKQU5NW
This commit is contained in:
@@ -1,3 +1,3 @@
|
||||
"""LinTunes — iTunes-style music library manager and player for Linux."""
|
||||
|
||||
__version__ = "0.9.0"
|
||||
__version__ = "0.9.1"
|
||||
|
||||
@@ -714,6 +714,15 @@ class TrackTableView(QTableView):
|
||||
def _on_section_resized(self, section, _old, new_width):
|
||||
if self._suppress_signals or section >= len(self.model_.fields):
|
||||
return
|
||||
header = self.horizontalHeader()
|
||||
if (header.stretchLastSection()
|
||||
and section == header.logicalIndex(header.count() - 1)):
|
||||
# The last column is stretch-sized, so Qt re-fires this every time
|
||||
# the viewport changes width — resizing the window or dragging the
|
||||
# splitter would otherwise rewrite the whole playlist JSON (and hand
|
||||
# Syncthing a conflict). Its width is derived, not chosen: there's no
|
||||
# right edge to drag, and apply_settings overrides it on load.
|
||||
return
|
||||
field = self.model_.fields[section]
|
||||
if field != INDEX_FIELD:
|
||||
self.column_width_changed.emit(field, new_width)
|
||||
|
||||
@@ -313,6 +313,11 @@ class LibraryManager(QObject):
|
||||
if playlist is None:
|
||||
return
|
||||
playlist.track_ids = list(ids)
|
||||
# The single funnel for every content change (add / remove / reorder /
|
||||
# undo), and deliberately the *only* place date_modified moves — a
|
||||
# column resize must not look like an edit to the merge. See
|
||||
# conflict_resolver._playlist_newer.
|
||||
playlist.date_modified = _utc_now_iso()
|
||||
self._mark_playlist(pid)
|
||||
self.playlist_content_changed.emit(pid)
|
||||
|
||||
@@ -974,13 +979,13 @@ def _reconcile_track(mem_track, disk_track):
|
||||
|
||||
|
||||
def _reconcile_playlist(mem_pl, disk_pl):
|
||||
"""Reconcile a playlist we were locally editing with the disk copy: keep our
|
||||
order and append any tracks that exist only on the other machine (a merge
|
||||
"""Reconcile a playlist we were locally editing with the disk copy: our order
|
||||
wins (the local edit is the more recent one), and tracks that exist only on
|
||||
the other machine are merged back into position rather than appended (a merge
|
||||
never drops tracks)."""
|
||||
local = list(mem_pl.track_ids)
|
||||
local_set = set(local)
|
||||
mem_pl.track_ids = local + [tid for tid in disk_pl.track_ids
|
||||
if tid not in local_set]
|
||||
from lintunes.storage.conflict_resolver import merge_track_order
|
||||
mem_pl.track_ids = merge_track_order(list(mem_pl.track_ids),
|
||||
list(disk_pl.track_ids))
|
||||
|
||||
|
||||
def _utc_now_iso() -> str:
|
||||
|
||||
@@ -58,6 +58,10 @@ class Playlist:
|
||||
track_ids: list[int] = field(default_factory=list)
|
||||
settings: PlaylistSettings = field(default_factory=PlaylistSettings)
|
||||
is_system: bool = False
|
||||
# Bumped only when track_ids change (LibraryManager._set_track_ids), never
|
||||
# for column widths or sort order — the conflict merge uses it to decide
|
||||
# whose order wins, and the file's mtime moves for cosmetic reasons.
|
||||
date_modified: Optional[str] = None
|
||||
# Set only for SMART playlists. Membership (track_ids) is derived from this
|
||||
# by the LibraryManager; for imported playlists track_ids also doubles as a
|
||||
# snapshot fallback when the criteria couldn't be fully parsed.
|
||||
@@ -78,6 +82,8 @@ class Playlist:
|
||||
}
|
||||
if self.parent_persistent_id:
|
||||
d["parent_persistent_id"] = self.parent_persistent_id
|
||||
if self.date_modified:
|
||||
d["date_modified"] = self.date_modified
|
||||
if self.smart_criteria is not None:
|
||||
d["smart_criteria"] = self.smart_criteria.to_dict()
|
||||
return d
|
||||
@@ -96,6 +102,7 @@ class Playlist:
|
||||
track_ids=d.get("track_ids", []),
|
||||
settings=PlaylistSettings.from_dict(d.get("settings", {})),
|
||||
is_system=d.get("is_system", False),
|
||||
date_modified=d.get("date_modified"),
|
||||
smart_criteria=smart_criteria,
|
||||
)
|
||||
|
||||
|
||||
@@ -226,43 +226,102 @@ def _merge_track_fields(orig: dict, conflict: dict) -> list[str]:
|
||||
return notes
|
||||
|
||||
|
||||
def merge_track_order(primary: list[int], secondary: list[int]) -> list[int]:
|
||||
"""Union two orderings of a playlist without losing position.
|
||||
|
||||
A plain union appends whatever the other copy had at the tail, so a track
|
||||
inserted in the *middle* on one machine arrives at the *bottom* on the other
|
||||
— which is what scrambled `a nissa one`. Instead, every track that exists
|
||||
only in ``secondary`` is re-inserted after its nearest preceding track that
|
||||
both copies share (its "anchor"); ``primary`` decides the order of everything
|
||||
the two have in common.
|
||||
|
||||
The result is always the union of the two lists: a merge must never drop a
|
||||
track, so anything whose anchor never turns up is flushed at the tail.
|
||||
"""
|
||||
common = set(primary) & set(secondary)
|
||||
|
||||
# Walk secondary once: each secondary-only track hangs off the last shared
|
||||
# track before it (None = it led the list).
|
||||
pending: dict[object, list[int]] = {}
|
||||
anchor = None
|
||||
for tid in secondary:
|
||||
if tid in common:
|
||||
anchor = tid
|
||||
else:
|
||||
pending.setdefault(anchor, []).append(tid)
|
||||
|
||||
merged = list(pending.pop(None, []))
|
||||
seen = set()
|
||||
for tid in primary:
|
||||
merged.append(tid)
|
||||
# First occurrence only — a playlist may hold the same track twice.
|
||||
if tid in common and tid not in seen:
|
||||
seen.add(tid)
|
||||
merged.extend(pending.pop(tid, []))
|
||||
for leftover in pending.values(): # anchor never appeared; never drop it
|
||||
merged.extend(leftover)
|
||||
return merged
|
||||
|
||||
|
||||
def _playlist_newer(original: dict, conflict: dict,
|
||||
original_path: Path, conflict_path: Path) -> bool:
|
||||
"""Is the incoming copy the more recently *edited* one?
|
||||
|
||||
Prefer the playlist's own ``date_modified``, which only moves when its
|
||||
contents change. File mtime is a lie: the last column is stretch-sized, so
|
||||
resizing the window rewrites the open playlist's JSON — "most recently
|
||||
edited" used to mean "most recently resized". Falls back to mtime for
|
||||
playlists written before ``date_modified`` existed.
|
||||
"""
|
||||
ours, theirs = original.get("date_modified"), conflict.get("date_modified")
|
||||
if ours and theirs:
|
||||
return theirs > ours
|
||||
return conflict_path.stat().st_mtime > original_path.stat().st_mtime
|
||||
|
||||
|
||||
def _merge_playlist(original_path: Path, conflict_path: Path) -> ConflictSummary:
|
||||
original = read_json(original_path)
|
||||
conflict = read_json(conflict_path)
|
||||
name = original.get("name") or conflict.get("name") or original_path.stem
|
||||
|
||||
conflict_newer = _playlist_newer(original, conflict, original_path, conflict_path)
|
||||
|
||||
# Smart playlists: membership is derived, so keep the newer criteria and let
|
||||
# it recompute (unioning derived track_ids would resurrect non-matches).
|
||||
if original.get("playlist_type") == "smart" or conflict.get("playlist_type") == "smart":
|
||||
newer = _keep_newer(original_path, conflict_path)
|
||||
newer.file, newer.kind = name, "playlist"
|
||||
newer.lines = ["Smart-playlist rules taken from the most recently edited copy."]
|
||||
return newer
|
||||
if conflict_newer:
|
||||
write_json(original_path, conflict)
|
||||
return ConflictSummary(
|
||||
name, "playlist",
|
||||
["Smart-playlist rules taken from the most recently edited copy."])
|
||||
|
||||
orig_ids = list(original.get("track_ids", []))
|
||||
conf_ids = list(conflict.get("track_ids", []))
|
||||
orig_set, conf_set = set(orig_ids), set(conf_ids)
|
||||
|
||||
conflict_newer = conflict_path.stat().st_mtime > original_path.stat().st_mtime
|
||||
if conflict_newer:
|
||||
base_order = conf_ids
|
||||
extra = [tid for tid in orig_ids if tid not in conf_set]
|
||||
original["track_ids"] = base_order + extra
|
||||
# The other copy was edited last, so it sets the order; anything only we
|
||||
# had is re-inserted at its anchor rather than dumped at the tail.
|
||||
original["track_ids"] = merge_track_order(conf_ids, orig_ids)
|
||||
if "settings" in conflict:
|
||||
original["settings"] = conflict["settings"]
|
||||
if conflict.get("date_modified"):
|
||||
original["date_modified"] = conflict["date_modified"]
|
||||
order_src = "the other machine"
|
||||
moved_in = len(conf_set - orig_set)
|
||||
else:
|
||||
extra = [tid for tid in conf_ids if tid not in orig_set]
|
||||
original["track_ids"] = orig_ids + extra
|
||||
original["track_ids"] = merge_track_order(orig_ids, conf_ids)
|
||||
order_src = "this machine"
|
||||
moved_in = len(orig_set - conf_set)
|
||||
|
||||
write_json(original_path, original)
|
||||
|
||||
added = len(set(original["track_ids"]) - orig_set)
|
||||
lines = [f"Order kept from {order_src} (most recently edited)."]
|
||||
if added:
|
||||
lines.append(f"{added} track(s) that were only in the other copy were kept "
|
||||
"(nothing is removed on a merge).")
|
||||
lines.append(f"{added} track(s) that were only in the other copy were kept, "
|
||||
"back in position (nothing is removed on a merge).")
|
||||
return ConflictSummary(name, "playlist", lines)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user