use "long filename" support instead of short truncation on Windows (#127)

(Patch by jonathan.buchanan. Thanks!)

NEWS

@@ -20,7 +20,7 @@
   completely wrong association of track names to files. The order
   applied was always just alphabetical by filename, which is frequently
   but not always what you want.
-* Filenames are now truncated to 30 characters on Windows.
+* We now use Windows' "long filename" support.
 * Fix crash in lastid when the artist name is not available.
 * Fixed a spurious crash when LANG or a related environment variable is
   set to an invalid value (such as 'UTF-8' on some installations of Mac

beets/library.py

@@ -24,7 +24,6 @@ from beets.mediafile import MediaFile, UnreadableFileError, FileTypeError
 from beets import plugins
 
 MAX_FILENAME_LENGTH = 200
-MAX_WINDOWS_FILENAME_LENGTH = 30
 
 # Fields in the "items" database table; all the metadata available for
 # items in the library. These are used directly in SQL; they are
@@ -178,6 +177,32 @@ def _bytestring_path(path):
     except UnicodeError:
         return path.encode('utf8')
 
+def _syspath(path, pathmod=None):
+    """Convert a path for use by the operating system. In particular,
+    paths on Windows must receive a magic prefix and must be converted
+    to unicode before they are sent to the OS.
+    """
+    pathmod = pathmod or os.path
+    windows = pathmod.__name__ == 'ntpath'
+
+    # Don't do anything if we're not on windows
+    if not windows:
+        return path
+
+    if not isinstance(path, unicode):
+        # Try to decode with default encodings, but fall back to UTF8.
+        encoding = sys.getfilesystemencoding() or sys.getdefaultencoding()
+        try:
+            path = path.decode(encoding, 'replace')
+        except UnicodeError:
+            path = path.decode('utf8', 'replace')
+
+    # Add the magic prefix if it isn't already there
+    if not path.startswith(u'\\\\?\\'):
+        path = u'\\\\?\\' + path
+
+    return path
+
 # Note: POSIX actually supports \ and : -- I just think they're
 # a pain. And ? has caused problems for some.
 CHAR_REPLACE = [
@@ -204,9 +229,7 @@ def _sanitize_path(path, pathmod=None):
             comp = regex.sub(repl, comp)
         
         # Truncate each component.
-        maxlen = MAX_WINDOWS_FILENAME_LENGTH if windows else MAX_FILENAME_LENGTH
-        if len(comp) > maxlen:
-            comp = comp[:maxlen]
+        comp = comp[:MAX_FILENAME_LENGTH]
                 
         comps[i] = comp
     return pathmod.join(*comps)
@@ -293,7 +316,7 @@ class Item(object):
             read_path = self.path
         else:
             read_path = _normpath(read_path)
-        f = MediaFile(read_path)
+        f = MediaFile(_syspath(read_path))
 
         for key in ITEM_KEYS_META:
             setattr(self, key, getattr(f, key))
@@ -302,7 +325,7 @@ class Item(object):
     def write(self):
         """Writes the item's metadata to the associated file.
         """
-        f = MediaFile(self.path)
+        f = MediaFile(_syspath(self.path))
         for key in ITEM_KEYS_WRITABLE:
             setattr(f, key, getattr(self, key))
         f.save()
@@ -330,14 +353,14 @@ class Item(object):
         # Create necessary ancestry for the move.
         _mkdirall(dest)
         
-        if not shutil._samefile(self.path, dest):
+        if not shutil._samefile(_syspath(self.path), _syspath(dest)):
             if copy:
                 # copyfile rather than copy will not copy permissions
                 # bits, thus possibly making the copy writable even when
                 # the original is read-only.
-                shutil.copyfile(self.path, dest)
+                shutil.copyfile(_syspath(self.path), _syspath(dest))
             else:
-                shutil.move(self.path, dest)
+                shutil.move(_syspath(self.path), _syspath(dest))
             
         # Either copying or moving succeeded, so update the stored path.
         self.path = dest
@@ -1133,7 +1156,7 @@ class Album(BaseAlbum):
         if delete:
             artpath = self.artpath
             if artpath:
-                os.unlink(artpath)
+                os.unlink(_syspath(artpath))
         
         # Remove album.
         self._library.conn.execute(
@@ -1157,9 +1180,9 @@ class Album(BaseAlbum):
             new_art = self.art_destination(old_art, newdir)
             if new_art != old_art:
                 if copy:
-                    shutil.copy(old_art, new_art)
+                    shutil.copy(_syspath(old_art), _syspath(new_art))
                 else:
-                    shutil.move(old_art, new_art)
+                    shutil.move(_syspath(old_art), _syspath(new_art))
                 self.artpath = new_art
 
         # Store new item paths. We do this at the end to avoid
@@ -1192,7 +1215,7 @@ class Album(BaseAlbum):
         oldart = self.artpath
         artdest = self.art_destination(path)
         if oldart == artdest:
-            os.unlink(oldart)
+            os.unlink(_syspath(oldart))
 
-        shutil.copy(path, artdest)
+        shutil.copy(_syspath(path), _syspath(artdest))
         self.artpath = artdest

test/test_db.py

@@ -259,14 +259,6 @@ class DestinationTest(unittest.TestCase):
         p = beets.library._sanitize_path(u':', posixpath)
         self.assertEqual(p, u'-')
     
-    def test_sanitize_windows_uses_very_short_names(self):
-        p = beets.library._sanitize_path('X'*300 + '/' + 'Y'*200, ntpath)
-        self.assertLessEqual(len(p), 100)
-
-    def test_sanitize_unix_uses_longer_names(self):
-        p = beets.library._sanitize_path('X'*300 + '/' + 'Y'*200, posixpath)
-        self.assertGreaterEqual(len(p), 100)
-
     def test_path_with_format(self):
         self.lib.path_format = '$artist/$album ($format)'
         p = self.lib.destination(self.i)
@@ -280,6 +272,18 @@ class DestinationTest(unittest.TestCase):
         dest1, dest2 = self.lib.destination(i1), self.lib.destination(i2)
         self.assertEqual(os.path.dirname(dest1), os.path.dirname(dest2))
 
+    def test_syspath_windows_format(self):
+        path = ntpath.join('a', 'b', 'c')
+        outpath = beets.library._syspath(path, ntpath)
+        self.assertTrue(isinstance(outpath, unicode))
+        self.assertTrue(outpath.startswith(u'\\\\?\\'))
+
+    def test_syspath_posix_unchanged(self):
+        path = posixpath.join('a', 'b', 'c')
+        outpath = beets.library._syspath(path, posixpath)
+        self.assertEqual(path, outpath)
+
+
 class MigrationTest(unittest.TestCase):
     """Tests the ability to change the database schema between
     versions.