dart-lang/path

relative() and isWithin() are not case insensitive on Windows

Closed this issue · 5 comments

This:

import 'package:path/path.dart' as p;

main() {
  print(p.windows.isWithin(r"a\b", r"a\B\c"));
  print(p.windows.isWithin(r"a:b\c", r"A:\b\c\d"));
  print(p.windows.relative(r"a:\b\c\d", from: r"a:\B\c"));
  print(p.windows.relative(r"a:\b\c\d", from: r"A:\b\c"));
}

Prints:

false
false
..\..\b\c\d
d

The first three are wrong. Interestingly, the last is right!

So isWithin() is not handling case sensitivity of drive letters or paths. relative() is handling drive letters, but not paths.

nex3 commented

This also raises the question of how to handle normalize(). Currently, normalize() is the best way to create a map that uses path keys that doesn't count textually-different paths as identical (watcher does this, for example). In order to make that safe, we'd have to make normalize() downcase paths on Windows as well.

Normalization is also expected to make a path more human-readable though, and wiping the case makes it less readable. So maybe the thing to do is add p.equals() and p.hash() methods that users can pass to new HashMap() to create a proper path map (or set).

So maybe the thing to do is add p.equals() and p.hash() methods that users can pass to new HashMap() to create a proper path map (or set).

+1 for equals().

I'm not crazy about hash() since I think users might expect that to return the actual hash code and not a string-which-can-be-hashed-to-get-a-good-hash-code.

Maybe canonicalize, or hashNormalize?

nex3 commented

I'm not crazy about hash() since I think users might expect that to return the actual hash code and not a string-which-can-be-hashed-to-get-a-good-hash-code.

The intention is for it to return the hash code, so it can be passed into new HashMap(). We could also add canonicalize() if you think it would be useful, although I can't come up with a use-case off the top of my head.

The intention is for it to return the hash code, so it can be passed into new HashMap().

Ah, I see. Yeah, that could work, though I feel HashMap() is pretty far off the beaten path. My hunch is most users would intuitively expect to use a vanilla map and just canonicalize their path strings such that equivalent paths become equivalent strings.

nex3 commented

We can totally add canonicalize() then—it's certainly easy enough to implement. Maybe we can point the user to HashMap() for more efficiency, too.