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

Detect import paths that only differ in capitalization #1295

Merged
merged 8 commits into from
Dec 11, 2023
Merged

Conversation

konnov
Copy link
Contributor

@konnov konnov commented Dec 7, 2023

Closes #1194. This PR adds detection of filenames that use different capitalization. If this is the case, the parser flags an error. We have been discussing the option of storing hashes instead of normalized filenames, but this seems to be an overkill at the moment.

  • Tests added for any new code
  • Entries added to the respective CHANGELOG.md for any new functionality

@konnov konnov changed the title detect different case filenames Detect import paths that only differ in capitalizatio Dec 7, 2023
@konnov konnov changed the title Detect import paths that only differ in capitalizatio Detect import paths that only differ in capitalization Dec 7, 2023
@konnov konnov marked this pull request as ready for review December 7, 2023 17:03
quint/cli-tests.md Outdated Show resolved Hide resolved

<!-- !test exit 1 -->
<!-- !test check parse testFixture/_1060case.qnt - parse case sensitive filenames -->
quint parse testFixture/_1060case.qnt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also check if the error matches the expected? I think this test would pass even before your change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also wanted to do that. But it's a bit hard to do, since the quint parse outputs absolute paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed this test by moving it to io-cli-tests.md and comparing the outputs.

Copy link
Contributor

@thpani thpani left a comment

Choose a reason for hiding this comment

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

left one comment, rest LGTM

quint/src/parsing/quintParserFrontend.ts Outdated Show resolved Hide resolved
@konnov
Copy link
Contributor Author

konnov commented Dec 11, 2023

I think I have addressed all of the comments. Please have a look

Copy link
Contributor

@thpani thpani left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@konnov konnov merged commit ead3c89 into main Dec 11, 2023
15 checks passed
@konnov konnov deleted the igor/filenames1194 branch December 11, 2023 13:51
@shonfeder shonfeder mentioned this pull request Jan 3, 2024
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.

Importing by path breaks on case-insensitive file systems
3 participants