Use text_type (unicode for PY2, str for PY3)

[sharkykh]

Closes #11
Thanks @miigotu

I figured, might as well change this to the whole file.

[ghost]

Thanks! merging.

[takluyver]

Any chance of a new release with this fix?

[takluyver]

Looking at this a bit more closely, I think it's better to work with bytes, since that's what Unix filesystems do natively. This module is never used on Windows, IIUC. I would do something like this:

try:
    fsencode = os.fsencode  # Py 3
except AttributeError
    def fsencode(p):    # Py 2
        return p.encode(sys.getfilesystemencoding())

def send2trash():
    if isinstance(path, text_type):
        # Convert to bytes
        path = fsencode(path)

Python 3 has ways of roundtripping bytes that can't be decoded in paths, which can't be fully emulated in Python 2.

It probably doesn't make much difference on modern desktop systems where the filesystem encoding is reported as UTF-8. But in some situations, Python might decide that the filesystem encoding is ASCII. If you convert all paths to unicode, no path with non-ascii characters can be handled. If you convert all paths to bytes, then passing in the raw bytes of the path will still work in that situation.

[ghost]

@takluyver As I write in #11 , I'm waiting for some kind of confirmation that the fix actually works. I don't use this package myself. As soon as someone bothers confirming, I'll bother releasing.

As for using bytes, I agree with you and I've said something along those lines in #11, but it's a bit more dangerous to work with bytes because the code below has been written assuming strings. This fix here has the advantage of being minimal and of fixing an immediate bug.

[takluyver]

Manually tested on Fedora; I confirm that this fixes it for a unicode path on Python 2, without breaking it for a bytes path, or for either kind of path on Python 3.

One future proofing note: I tend to check sys.version_info[0] >= 3, instead of ==. It shouldn't matter for a while, but I think the Python after 3.9 might be 4.0 rather than 3.10.

Would you be interested in any automated testing in this repo? I can make a PR to use Travis or Circle to run the test_plat_other.py file on each commit, if you like. You'd just need to log in to the relevant service and click a couple of things to enable builds.

[ghost]

Alright, thanks for confirming. Creating a release now.

Thanks for the CI offer, but this is a very low activity project. I suspect that the .travis.yml file would be changing more often than the code itself. I don't think it's worth it as long as activity level stays this low.

[takluyver]

Fair enough. We're having a go at integrating send2trash into the Jupyter Notebook, so if things come up, we can try to help fix them. If that means there's a bit more activity, the offer of CI setup stands. A simple Travis config like this changes very little.

[sharkykh]

One future proofing note: I tend to check sys.version_info[0] >= 3, instead of ==. It shouldn't matter for a while, but I think the Python after 3.9 might be 4.0 rather than 3.10.

Code is based on the six package, we just didn't think it justified adding it as a dependency.
I feel like Python 4 is still far...

[sharkykh]

@hsoft
I saw you released a new version on PyPI.
I suggest you also push the latest code here (the changes file and setup.py) and create a new tag for 1.3.1

[takluyver]

Thanks for the release!

[takluyver]

That fixed the error we were seeing, but unfortunately there's a different error now.

  File "/home/travis/virtualenv/python2.7.13/lib/python2.7/site-packages/send2trash/plat_other.py", line 163, in send2trash
    trash_move(path, dest_trash, topdir)
  File "/home/travis/virtualenv/python2.7.13/lib/python2.7/site-packages/send2trash/plat_other.py", line 88, in trash_move
    f.write(info_for(src, topdir))
  File "/home/travis/virtualenv/python2.7.13/lib/python2.7/site-packages/send2trash/plat_other.py", line 62, in info_for
    info += "Path=" + quote(src) + "\n"
  File "/opt/python/2.7.13/lib/python2.7/urllib.py", line 1299, in quote
    return ''.join(map(quoter, s))
KeyError: u'\xe9'

Sorry, I should have tested it more carefully before giving the thumbs up. 😞 It looks like Python 2's urllib.quote() doesn't handle unicode strings.

I'll work on a fix tomorrow.

[miigotu]

Also add unicode_literals import
from ___future__ import unicode_literals

That will likely fix the next problem you will hit, catting str and unicode

[takluyver]

Unfortunately not. The problem is coming from urllib.quote(), which is part of the standard library. It looks like we need to pass it bytes on Python 2.

I'm working on changing the plat_other code to use bytes paths throughout.