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

Normalize drive letter in file path #980

Merged
merged 7 commits into from
Jun 25, 2024

Conversation

xt0rted
Copy link
Contributor

@xt0rted xt0rted commented Jun 17, 2024

I've been trying to figure out why my files don't always get picked up by this extension. I tracked it back to an issue in VSCode where opening a workspace using a different casing than what's on disk can cause the 'wrong' value to be passed into this function, and then matching doesn't work. Even after I corrected the config on my end I was still having an inconsistent experience. What I then found was my pattern had an uppercase C (#960) while my file path had a lowercase c. Normalizing both paths seems to fix this but I'm not sure if there's other spots that may need to be updated too.

Some background on the VSCode issue since it was driving me nuts for weeks, if not months. The folder my repo is in uses PascalCase, but I apparently added it to GitHub Desktop using camelCase. Every time I opened a file, or the workspace, from GitHub Desktop it would pass VSCode a path like c:/myProjects/Project, but on disk it's actually C:/MyProjects/Project. This meant the value of document.uri had a different case than the pattern had so picomatch wasn't returning true. This was made worse by VSCode caching these file paths in C:\Users\<user>\AppData\Roaming\Code\User\workspaceStorage\<workspace>\state.vscdb. Correcting my path in GitHub Desktop, deleting the cache, and the drive letter normalization change here looks to fix my problem and I get IntelliSense again.

# Conflicts:
#	packages/vscode-tailwindcss/CHANGELOG.md
@thecrypticace
Copy link
Contributor

I hadn't merged this because I wasn't ready to because I wanted to do more testing but after all the Windows issues some people have I'm going to because it's better than the status quo (plus in my testing it does fix at least one project with problems). Thanks for this!

Weird thing is that some of my projects work perfectly and then some don't and I can't figure out when VSCode decides C: vs c: sigh.

@thecrypticace thecrypticace merged commit 52858d5 into tailwindlabs:master Jun 25, 2024
@xt0rted xt0rted deleted the patch-1 branch June 25, 2024 16:17
@thecrypticace
Copy link
Contributor

And I think I figured it out. At some point somewhere the vscode-uri package updated for fsPath to normalize the drive letter to lowercase on Windows.

@xt0rted
Copy link
Contributor Author

xt0rted commented Jun 25, 2024

Weird thing is that some of my projects work perfectly and then some don't and I can't figure out when VSCode decides C: vs c: sigh.

@thecrypticace I had the same issue. An Astro based project worked fine, but an ASP.NET project didn't. The ASP.NET project is the one I was having all the issues with which was leading me to think it was coming from one of those extensions, or just a lack of support for Razor views/syntax which is an issue but is unrelated.

Even with v0.12.1 I still wasn't getting IntelliSense in one of my workspaces, and what I just realized is I had it pinned in my taskbar with the same incorrect casing that GitHub Desktop was using. Un-pin, re-open, re-pin and now it's working.

Picomatch has two options not currently used, one is for Windows and the other is for case sensitivity. My first attempt at fixing this set the case sensitivity flag if the OS was Windows. That worked but still has room for error since an equality check could fail even though picomatch would pass, and then you're chasing other random bugs.

From what I was reading in the VSCode issues they don't make any guarantees that the casing of the file path you get on Windows is accurate. A commonly reported issue from extension authors is open a workspace with Index.js, then rename it to index.js, and the extension will continue to get Index.js until you restart the app.

Their stance seems to be extension authors who care about casing should use the path they're given and check with the filesystem via fs.realpathSync.native() to get the actual path before using it. That seems crazy to me and I would expect them to do that once and be the source of truth instead of putting the burden on every extension.

The ESLint extension had to work around this exact issue, you can see some of their helpers here (getFilesystemPath is the one to work around this issue from what I recall).

@thecrypticace
Copy link
Contributor

What a nightmare 😱

I guess I have some work ahead of me lol — going to do an audit of all filesystem path stuff in intellisense and the language server. Also going to set up Windows CI alongside all this too.

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.

2 participants