path: Fix joining Windows path when the receiver is "C:" #11579
Conversation
WindowsPath::new("C:").join("a") produces r"C:\a". This is incorrect.
It should produce "C:a".
|
No. |
|
@liigo Unless Windows 7 changed paths dramatically, |
|
@kballard As a user of Windows for over 10 years, I always use |
|
@liigo Well yes, |
WindowsPath::new("C:").join("a") produces r"C:�". This is incorrect.
It should produce "C:a".
|
@liigo |
|
@chris-morgan "...process's working directory on the C: drive..." A windows process has only one current directory, not one per drive. The concept of a current directory per-drive (and the use of C: to refer to the current directory on drive C) only exists within cmd.exe, not in the Windows API. There is no useful action a rust program can perform with a drive-relative path. This patch should be reverted. The correct behaviour of WindowsPath::new("C:") is to error or to be equivalent to WindowsPath::new("C:") (The other option is for rust to resurrect the concept of a current directory per drive. This will interact poorly with non-rust libraries that use relative paths or alter the current directory, and will be not be understood by windows programmers. Don't do this.) See also: http://blogs.msdn.com/b/oldnewthing/archive/2010/10/11/10073890.aspx |
|
@ytinasni If this is true, than we need to do more than just revert this patch. There's other behavior in What does the Windows API do if you try and use a path that looks like |
|
https://gist.github.com/ytinasni/8507921 Summary:
As a user, I would expect that "C:" refers to C:, and "C:a" is an error. I request that Rust implement this. I maintain my opinion that there is no useful action a rust program can perform with a drive-relative path. For comparison, in c# the following throws an exception. |
|
@ytinasni We don't currently have a notion of an illegal path. There are paths that perhaps don't parse the way you expect, but the only way to actually throw an error is if your path contains non-utf8 (on Windows) or NUL. I don't think it's appropriate to make e.g. "C:a\b" throw an error. Given that, we need to figure out some way to parse "C:a" that makes sense. We could just interpret it as "C:\a", although that strikes me as less than ideal. |
|
It's bad to be merged. I'll give it a -1. @erickt
On Windows (after Windows 95), the |
|
The summary of the situation appears to be that drive-relative paths are now in fact down to the level of "broken Windows feature", functioning as it did in the days of long ago in some places, not functioning at all in others and implemented brokenly in others. (Thanks for the clarifications there, @ytinasni.) My own experience (mostly only up to XP) has been that most things work with drive-relative paths, but not quite all (mostly those things that would be doing their own path management rather than using native APIs directly). On a nearby Windows 7 machine with lots of tools on it from the mid '90s, I just tried the GNU @liigo We do not care whether people "should" or "should not" use such things: that is not our domain unless we do rigorous validation of absolutely everything in it at great expense—a model that was tried earlier, weighed in the balances and found wanting for various reasons. What we care about is what behaviour we use when we do encounter such paths, which we inevitably will. Please be careful about taking solely from your own experiences and declaring from that alone that a feature is bad and should not be used—especially when designing libraries, one must necessarily be liberal in what one accepts. Also, as noted by @kballard, reverting this patch alone is not an option; we must do something, one way or another, consistently throughout @kballard I am of the opinion that the behaviour that we now have is entirely acceptable and most likely to be what is desired by most users. |
|
If this is only partially supported by applications written by Microsoft itself, I'd be in favor of undoing this patch. To handle this error, we could add a |
|
@erickt The python behaviour is correct. The filename 'c:foo' can refer to an Alternate Data Stream (on ntfs) or an actual file (e.g. a UNC path that refers to a file on a linux server.) |
[`manual_let_else`]: only omit block if span is from same ctxt Fixes rust-lang#11579. The lint already had logic for omitting a block in `else` if a block is already present, however this didn't handle the case where the block is from a different expansion/syntax context. E.g. ```rs macro_rules! panic_in_block { () => { { panic!() } } } let _ = match Some(1) { Some(v) => v, _ => panic_in_block!() }; ``` It would see this in its expanded form as `_ => { panic!() }` and think it doesn't have to include a block in its suggestion because it is already there, however that's not true if it's from a different expansion like in this case. changelog: [`manual_let_else`]: only omit block in suggestion if the block is from the same expansion
WindowsPath::new("C:").join("a") produces r"C:\a". This is incorrect.
It should produce "C:a".