eonpatapon / mpDris2

MPRIS V2.1 support for mpd
GNU General Public License v3.0
211 stars 46 forks source link

Use unicode instead of str for song urls #82

Closed MadeOfMagicAndWires closed 5 years ago

MadeOfMagicAndWires commented 7 years ago

This fixes a UnicodeDecodeError when handling non-ascii filenames or paths.

So far I haven't changed any of the other strings to unicode because they all seemed to process fine, but that might be a pull for another day.

grawity commented 7 years ago

This also breaks compatibility with Python 3, which has no type named unicode. (Unicode strings in Py3 are called just str.)

I'd much rather have a properly working Python 3 version than a halfway patched Python 2 one, so I recommend a) using Py3, or b) figuring out a way to support Unicode in both.

MadeOfMagicAndWires commented 7 years ago

Oh sorry, our package manager has this as a python 2 package(I think because it doesn't have a python3 package for the mpd module) so I didn't even look if it was compatible.

grawity commented 7 years ago

There certainly are Python 3 mpd clients: https://aur.archlinux.org/packages/python-mpd2/ is compatible.

MadeOfMagicAndWires commented 7 years ago

Well, it's pretty ugly, but at least it works. I'll let you decide on whether to actually merge this but I'll at least make a python3 package for the AUR because the least python2 packages on my system the better.

ferdnyc commented 5 years ago

Given that Python2 is close to death, and Python3 str is intrinsically unicode encoded, this can probably just be closed no?

MadeOfMagicAndWires commented 5 years ago

Oh yeah, sorry I didn't mean to keep this open for so long.