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

Deleting local playlist does not delete corresponding M3U file #937

Closed
tkem opened this Issue Jan 16, 2015 · 4 comments

Comments

3 participants
@tkem
Member

tkem commented Jan 16, 2015

Calling LocalPlaylistsProvider.delete(uri) in Mopidy 0.19.5 effectively removes the playlist only from self._playlists, but does not delete the underlying M3U file. Therefore, the playlist will re-appear after a restart or a PlaylistsController.refresh().

Looking into local/playlists.py, there seems to be an issue with the mapping of file names to URIs. For example, the file radio.m3u in my playlists directory will be assigned the URI local:playlist:radio, i.e. without extension, but AFAICS the extension will not be re-added by LocalPlaylistsProvider._m3u_uri_to_path(), so LocalPlaylistsProvider._delete_m3u() will silently(!) fail.

I guess this does not only affect deletion of playlists, but probably other playlist API functions that need to access the underlying M3U files, too.

Regarding local playlist URIs, I also noted that the file "Radio Österreich.m3u" is given the URI 'local:playlist:Radio \xd6sterreich', which is not properly percent-encoded. So, may I suggest refactoring the whole URI-related stuff in local/playlists.py, maybe reusing or adapting some of the URI encoding/decoding functions already used for local tracks ;-)

@tkem tkem changed the title from Deleting local playlists does not delete corresponding M3U file to Deleting local playlist does not delete corresponding M3U file Jan 16, 2015

@tkem

This comment has been minimized.

Member

tkem commented Jan 16, 2015

Update: As somewhat expected, when trying to save an existing playlist with another playlist.name, I get

[Errno 2] No such file or directory: u'/home/tkemmer/.local/share/mopidy/local/playlists/radio'

for M3U file "radio.m3u".

@tkem

This comment has been minimized.

Member

tkem commented Feb 23, 2015

Since nobody else seems to be working on this, I started looking into this myself:
https://github.com/tkem/mopidy/tree/fix/937
So you can label this as as "2 - working", I guess ;-)

@jodal

This comment has been minimized.

Member

jodal commented Feb 23, 2015

I invited you to join the team, so you can assign yourself and update the labels :-)

@tkem tkem added the 2 - Working label Feb 23, 2015

@tkem

This comment has been minimized.

Member

tkem commented Feb 23, 2015

Hey, thanks... I feel flattered ;-)

@tkem tkem added 3 - Done and removed 2 - Working labels Feb 23, 2015

@tkem tkem closed this in dd54fdb Feb 25, 2015

adamcik added a commit that referenced this issue Feb 25, 2015

Merge pull request #1000 from tkem/fix/937
Fix #937: Local playlists refactoring.

@jodal jodal added this to the v0.20 - Audio cleanup 1 milestone Feb 25, 2015

ZenithDK added a commit to ZenithDK/mopidy that referenced this issue Feb 28, 2015

@jodal jodal removed the 3 - Done label Mar 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment