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

FileSystemDirectoryHandle.resolve is a very vague method name, consider renaming it #4

Open
smaug---- opened this issue Jan 20, 2022 · 3 comments

Comments

@smaug----
Copy link

"resolve" doesn't hint too much what it is actually doing. One needs to read the algorithm to see what it actually returns.

@a-sully
Copy link
Collaborator

a-sully commented May 27, 2022

I agree that the name is not great. Current usage is still low so I'm not necessarily opposed to a name change, but it would be a bit of a hassle to deprecate the current name. Do you think the benefits of renaming this method would be worth that hassle?

If so, the next question is what this should be named instead. Something like getRelativePath() or getRelativePathComponents() is more descriptive of what's happening here (since we don't actually return the path, but an array of path components). While we're at it, reversing the parameter order might make more sense...

Current: dir.resolve(file)

Alternatives:

  • file.getRelativePath(dir)
  • file.getRelativePathComponents(dir)
  • file.getPathRelativeTo(dir)
  • others?

@jesup
Copy link
Contributor

jesup commented May 31, 2022

I would be in favor of renaming it - the current name is really bad.
Of those, I like file.getPathRelativeTo(dir)

@poirierlouis
Copy link

Hello, I don't know if I'm trespassing by commenting here. Feel free to remove this if necessary.

Do you think the benefits of renaming this method would be worth that hassle?

IMHO, it would be worth, as resolve is not particularly explicit. Someone discovering the API would need to read the documentation to be certain of what this method does. I believe alternatives proposed are more explicit when discovering this API, as 'path' and 'relative' makes direct sense in API's context.

I also like file.getPathRelativeTo(dir).

While we're at it, reversing the parameter order might make more sense...

Do you mean it would be called like this dir.getPathRelativeTo(file) / dir.resolve(file)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants