Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isAbsolute(r'\') should return false for Windows-style filesystem #143

Open
aiurovet opened this issue May 1, 2023 · 4 comments
Open

isAbsolute(r'\') should return false for Windows-style filesystem #143

aiurovet opened this issue May 1, 2023 · 4 comments
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@aiurovet
Copy link

aiurovet commented May 1, 2023

As per comment by @jamesderlin at google/file.dart#198 (comment) on the issue related to the file package,

The expectation is that _context.isAbsolute(path) logically implies that path includes style.drive as a prefix, which isn't true in this case. Seems like the fix for that might be more appropriate for package:path, but I'm reluctant to touch that because I'm not sure what changes are planned (or when) for v2.0 of that package.

@lrhn lrhn changed the title isAbsolute(r'\') returns false for Windows-style filesystem isAbsolute(r'\') should return false for Windows-style filesystem May 1, 2023
@lrhn
Copy link
Member

lrhn commented May 1, 2023

From the documentation of isAbsolute on Windows, returning false for r"\" is the correct and desired behavior.

It's doesn't do that. Tested on DartPad:

import "package:path/path.dart" as path;
void main() {
  var c = path.Context(style: path.Style.windows);
  print(c.isAbsolute(r'')); // false
  print(c.isAbsolute(r'\')); // true ??
  print(c.isAbsolute(r'\\')); // true
  print(c.isAbsolute(r'a:')); // false
  print(c.isAbsolute(r'a:\')); // true
}

I think the bug is in this line:
https://github.com/dart-lang/path/blob/master/lib/src/style/windows.dart#L51

    if (path.length < 2 || path.codeUnitAt(1) != chars.backslash) return 1;

where it should return 0 as "root length" for a single \ input, so:

    if (path.length < 2) return 0;
    if (path.codeUnitAt(1) != chars.backslash) return 1;

@lrhn lrhn added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label May 1, 2023
@jamesderlin
Copy link

From the documentation of isAbsolute on Windows, returning false for r"\" is the correct and desired behavior.

Yes. When I wrote "The expectation is that _context.isAbsolute(path) logically implies that path includes style.drive as a prefix, which isn't true in this case", I meant that the implication (the boolean logic operation) wasn't true, not that _context.isAbsolute(path) wasn't returning true. _context.isAbsolute(r'\') currently returns true but should be returning false. Sorry for not being clearer.

@aiurovet
Copy link
Author

aiurovet commented May 1, 2023

OK, I'm happy to close the issue.

@aiurovet aiurovet closed this as completed May 1, 2023
@jamesderlin
Copy link

No no, isAbsolute is returning the wrong value, which is definitely a bug.

@devoncarew devoncarew reopened this May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants