msg163885 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 03:39 |
When you pass in a file descriptor as the first argument to listdir, its output is a list of strings, not a list of bytes. This should be mentioned in the documentation. It's also worth making a pass over the other functions accepting an fd. As it happens, the code that implements this behavior is obtuse. So I propose also modifying it to be more intelligible. |
|
|
msg163887 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 04:33 |
Maybe the correct fix for this is to change the interface? os.listdir(path, *, dir_fd=None) Then we'll pick up the return type (str/bytes) from the path variable. It would make #15177 even easier too, should we go down that route. |
|
|
msg163890 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2012-06-25 04:57 |
+1 for changing the API for any cases where the filesystem path is also used to determine the return type - given that we stuffed this up for the rmtree implementation, I expect end users would be prone to making exactly the same mistake. Retaining the ability to control the return type explicitly even when pass a descriptor would be a *lot* cleaner than other possibilities (which would all involve a bunch of manual encoding and decoding with os.fsencode() and os.fsdecode()). |
|
|
msg163891 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 04:59 |
Nick, how do you feel specifically about listdir(path, *, dir_fd)? I'm mentally exhausted from the past couple of days and have temporarily lost the ability to infer. |
|
|
msg163895 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 05:20 |
Nevermind, I'm an idiot. fdopendir doesn't support dir_fd as a separate path, so listdir can't support it either. There's an unwritten property of the os module on a POSIX system: all the functions are essentially-atomic. This is useful and should not be broken lightly. I'll make a patch clarifying the behavior of listdir. |
|
|
msg163896 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2012-06-25 05:35 |
Don't forget to point people to os.fsencode() if they actually wanted bytes, or os.fsdecode() if they already have bytes and want to convert them to text. |
|
|
msg163908 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 07:09 |
My first cut at a patch. Made the logic in posix_listdir easy to follow, fixed up the docstring and docs. Posted in the correct issue this time. (And Nick: I thought of that independently :D ) |
|
|
msg163921 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 09:49 |
Removed "do we have os.listdir" checks from the unit tests. Yes, Virginia, we always have os.listdir in Python 3.3. |
|
|
msg163930 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 10:32 |
Regenerated patch to make Reitveld happy. |
|
|
msg163932 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 10:41 |
GRAHH HULK SMASH Regenerating again, because I wasn't actually fresh enough last time. |
|
|
msg163944 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-06-25 11:42 |
New changeset 8172d8e907a4 by Larry Hastings in branch 'default': Issue #15176: Clarified behavior, documentation, and implementation http://hg.python.org/cpython/rev/8172d8e907a4 |
|
|
msg163951 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-25 11:50 |
Good news, everyone! |
|
|