Fix absolutePathToLang() issue#3298
Conversation
🦋 Changeset detectedLatest commit: 5df5ec1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
delucis
left a comment
There was a problem hiding this comment.
Thanks @HiDeoo, this looks good.
One question: absolutePathToLang() is a public API. Would we expect this change to impact Starlight plugin authors at all? I notice we have the test file changing the direct use of URL#pathname, but I guess that was a slightly unusual usage and not something we’d expect real-world users to be doing?
|
Following your comment, I pushed 2 changes that I thought would be useful:
To answer your question, the only way this change would impact a plugin API authors would be if they were somehow relying on the previous behavior that would result in an invalid language in some cases, e.g. with spaces in the path (I cannot imagine why or how). Otherwise, it's a fix that would benefit all plugins using that API. |
* main: Small updates to `tailwind` template (withastro#3303) docs: showcase `astro-d2` and "Starlight Plugins by Example" (withastro#3302) [ci] release (withastro#3301) Fix `absolutePathToLang()` issue (withastro#3298)
Description
This PR fixes an issue with the
absolutePathToLang()helper function not handling absolute paths with spaces correctly.absolutePathToLang()is meant to be called with thepathproperty of a virtual file (altho not mandatory), which is always a POSIX-style path (using/as a separator) no matter the operating system.getCollectionPath()that relies on an URLpathnamewhich will also be POSIX-style but also encoded, e.g. spaces are replaced with%20.pathproperty does not encodes special characters like spaces.This can lead to issues when the path contains spaces which can be solved in different approaches:
fileURLToPath()ingetCollectionPath()to convert the URL to a file path, altho depending on the OS, different separators will be used and would require an additional step to convert it again to a POSIX-style path.pathToFileURL()on the input path ofabsolutePathToLang()to convert it to a URL-encoded path also using POSIX-style separators and using itspathname.I went with the second approach which replaces an existing step instead of adding a new one.
On top of the added test, I also manually tested on Windows using a path with spaces: