path converter

[altendky]

fixes #405

[altendky]

WIP for:

• Changes entry

[SoniEx2]

missing tests when path is a bytes instead of str.

[altendky]

@SoniEx2, when does that happen?

[SoniEx2]

POSIX.

[davidism]

sys.argv is always str in Python 3, which is all master supports now.

[SoniEx2]

but paths aren't always valid unicode. how is that handled by click? I just looked and paths can still be bytes, or at least the documentation still says so.

[altendky]

@SoniEx2 as in my example you ignored in #python... surrogate escapes afaik.

print(os.fspath(pathlib.Path(b'foo\xff\xff\xff\xfd\xfd\xfd'.decode(errors='surrogateescape'))))

[altendky]

Well, os.fsdecode(), but the surrogate escapes is the interesting part.

https://docs.python.org/3/library/sys.html#sys.argv
https://docs.python.org/3/library/os.html#os.fsdecode

[davidism]

It's Python, not Click, that is treating sys.argv as unicode. This has been documented for quite a while in Click: https://click.palletsprojects.com/en/7.x/python3/#python-2-and-3-differences

• sys.argv is always Unicode-based. This also means that the native type for input values to the types in Click is Unicode, and not bytes.

  This causes problems if the terminal is incorrectly set and Python does not figure out the encoding. In that case, the Unicode string will contain error bytes encoded as surrogate escapes.

• When dealing with files, Click will always use the Unicode file system API calls by using the operating system’s reported or guessed filesystem encoding. Surrogates are supported for filenames, so it should be possible to open files through the File type even if the environment is misconfigured.

Based on this, it sounds like Click doesn't need to do anything extra. Python's file operations work with Python's argv handling.

[SoniEx2]

that's for File, not Path. Path still claims "str or bytes".

[davidism]

That's an issue with the docs, not with Click. The docs were very focused on Python 2, which is what had the difference in this case. Fairly sure I updated them in master already.

Also, that's totally tangential to this PR, which is only about passing whatever we were already getting to a converter, mainly to work with pathlib. Nothing you've commented so far seems to indicate there's an issue with that.

[SoniEx2]

the PR just grabs the former result and optionally wraps it. since the former result can be bytes (according to the docs in master), this could be a problem.

[davidism]

I think we're going in circles. If you have an issue with this PR that can be addressed by Click, in this PR, you need to provide a concrete example or reference to what you're referring to.

The fact that pathlib doesn't accept bytes, even though the filesystem may be bytes, is an issue (or design decision) of pathlib. There's nothing Click can do about it, all it sees is a function (not necessarily pathlib.Path, it's generic) to pass a value to. It seems like a non-issue as well because Python only sends sys.argv as str, which is what Pathlib accepts. Again, if the docs say it sometimes returns bytes, that's an issue with the docs still referring to Python 2 behavior.

[altendky]

@SoniEx2 since you already found something you think is a problem, could you link it so we know where you found it? We like to fix things (well I do, I presume @davidism does as well...) but it's a pain to have to dig up what you claim is wrong when you haven't even said what docs you have a problem with.

[SoniEx2]

... meh. I think the problem is that click.Path and pathlib.Path are both called Path tbh.

but just taking the return value of click.Path and passing it to pathlib.Path, when click.Path is documented (in master) as returning bytes sometimes, sounds like a recipe for disaster.

[altendky]

@SoniEx2 'master' is a big space. Where is the documentation? Unlike your system GitHub allows you to link to specific lines of code. Use that feature. Let us know where the problem is. I'm not questioning whether an issue exists but I know it's easy to miss stuff. Another PR here I worked on involved at least three different ways a thing was implemented/documented... lots of confusion there as people said 'it says x' and someone else said 'it says y' and another said 'it does z' and all were correct. So, what exactly are you talking about? What line of documentation in what file?

[SoniEx2]

https://github.com/pallets/click/blob/master/docs/arguments.rst#file-path-arguments