Bug 10348 – isRooted is either wrong or poorly specified

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
All
OS
Windows
Creation time
2013-06-12T18:19:00Z
Last change time
2013-08-06T23:32:38Z
Assigned to
bugzilla
Creator
andrej.mitrovich
Blocks
10347

Comments

Comment #0 by andrej.mitrovich — 2013-06-12T18:19:15Z
----- import std.path; void main() { assert(!isAbsolute(r"\")); // ok assert(!isRooted(r"\")); // fails, path is rooted } ----- How can a non-absolute path be rooted? It doesn't make sense to me that r"\" itself is rooted (where is it rooted? which drive or network drive?). It's not a valid path on Windows. Another example: ----- import std.path; void main() { assert(!isAbsolute(r"\")); // ok assert(absolutePath(r"\") != r"\"); // fails, it returned r"\" } ----- So if r"\" is not an absolute path, how can the absolute path of r"\" be r"\" when it's not absolute? The docs for isRooted state: "Determines whether a path starts at a root directory." So in that case absolutePath() should have no problem creating an absolute path of such a rooted path. But you can't create an absolute path out of r"\". r"\" is not rooted anywhere. I think isRooted should have these tests: assert (!isRooted(`\`)); // changed assert (!isRooted(`\foo`)); // changed assert (isRooted(`d:\foo`)); assert (isRooted(`\\foo\bar`)); assert (!isRooted("foo")); assert (!isRooted("d:foo"));
Comment #1 by lt.infiltrator — 2013-06-12T18:48:55Z
The fixes for this and #10347 are trivial once isRooted and isAbsolute are properly defined.
Comment #2 by bugzilla — 2013-07-28T08:59:24Z
Surely \ is a valid path on Windows. Just open cmd.exe and run dir \ and it will list the files in the root directory of the current drive. I agree that absoluteDir(`\`) returns a wrong result. It should probably return rootName(base) instead. I'll take care of it. Whether isRooted(`\`) should be true or false is just a matter of definition. `\` refers to the *root directory* of the current drive, so it made sense to me to define it as true. Redefining it now would be a breaking change. If you think it is worth it, it should at least be brought up for community discussion first.
Comment #3 by andrej.mitrovich — 2013-07-28T13:24:17Z
(In reply to comment #2) > I agree that absoluteDir(`\`) returns a wrong result. It should probably > return rootName(base) instead. I'll take care of it. .net seems to return the drive path, e.g.: ----- using System; using System.IO; namespace ConsoleApplication1 { class Program { static void Main(string[] args) { string path1 = "\\"; string fullPath; fullPath = Path.GetFullPath(path1); Console.WriteLine("GetFullPath('{0}') returns '{1}'", path1, fullPath); } } } ----- Prints: GetFullPath('\') returns 'c:\' So I guess only 'absolutePath' needs to be fixed.
Comment #4 by bugzilla — 2013-07-31T10:42:18Z
Pull request 1442 coincidentally fixes this bug. https://github.com/D-Programming-Language/phobos/pull/1442
Comment #5 by github-bugzilla — 2013-08-06T14:38:59Z