From 963a66a40c51a2fecae8f21daa01097e8457e4ca Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 8 Jan 2016 14:56:52 -0800
Subject: [PATCH] Refactor `try_sync` to also include moving

This will remove a bunch of duplication we currently have for moving files
*inside the library directory* when their metadata changes.
---
 beets/library.py     | 41 +++++++++++++++++++++++++----------------
 beets/ui/commands.py | 12 ++++--------
 beetsplug/edit.py    |  6 +++---
 3 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/beets/library.py b/beets/library.py
index b42dd2c47..59a20cabf 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -630,20 +630,26 @@ class Item(LibModel):
             log.error("{0}", exc)
             return False
 
-    def try_sync(self, write=None):
-        """Synchronize the item with the database and the media file
-        tags, updating them with this object's current state.
+    def try_sync(self, write, move, with_album=True):
+        """Synchronize the item with the database and, possibly, updates its
+        tags on disk and its path (by moving the file).
 
-        By default, the current `path` for the item is used to write
-        tags. If `write` is `False`, no tags are written. If `write` is
-        a path, tags are written to that file instead.
+        `write` indicates whether to write new tags into the file. Similarly,
+        `move` controls whether the path should be updated. In the
+        latter case, files are *only* moved when they are inside their
+        library's directory (if any).
 
-        Similar to calling :meth:`write` and :meth:`store`.
+        Similar to calling :meth:`write`, :meth:`move`, and :meth:`store`
+        (conditionally).
         """
-        if write is True:
-            write = None
-        if write is not False:
-            self.try_write(path=write)
+        if write:
+            self.try_write()
+        if move:
+            # Check whether this file is inside the library directory.
+            if self._db and self._db.directory in util.ancestry(self.path):
+                log.debug('moving {0} to synchronize path',
+                          util.displayable_path(self.path))
+                self.move(with_album=with_album)
         self.store()
 
     # Files themselves.
@@ -1108,15 +1114,18 @@ class Album(LibModel):
                         item[key] = value
                     item.store()
 
-    def try_sync(self, write=True):
-        """Synchronize the album and its items with the database and
-        their files by updating them with this object's current state.
+    def try_sync(self, write, move):
+        """Synchronize the album and its items with the database.
+        Optionally, also write any new tags into the files and update
+        their paths.
 
-        `write` indicates whether to write tags to the item files.
+        `write` indicates whether to write tags to the item files, and
+        `move` controls whether files (both audio and album art) are
+        moved.
         """
         self.store()
         for item in self.items():
-            item.try_sync(bool(write))
+            item.try_sync(write, move)
 
 
 # Query construction helpers.
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 72c345c68..773c10cae 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1353,13 +1353,7 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm):
     # Apply changes to database and files
     with lib.transaction():
         for obj in changed:
-            if move:
-                cur_path = obj.path
-                if lib.directory in ancestry(cur_path):  # In library?
-                    log.debug(u'moving object {0}', displayable_path(cur_path))
-                    obj.move()
-
-            obj.try_sync(write)
+            obj.try_sync(write, move)
 
 
 def modify_parse_args(args):
@@ -1510,7 +1504,9 @@ def write_items(lib, query, pretend, force):
         changed = ui.show_model_changes(item, clean_item,
                                         library.Item._media_tag_fields, force)
         if (changed or force) and not pretend:
-            item.try_sync()
+            # We use `try_sync` here to keep the mtime up to date in the
+            # database.
+            item.try_sync(True, False)
 
 
 def write_func(lib, opts, args):
diff --git a/beetsplug/edit.py b/beetsplug/edit.py
index 0aa56b885..b72b033df 100644
--- a/beetsplug/edit.py
+++ b/beetsplug/edit.py
@@ -220,7 +220,7 @@ class EditPlugin(plugins.BeetsPlugin):
 
         # Save the new data.
         if success:
-            self.save_write(objs)
+            self.save_changes(objs)
 
     def edit_objects(self, objs, fields):
         """Dump a set of Model objects to a file as text, ask the user
@@ -316,11 +316,11 @@ class EditPlugin(plugins.BeetsPlugin):
             id = int(old_dict['id'])
             apply(obj_by_id[id], new_dict)
 
-    def save_write(self, objs):
+    def save_changes(self, objs):
         """Save a list of updated Model objects to the database.
         """
         # Save to the database and possibly write tags.
         for ob in objs:
             if ob._dirty:
                 self._log.debug('saving changes to {}', ob)
-                ob.try_sync(ui.should_write())
+                ob.try_sync(ui.should_write(), False)