-
Notifications
You must be signed in to change notification settings - Fork 463
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
fix: FileSystem.normalize #6983
base: master
Are you sure you want to change the base?
Conversation
Implement the TODO to handle multiple path separators and the . special character in paths
4c13822
to
7356177
Compare
Mathlib CI status (docs):
|
Thanks for the PR, was this implementation taken from some other stdlib? This definitely isn't something we should just reinvent. |
Nope, this is just some functionality that I personally needed (and that I saw the TODO comment for), I wrote it by hand based on what seemed reasonable. |
Ok, then I don't think we can upstream it as is |
What would be necessary in order to upstream it? |
The implementation should be based on one from another reputable stdlib such that we do not repeat mistakes that other people have made and already have support for weird edge cases early on. Additionally we should probably have tests to demonstrate that we do indeed manage to handle such edge cases. |
I think the Rust stdlib PathBuf normalization mostly happens in this method: https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.components which might be a good guide for this lean implementation |
I've converted this to a draft until that point |
This PR implements the TODO comment here, to handle multiple path separators and the . special character when normalizing file paths.