Fix case sensitive check to be case insensitive - #192
Conversation
Case sensitivity between os.getcwd and os.realpath can fail due to different drive letter casing. C:\ vs c:\. This change addresses that by normalizing the strings before comparing.
lstein
left a comment
There was a problem hiding this comment.
This looks fine. I didn't spot any issues.
Will this automagically fix pull requests that use CRLF rather than Unix LF? That would be amazing! |
| cwd = os.getcwd().lower() | ||
| is_in_cwd = os.path.commonprefix((os.path.realpath(path).lower(), cwd)) == cwd |
There was a problem hiding this comment.
@david-ford Can you check if the following works instead?
| cwd = os.getcwd().lower() | |
| is_in_cwd = os.path.commonprefix((os.path.realpath(path).lower(), cwd)) == cwd | |
| cwd = os.path.realpath(os.getcwd()) | |
| is_in_cwd = os.path.commonprefix((os.path.realpath(path), cwd)) == cwd |
If that works, it would be better, since then the check is still correct on case-sensitive filesystems.
There was a problem hiding this comment.
Yeah, this looks better because on Linux we could have ./static and ./Static side-by-side and they shouldn't both pass the test. Is there a specific test case for case-sensitive filesystems that I can check?
There was a problem hiding this comment.
I couldn't repro the issue personally, but a user tested my suggestion in #196 (comment) and reports that it worked, so it should be ok. Do you want to just add that in while merging this PR? Alternatively I could open a new one with just that change.
There was a problem hiding this comment.
Yeah, this looks better because on Linux we could have ./static and ./Static side-by-side and they shouldn't both pass the test. Is there a specific test case for case-sensitive filesystems that I can check?
I'm using Windows 10, with my environment setup following the instructions in the readme using Anaconda. When I do os.getcwd() I get c:\path\to\project and when I do os.realpath I get C:\path\to\project.
I believe the suggestion here will work, but my PC is currently training so it will be another hour~ before I can get this updated. Strangely, the original dataset has no idea what a scythe is.
As mentioned below, I'll pull the unrelated changes from this PR, and then commit them in separate PRs.
There was a problem hiding this comment.
I couldn't repro the issue personally, but a user tested my suggestion in #196 (comment) and reports that it worked, so it should be ok. Do you want to just add that in while merging this PR? Alternatively I could open a new one with just that change.
I made those changes and got the PR updated for ya. I can confirm your suggestion worked, and it's so much cleaner than the way I was trying to do it.
That would indeed be the intention. It won't fix old items, but I went ahead and already ran the command to update them based on the config in this pr. There weren't any because you've been diligent. What it will do is if your local machine uses CRLF and the repo uses LF, it will convert line endings automagically on commit to LF, and when doing a pull, it will convert them back to CRLF for the local user. As for the other changes, I will break this into three branch/commits. One for the CRLF <> LF, one for the suggested fix for the case insensitive match, and one for the HTML changes -- minus the placeholder for now. |
Removed the changes to the index.html and .gitattributes for this PR. Will add them in separate PRs. Applied recommended change for resolving the case issue.
Fix case sensitive check to be case insensitive
…locator-credit fix(model cache): credit the allocator's reclaimable reserve in the VRAM budget
Case sensitivity between os.getcwd and os.realpath can fail due to different drive letter casing. C:\ vs c:. This change addresses that by normalizing the strings before comparing. Otherwise this will throw a 404 for files like index.js
Includes a .gitattributes file to auto normalize text files (ignores images and such) so that line endings get normalized on commit but local line endings for devs remain unchanged. Git will handle the translation to and from on commit.
Some minor changes to the HTML for the web version to include tags expected by browsers and some accessibility.