Message163242
I'm not sure why there isn't a review link for your patch.
"which(file..."
I don't think file is a good name. 'fn' or 'filename' or 'string' would all be better choices, I think. 'file' implies a Python file object (to me). And "the give *file* command is called" just looks wrong. "Yield the full path to the executables which could be run if the given *filename* were looked up...."
Wait, why are we even returning more than one result? I don't see any use cases for that in the issue (though I admit I just skimmed it). The unix which command doesn't. Wanting only the first match is going to be far more common than wanting a list, so it should be the case supported most easily by the API. If getting the complete list is useful, the API can be extended or a new function added later.
Then the motivation statement becomes clearer: "Yield the full path to the executables which would be run if the given *filename* were typed at the shell prompt or passed to the os.exec*p* functions."
Given that the function works on Windows, its behavior (looking in the current directory and in PATHEXT) should be documented. And if that's not what exec*p* does on windows, that should be noted, I think (or fixed?)
I'm also not sure what the 'path' argument is for. Does it have a use case? If not, drop it.
You are doing the PATHEXT extraction regardless of platform, which I don't think is a good idea.
This line appears to be useless:
base = os.path.join(dir, file)
Per my suggestion of dropping the list form (at least for now), the yield should become a return, with the default return value of None indicating not found. |
|
Date |
User |
Action |
Args |
2012-06-20 01:37:50 | r.david.murray | set | recipients:
+ r.david.murray, loewis, tmick, edemaine, pitrou, vstinner, wrstlprmpft, giampaolo.rodola, christian.heimes, ajaksu2, sfllaw, schmir, tarek, eric.araujo, Christophe Simonis, andybuckley, weeble, tleeuwenburg@gmail.com, michael.foord, brian.curtin, meatballhat, sandro.tosi, iki, Iztok.Kavkler, Omega_Weapon |
2012-06-20 01:37:49 | r.david.murray | set | messageid: <1340156269.22.0.102067071459.issue444582@psf.upfronthosting.co.za> |
2012-06-20 01:37:48 | r.david.murray | link | issue444582 messages |
2012-06-20 01:37:44 | r.david.murray | create | |
|