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

Handle file extensions case sensitive #3542

Merged
merged 9 commits into from
May 21, 2024

Conversation

jojo2357
Copy link
Contributor

@jojo2357 jojo2357 commented May 4, 2024

Fix #3537 on how to test.

What was happening was twofold: the casing on extensions was being thrown out at command definition, and when testing different exts, it would also discard the requested casing. This doesnt matter on 🪟, because checking for little jpg or big JPG would give the same result from the fs.

@PHPirates
Copy link
Collaborator

Thanks! Could you add a test as well?

@jojo2357
Copy link
Contributor Author

All tests passed locally

Copy link
Collaborator

@PHPirates PHPirates left a comment

Choose a reason for hiding this comment

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

Great! I just made a change to make the tests less confusing.

@jojo2357
Copy link
Contributor Author

Ready to go if you are

@jojo2357
Copy link
Contributor Author

Forgot to mention, #3549 is not fixed here. Do you want to do that here? I was thinking it would need more discussion and refactoring and could be its own PR.

@jojo2357 jojo2357 marked this pull request as draft May 16, 2024 22:03
@jojo2357
Copy link
Contributor Author

Yeah lets tackle the other issue too

@PHPirates
Copy link
Collaborator

Yes that can be a separate PR, whatever you want

@jojo2357
Copy link
Contributor Author

Im stuck on the test where adding an extension with different casing is supposed to ignore the casing if the letters are the same. Makes no sense to me.

@PHPirates
Copy link
Collaborator

I'm not sure what you mean, but if you create a PR with a test that is not working properly I might be able to help

@jojo2357
Copy link
Contributor Author

theres your failing tests

@PHPirates
Copy link
Collaborator

Im stuck on the test where adding an extension with different casing is supposed to ignore the casing if the letters are the same. Makes no sense to me.

Now I understand your comment, if you were wondering why extensions were case insensitive: the code was probably written (six years ago) with only Windows in mind. However, even though some things might technically work on Windows I think we should encourage LaTeX that also works on Linux, so that failing test is not something we want to support anymore.

I have one failing test locally because it finds myOtherPicture.png but that file doesn't exist anymore, strange.

@PHPirates PHPirates marked this pull request as ready for review May 18, 2024 12:42
# Conflicts:
#	src/nl/hannahsten/texifyidea/completion/pathcompletion/LatexPathProviderBase.kt
@jojo2357
Copy link
Contributor Author

I have one failing test locally because it finds myOtherPicture.png but that file doesn't exist anymore, strange.

I saw that too. Just occurred to me that I should maybe delete the build dir

@PHPirates PHPirates changed the title Fix #3537 Handle file extensions case sensitive May 21, 2024
@PHPirates PHPirates merged commit f0b3cdf into Hannah-Sten:master May 21, 2024
10 checks passed
@PHPirates PHPirates added this to the Next milestone May 21, 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.

Image ending in .JPG missing false positive
2 participants