Skip to content

feat: add Unicode normalization for path handling and detect case-insensitive filename collisions #1621

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

Merged
merged 5 commits into from
May 6, 2025

Conversation

zelosleone
Copy link
Collaborator

closes #1262 hopefully, i am only confident with unit tests but hopefully it passes mac os!

@zelosleone zelosleone marked this pull request as draft May 5, 2025 15:19
@zelosleone zelosleone marked this pull request as ready for review May 5, 2025 16:11
@zelosleone
Copy link
Collaborator Author

@wolfv forgot to copy the case sensitive files to PREFIX folder :DD thankfully it passes now

@zelosleone
Copy link
Collaborator Author

Alright, ubuntu passes, macos doesn't, i will need to check that out too

@zelosleone
Copy link
Collaborator Author

@wolfv We cannot create the test files at all on mac os due to case insensivity, same as windows :/ even if i try to make a package it would fail sadly. but i think unit tests + e2e test working on linux could be enough?

@wolfv
Copy link
Member

wolfv commented May 5, 2025

Sounds good indeed!

fn test_find_case_insensitive_collisions_unicode() {
let files = vec![
Path::new("foo/straße"),
Path::new("foo/STRASSE"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these two (straße and STRASSE) would be the same on case-insensitive file systems.. or would they?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, they are! Just checked locally on macOS and this is true :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! I also checked İSTANBUL and istanbul as well, it should work correctly for all languages!

@wolfv wolfv merged commit eca3d8f into prefix-dev:main May 6, 2025
17 checks passed
@zelosleone zelosleone deleted the casesensivity branch May 6, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a warning for packages with mixed case
2 participants