Teach http_get to read "file:///" urls
#1694
Conversation
We can already host local repositories -- i.e. "repositories" in the
PC settings understand absolute local paths as additional repositories.
But then inside such a repository, we cannot point a release url to a
local file but MUST use the http-protocol.
That seems unnecessary as the "file:///"-scheme denotes a local file
expressed as an url, and frankly, other fetchers would accept them just
as-is. E.g. a simple `urllib.request.urlopen(location)` would work with
both http and file urls.
Here, it doesn't work because of basic tests in `_grab`, e.g.:
```
parsed = urlparse(url)
if not parsed or not parsed.hostname:
raise DownloaderException('The URL "%s" is malformed' % url)
```
Don't scratch these tests but instead check for the "file:///" scheme at
the top, and do a straight, dead-simple `file.open` instead.
That's the minimal patch to support the usecase.
Replace support of absolute local paths on channel/repository level by global support for file:// protocol to handle local file resources, including packages and libraries. Enables adding local channels or repositories via Command Palette.
|
Wow, that's cool. You could delete quite a bit of lines with that small addition. |
|
Should we test this or do you make releases on a specific schedule? |
|
Ah, you made a breaking change here. You dropped support for absolute paths in "repositories" in the PC settings. I think that change should be additive. It is more consistent now but a breaking change. |
|
Which bit of lines do you meand? Absolute local paths are not officially promoted and not supported by Add Repository / Add Channel Commands. Supporting them, would also need to add another code path in http_get() for exaclty the same thing. Hence I expect it acceptable for users to adjust possible settings accordingly. |
Just generally, the original patch was +7 additive. But you could refactor and use it so that we have -61 in the overall diff now.
FichteFoll told me that I can use absolute paths years ago, but indeed, it is not in the settings comments. |
|
Can't you use standard absolute paths with the file protocol as well? That's what browsers do anyway. It just looks a bit weird because of the triple forward slashes at the beginning (and I honstly don't know about Windows here). Edit: In fact, that's what all the regular expressions match as well, so not sure I understand the "absolute local path" problem. |
|
To add support for the file protocol is the meat of this PR. In one undocumented part, the PC settings, it was possible to just have standard absolute paths, e.g. The con is really just that -- as the settings are user facing -- it is easier and feels "normal" to add a standard absolute path, as you can just copy and paste that from everywhere. |
|
Browsers prepend The actual protocol name is Linux: /www/index.html becomes file:///www/index.html Windows: C:\www\index.html is converted to file:///C:/www/index.html. The extra PC suppport(ed) adding absolute local paths into "channel" and "repository" settings to support locally hosted channel.json/repository.json by using a separate code path in ChannelProvider and JsonRepositoryProvider to load them from disk. Any download asset specified via "url" however is downloaded by This made local file hosting impossible. Of course we could also teach Official UI / Command Palette entries, didn't support adding local file paths anyway. That was just a somewhat "hidden" expert feature. Now adding local files is possible as well, when using a |
We can already host local repositories -- i.e. "repositories" in the PC settings understand absolute local paths as additional repositories. But then inside such a repository, we cannot point a release url to a local file but MUST use the http-protocol.
E.g. for testing purposes, that means to create a local file server just to host such release-zip files.
See https://github.com/SublimeLinter/SublimeLinter/blob/029ff3dcf441f5606ecb329d66d76ed7df601e96/scripts/repo.json#L14-L14 Over there, I just use "python -m http.server 8000" to host a folder that contains SublimeLinter releases.
That seems unnecessary as the "file:///"-scheme denotes a local file expressed as an url, and frankly, other fetchers would accept them just as-is. E.g. a simple
urllib.request.urlopen(location)would work with both http and file urls.It doesn't work here because of basic tests in
_grab, namely:Don't scratch that test but instead check for the "file:///" scheme at the top, and do a straight, dead-simple
file.openinstead.That's the minimal patch to support the usecase.