Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sanitize playlist names #2258

Merged
merged 7 commits into from Nov 12, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions beetsplug/smartplaylist.py
Expand Up @@ -20,7 +20,7 @@

from beets.plugins import BeetsPlugin
from beets import ui
from beets.util import mkdirall, normpath, syspath, bytestring_path
from beets.util import mkdirall, normpath, sanitize_path, syspath, bytestring_path
from beets.library import Item, Album, parse_query_string
from beets.dbcore import OrQuery
from beets.dbcore.query import MultipleSort, ParsingError
Expand Down Expand Up @@ -173,7 +173,8 @@ def update_playlists(self, lib):

for playlist in self._matched_playlists:
name, (query, q_sort), (album_query, a_q_sort) = playlist
self._log.debug(u"Creating playlist {0}", name)
sanitized_name = sanitize_path(name, lib.replacements)
self._log.debug(u"Creating playlist {0}", sanitized_name)
items = []

if query:
Expand All @@ -186,7 +187,7 @@ def update_playlists(self, lib):
# As we allow tags in the m3u names, we'll need to iterate through
# the items and generate the correct m3u file names.
for item in items:
m3u_name = item.evaluate_template(name, True)
m3u_name = item.evaluate_template(sanitized_name, True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm; are you sure you want to be sanitizing the template instead of the result of filling in the template?

if m3u_name not in m3us:
m3us[m3u_name] = []
item_path = item.path
Expand Down
2 changes: 2 additions & 0 deletions docs/changelog.rst
Expand Up @@ -32,6 +32,8 @@ Deprecated configuration optional removals:

The are a couple of small new features:

* :doc:`/plugins/smartplaylist`: Playlist names will be sanitized to
ensure valid filenames. :bug:`2258`
* :doc:`/plugins/mpdupdate`, :doc:`/plugins/mpdstats`: When the ``host`` option
is not set, these plugins will now look for the ``$MPD_HOST`` environment
variable before falling back to ``localhost``. Thanks to :user:`tarruda`.
Expand Down
16 changes: 9 additions & 7 deletions test/test_smartplaylist.py
Expand Up @@ -25,7 +25,7 @@
from beets.library import Item, Album, parse_query_string
from beets.dbcore import OrQuery
from beets.dbcore.query import NullSort, MultipleSort, FixedFieldSort
from beets.util import syspath, bytestring_path, py3_path
from beets.util import syspath, bytestring_path, py3_path, CHAR_REPLACE
from beets.ui import UserError
from beets import config

Expand Down Expand Up @@ -151,12 +151,15 @@ def test_playlist_update(self):

i = Mock(path=b'/tagada.mp3')
i.evaluate_template.side_effect = lambda x, _: x
q = Mock()
a_q = Mock()

lib = Mock()
lib.replacements = CHAR_REPLACE
lib.items.return_value = [i]
lib.albums.return_value = []
pl = b'my_playlist.m3u', (q, None), (a_q, None)

q = Mock()
a_q = Mock()
pl = b'.my:<playlist>.m3u', (q, None), (a_q, None)
spl._matched_playlists = [pl]

dir = bytestring_path(mkdtemp())
Expand All @@ -171,15 +174,14 @@ def test_playlist_update(self):
lib.items.assert_called_once_with(q, None)
lib.albums.assert_called_once_with(a_q, None)

m3u_filepath = path.join(dir, pl[0])
m3u_filepath = path.join(dir, b'_my__playlist_.m3u')
self.assertTrue(path.exists(m3u_filepath))
with open(syspath(m3u_filepath), 'rb') as f:
content = f.read()
rmtree(dir)

self.assertEqual(content, b'/tagada.mp3\n')



class SmartPlaylistCLITest(unittest.TestCase, TestHelper):
def setUp(self):
self.setup_beets()
Expand Down