Handle relative paths where the trailing directory has an extension.

utils/pub/path.dart

Index: utils/pub/path.dart
diff --git a/utils/pub/path.dart b/utils/pub/path.dart
index 3b889d380654bd50e2d75c97e3e8f3e5964bfc15..19a5a62ee09252695732b845aa2717ef1550c201 100644
--- a/utils/pub/path.dart
+++ b/utils/pub/path.dart
@@ -312,14 +312,15 @@ class Builder {
   ///
   ///     builder.withoutExtension('path/to/foo.dart'); // -> 'path/to/foo'
   String withoutExtension(String path) {
-    var lastSeparator = path.lastIndexOf(separator);
-    var lastDot = path.lastIndexOf('.');
+    var parsed = _parse(path);
+    if (parsed.hasTrailingSeparator) return parsed.toString();

[nweiz, 2012/12/11 02:09:42]

I really don't like changing behavior based on whether or not there's a trailing
separator. Most programs treat "foo/" the same as "foo", and those that don't
are inconsistent about what "foo/" means.

[Bob Nystrom, 2012/12/11 02:24:59]

This is how Python works. I think it would be confusing if
withoutExtension("foo.bar/") returned "foo". I believe the expectation is that
it will strip the extension from the file name, if there is a file at the end of
the path. If you have a trailing separator, I don't think there's a file at the
end of the path.

[nweiz, 2012/12/11 02:34:46]

But not Ruby :).
I disagree. I believe the expectation is that it will strip the extension from
the entry pointed to be the path (in this case, "foo.bar").

I think it's easier to see for the dual, extension(). It's relatively common
that directories will have meaningful extensions that users will want to
extract, and it's also relatively common that paths come directly from user
input. We could be introducing subtle bugs where "some_command foo.ext" on the
command line works, but "some_command foo.ext/" does not.

Here's a very real-world example: some Git servers use the ".git" suffix on
directories to determine if a directory is supposed to be Git. It would be
potentially confusing if such a server were sensitive to the presence of a slash
when finding directories.

It also forces users (and you, the API designer) to consider every API's
behavior with respect to trailing slashes. For example, does dirname("foo/bar")
return "foo" or "foo/"? Why?

[Bob Nystrom, 2012/12/11 02:47:01]

I'm more interested here in what dirname("foo/bar/") returns. Python and Ruby
diverge there too: Python returns "foo/bar" and Ruby "foo".

I'm OK with changing this, but we should change it consistently across all
methods. They should all ignore trailing separators or treat them as meaningful
(essentially as denoting a path with a trailing unnamed file).

 
-    // Ignore '.' in anything but the last component.
-    if (lastSeparator != -1 && lastDot <= lastSeparator + 1) lastDot = -1;
+    if (!parsed.parts.isEmpty) {
+      var split = parsed._splitExtension();
+      parsed.parts[parsed.parts.length - 1] = split[0];

[nweiz, 2012/12/11 02:09:42]

Calling a private method here seems like a code smell. Why can't you do
`parsed.parts[parsed.parts.length - 1] = parsed.filenameWithoutExtension`
instead?

[Bob Nystrom, 2012/12/11 02:24:59]

Done, though I don't think there's anything wrong with using a private here.
Privacy is library-level and it's all in the same library. I like library (as
opposed to class privacy). I also like "friend" in C++. :)

+    }
 
-    if (lastDot <= 0) return path;
-    return path.substring(0, lastDot);
+    return parsed.toString();
   }
 
   _ParsedPath _parse(String path) {
@@ -345,22 +346,7 @@ class Builder {
       separators.add('');
     }
 
-    // Separate out the file extension.
-    var extension = '';
-    if (parts.length > 0) {
-      var file = parts.last;
-      if (file != '..') {
-        var lastDot = file.lastIndexOf('.');
-
-        // If there is a dot (and it's not the first character, like '.bashrc').
-        if (lastDot > 0) {
-          parts[parts.length - 1] = file.substring(0, lastDot);
-          extension = file.substring(lastDot);
-        }
-      }
-    }
-
-    return new _ParsedPath(style, root, parts, separators, extension);
+    return new _ParsedPath(style, root, parts, separators);
   }
 }
 
@@ -419,16 +405,15 @@ class _ParsedPath {
   String root;
 
   /// The path-separated parts of the path. All but the last will be
-  /// directories. The last could be a directory, or could be the file name
-  /// without its extension.
+  /// directories.
   List<String> parts;
 
   /// The path separators following each part. The last one will be an empty
   /// string unless the path ends with a trailing separator.
   List<String> separators;
 
-  /// The file's extension, or "" if it doesn't have one.
-  String extension;
+  /// The file extension of the last part, or "" if it doesn't have one.
+  String get extension => _splitExtension()[1];
 
   /// `true` if the path ends with a trailing separator.
   bool get hasTrailingSeparator {
@@ -439,21 +424,16 @@ class _ParsedPath {
   /// `true` if this is an absolute path.
   bool get isAbsolute => root != null;
 
-  _ParsedPath(this.style, this.root, this.parts, this.separators,
-              this.extension);
+  _ParsedPath(this.style, this.root, this.parts, this.separators);
 
   String get filename {
-    if (parts.length == 0) return extension;
-    if (hasTrailingSeparator) return '';
-    return '${parts.last}$extension';
-  }
-
-  String get filenameWithoutExtension {
-    if (parts.length == 0) return '';
+    if (parts.isEmpty) return '';
     if (hasTrailingSeparator) return '';
     return parts.last;
   }
 
+  String get filenameWithoutExtension => _splitExtension()[0];
+
   void removeTrailingSeparator() {
     if (separators.length > 0) {
       separators[separators.length - 1] = '';
@@ -505,10 +485,28 @@ class _ParsedPath {
     if (root != null) builder.add(root);
     for (var i = 0; i < parts.length; i++) {
       builder.add(parts[i]);
-      if (extension != null && i == parts.length - 1) builder.add(extension);
       builder.add(separators[i]);
     }
 
     return builder.toString();
   }
+
+  /// Splits the last part of the path into a two-element list. The first is
+  /// the name of the file without any extension. The second is the extension
+  /// or "" if it has none.
+  List<String> _splitExtension() {
+    if (parts.isEmpty) return ['', ''];
+    if (hasTrailingSeparator) return ['', ''];
+
+    var file = parts.last;
+    if (file == '..') return ['..', ''];
+
+    var lastDot = file.lastIndexOf('.');
+
+    // If there is a dot (and it's not the first character, like '.bashrc').

[nweiz, 2012/12/11 02:09:42]

This looks like it's supposed to go a couple lines further down.

[Bob Nystrom, 2012/12/11 02:24:59]

Reworded the comment.

+    if (lastDot <= 0) return [file, ''];
+
+    parts[parts.length - 1] = file.substring(0, lastDot);

[nweiz, 2012/12/11 02:09:42]

Why is this useful? Doesn't this mean that calling `parsed.extension` or
`parsed.filenameWithoutExtension` will destructively modify `parsed`?

[Bob Nystrom, 2012/12/11 02:24:59]

Eek! Copy/paste bug. Removed.

+    return [file.substring(0, lastDot), file.substring(lastDot)];
+  }
 }