Add support for reading file URI's #3
Conversation
| , ("hoogle", Just $ Text.encodeUtf8 $ Text.pack term) | ||
| ] | ||
| res <- fetch req | ||
| res <- fetchHttp req |
There was a problem hiding this comment.
These requests need to be cached. Let's use fetch here.
There was a problem hiding this comment.
Oh yes, of course we need to cache that. But I have instead changed fetchHttp to do the caching, as suggested below :)
src/Docs/CLI/Evaluate.hs
Outdated
| src <- fetch req | ||
| cache <- State.gets sCache | ||
| let uri = getUrl x | ||
| src <- cached cache uri $ fetch uri |
There was a problem hiding this comment.
We want to cache HTTP requests because of network and server latency.
When we are fetching stuff from the local computer we can skip the cache altogether. In fact, what caching does is precisely reading files saved locally.
I think we can have fetch the way it was before, operating on Http.Request and caching the result, then we decide inside of fetchHTML whether we will call fetch or just read a local file.
| cached cache (show req) $ do | ||
| -- | Decide how to fetch depending on the URI | ||
| fetch :: Url -> M LB.ByteString | ||
| fetch uri | "https://" `isPrefixOf` uri |
There was a problem hiding this comment.
We could separate the concerns of this check a little bit by using a new datatype and a conversion function.
data Location
= Local FilePath
| Remote Url
location :: Url -> Maybe LocationThere was a problem hiding this comment.
I am not completely sure how you would do this, so maybe you could just do this, if you think it would be good.
|
I'm glad you are liking the tool! This piece is part of a broader problem, which is that of supporting private Hoogle instances. Both running locally and on custom servers. This solves a piece of the problem, but there are a few edge cases.
However, I tried this locally and its super cool and makes using a local Hoogle work with most of the features, so thank you very much for dedicating the time. Let's get this merged! |
Hope the changes are fine for now. I will happily look into some of these other edge cases at some other time :) |
2 participants
First of all, this is a REALLY nice program! I was previously quite annoyed at how bad the cli experience was for hoogle and hackage, and just resorted to using the browser. But this is really good! I haven't used it much, but it was already easier to use, than opening a web page, while I made this change :)
My change is adding support for handling file uri's, which is necessary when using a local Hoogle server.
Some url's are changed to file uri's on a local Hoogle server. This made haskell-docs-cli crash when trying to open one of those.
This change makes it so that haskell-docs-cli instead reads the file contents, and continues.
Considerations:
file://orhttp[s]://. If there can be any links that either start with something else (maybeftp://hackage.haskell.org/.../...), or does not start with any uri-type (eg.hackage.haskell.org/.../...), it will fail.fetchHttp, which would make sure it does not crash on something now, which it did not crash on before.