Directory.parent always returns "." for relative path

[sgrekhov]

  Directory dir = new Directory("SomeDir");
  print(dir.parent); // Directory: '.'
  print(dir.parent.parent.parent); // Directory: '.'

Tested on Dart VM version: 1.24.0-dev.5.0 (Fri May 19 01:47:01 2017) on "windows_x64"

[lrhn]

Can you tell us what you expected to happen? Should it resolve itself against the current working directory and traverse from there?
Or should it start adding ".." prefixes instead, so you would print:

.
../..

instead?

What it does is to treat the argument as a relative path, and not assume anything about what is above the beginning of that path.
As a path, there is no parent information in the example. If you had written:

var dir = new Directory("a/b/c");

then you would have another relative directory, and the parent getter would give you a non '.' path twice ('a/b/ and a/) before reaching '.'.

[sgrekhov]

// There is a directory 'D:\Google\dart\co19\LibTest\io\Directory\TestDir'
// Test I run is in D:\Google\dart\co19\LibTest\io\Directory, 

  Directory dir = new Directory(Directory.current.path +
      Platform.pathSeparator + "TestDir");
  print(dir.parent); // Directory: 'D:\Google\dart\co19\LibTest\io\Directory'
  print(dir.parent.parent); // Directory: 'D:\Google\dart\co19\LibTest\io'
  print(dir.parent.parent.parent); // Directory: 'D:\Google\dart\co19\LibTest'

  dir = new Directory("TestDir");
  print(dir.parent); // Directory: '.'
  print(dir.parent.parent); // Directory: '.'
  print(dir.parent.parent.parent); // Directory: '.'

In both cases dir is pointing to the same directory. Therefore I'd expect that parent (and any other property) should behave in the same way no matter which way I choose to get this directory (relative path or absolute or, may be, URI). Otherwise it surprises me

[lrhn]

Actually, the relative Directory object doesn't point to a specific directory by itself.

The current path can be changed (it's has a setter), and it if you do that, the meaning of the relative Directory will change with it. Example:

import "dart:io";
main() async {
  var dir = new Directory(".");
  print(dir.absolute);
  dir.listSync().forEach(print);
  print("---------");
  Directory.current = new Directory("/tmp/");  // or somewhere else that works for you.
  print(dir.absolute);
  dir.listSync().forEach(print);
}

So, the relative directory doesn't have a fixed path, it's only when you use it that it is temporarily resolved relative to the current path.
That also means that it doesn't have a fixed parent, and when you move above the path that the directory itself knows about, it has to give up.

We could change that - make it go to ".." instead, or act like the absolute path when you do .parent (maybe only if you go above the start of the relative path), but I'm not sure whether that would be more or less surprising.

[sgrekhov]

I'd expect something like

  Directory dir = new Directory("a/b/c");
  print(dir.parent.path); // a/b
  print(dir.parent.parent.path); // a
  print(dir.parent.parent.parent.path); // "."
  print(dir.parent.parent.parent.parent.path); // ".."
  print(dir.parent.parent.parent.parent.parent.path); // "../.."

[gaaclarke]

but I'm not sure whether that would be more or less surprising.

Having relative paths implicitly based off of the current working directory is ubiquitous. Deviating from that is surprising. I would even argue that our current behavior is incorrect in that it is never useful. When is Directory('..').parent.path == '.' ever useful? "The parent of the parent directory of the current working directory is the current working directory." It's like some circular logic they use to destroy robots in 1950's sci-fi.

[lrhn]

The Directory class is not very clever. At all.
It's really just an uninterpreted "path string", and .parent removes everything from the last / (or \, or start of the path if there is no separator) and forward. If there is nothing left, it becomes ..
It doesn't actually understand what it's doing. The path is not normalized, it's not interpreted. It really just looks for the last path separator and blindly cuts the path off there.

If you do:

var d = Directory("a/b/c/.././d");

then

print(d);                      // a/b/c/.././d
print(d.parent) ;              // a/b/c/../.
print(d.parent.parent) ;       // a/b/c/..   <- Not going up.
print(d.parent.parent.parent); // a/b/c  <- Now going deeper!

We could make Directory("..").parent be Directory("..") (add /.. to any path ending in /.. or being .), but that's a lot more intelligence than the Directory class shows anywhere else.

[gaaclarke]

We could make Directory("..").parent be Directory("..") (add /.. to any path ending in /.. or being .), but that's a lot more intelligence than the Directory class shows anywhere else.

That sounds good to me, why don't we do that? It's hardly any more logic and gives correct results. Today we are using '.' to signify an error in the algorithm which is the worst thing we can do since it can be interpreted as a valid response. Throwing an exception to Directory('..').parent would be better imo.

// This obviously should be refactored to a single pass function for performance.
String parent(String path) {
  if (path.endsWith('/..') || path.endsWith('/.') || path == '.' || path == '..') {
    return '$path/..';
  } else {
    String? stripped = stripLastComponent(path);
    return stripped ?? '.';
  }
}

[gaaclarke]

@lrhn do I have to make a formal proposition to make this change? I'm happy to implement it if we want it.

[lrhn]

Let's consider this issue as that request.

We can noodle on the details, but starting out with just the above should be a valid start:

If the path ends in a path segment of the form . or .., then the parent path is found by appending ${pathSeparator}...
If the path becomes empty after removing the last segment, the parent path is ..

(Plus all the Windows-special-path things it already does, which should not interfere with those rules.)

We could get cleverer:

• If the path ends with a segment of the form, ., remove it before finding the parent.
• If the path ends with one or more .. segments, remove it and recursively find the parent of the remaining path, then take the parent of that. (Or however we specify that normalization).

That way, we are not blindly removing a trailing . or .. segment and treating it as a downwards directory name.

Full normalization costs, but doing tail-only normalization during the parent operation seems reasonable.

[gaaclarke]

Here's a quick take. I'll have to look at the windows special cases. I don't think we should simplify the path since that sounds like useful functionality that could be a different function and it would require more compute. A statemachine that gobbles up the end of the path can do it. Another weird case is Directory('/').parent.path == '/'. I'm not sure if we want to address that too. I would probably throw an error.

const int _pathSeparator = 47;
const int _dot = 46;
const String _pathSeparatorString = '/';

String parent(String path) {
  int chop = 0;
  int state = 0;
  for (int ch in path.codeUnits.reversed) {
    chop += 1;
    if (state == 0) {
      if (ch == _dot) {
        state = 2;
      } else if (ch == _pathSeparator) {
        // pass
      } else {
        state = 1;
      }
    } else if (state == 1) {
      if (ch == _pathSeparator) {
        state = 4;
      }
    } else if (state == 2) {
      if (ch == _dot) {
        state = 3;
      } else if (ch == _pathSeparator) {
        return '${path.substring(0, path.length - chop)}$_pathSeparatorString..';
      } else {
        state = 1;
      }
    } else if (state == 3) {
      if (ch == _pathSeparator) {
        return '$path$_pathSeparatorString..';
      } else {
        state = 1;
      }
    } else if (state == 4) {
      if (ch != _pathSeparator) {
        return path.substring(0, path.length - chop + 1);
      }
    }
  }
  if (state == 0 || state == 4) {
    return _pathSeparatorString;
  } else if (state == 2) {
    return '..';
  } else if (state == 3) {
    return '$path$_pathSeparatorString..';
  } else {
    return '..';
  }
}

void main() {
  assert(parent('/foo/bar') == '/foo');
  assert(parent('/foo/bar/') == '/foo');
  assert(parent('..') == '../..');
  assert(parent('.') == '..');
  assert(parent('hello') == '..');
  assert(parent('/foo/bar.') == '/foo');
  assert(parent('foo/..') == 'foo/../..');
  assert(parent('foo/.') == 'foo/..');
  assert(parent('/') == '/'); // Matching old behavior.
  assert(parent('/foo//bar') == '/foo');
  assert(parent('//foo') == '/');
}

[gaaclarke]

I made a little workspace for the single pass rewrite of parentOf that I'll contribute to as I have time. It's just a bit more convenient to work there so I can fake Windows and just run the tests directly: https://github.com/gaaclarke/dart_parentOf . All of our posix tests are passing and about half of the windows ones. I'll probably have to do 2 passes for Windows, one starting at the front and one starting at the back.