ndmitchell / hoogle

Haskell API search engine
http://hoogle.haskell.org/
Other
753 stars 137 forks source link

haddockfile:// urls #356

Closed pepeiborra closed 4 years ago

pepeiborra commented 4 years ago

When I run a hoogle server locally with the command line hoogle server --local --database=... --haddock=.. the links in the results page have the scheme haddockfile:// which my browser doesn't know what to do with. The problem does not appear when I don't use --haddock.

The code for showUrl looks suspect:

https://github.com/ndmitchell/hoogle/blob/master/src/Action/Server.hs#L248

ndmitchell commented 4 years ago

Hmm. Reading the code, maybe it should be:

showURL _ (Just _) x = "haddock" ++ dropPrefix "file://" x

But I'm not really sure. I don't remember enough about what --haddock and --local do to be sure how they'd combine.

pepeiborra commented 4 years ago

I don't know either, but I think it makes sense that --haddock implies --local. I patched it to something like:

showURL _ (Just haddock) x = "file/" ++ haddock ++ (takeFileName $ dropPrefix "file://" x)
ndmitchell commented 4 years ago

I think the correct fix is:

showURL _ (Just _) x = "haddock/" ++ dropPrefix "file:///" x

With the idea that you now hit the haddock handler on line 147. Doing so ensures you don't end up putting the haddock variable into the URL, which might require escaping, and potentially might conflict with one of the other handlers.

I just pushed that, on the assumption that what was there already is definitely wrong, so it can't hurt. Somewhat sad that I'm reduced to random prodding of my own code, and there's no test suite to know if I got it right or not. Hoogle has a weird state of mostly being a website (which is tested) and also being a program.

pepeiborra commented 4 years ago

I'll check it out on Monday

pepeiborra commented 4 years ago

It works, thanks!

ndmitchell commented 4 years ago

Thanks for checking. I probably won't bother making a release, but if it would be easier for you, let me know.

pepeiborra commented 4 years ago

I made a mistake while testing, sorry about that. It does not work as expected. The fix I am testing is:

showURL _ (Just _) x = "haddock/" ++ dropPrefix "file:///" x

The reason it doesn't work is that x below is an absolute path, so the URL ends up being haddock/long/file/system/path/Module.html whereas we probably want haddock/Package/Module.html

I'll continue working around this locally and update back here with any leads

FWIW I'm generating with hoogle generate --local=path --database=foo.hoo package-name

pepeiborra commented 4 years ago

I think I understand why I'm tripping on this. What I'm trying to do is to produce a relocatable set of Hoogle artifacts. In short, I want to produce Haddocks, produce a Hoogle index, package them together, and upload to a cloud server where the Hoogle server will run with a completely independent file system structure. It just dawned on me that this is probably not a supported use case

ndmitchell commented 4 years ago

It's not supported in the sense that I've never done it, but it's a totally valid use case to have. Where are the absolute paths coming from? From ghc-pkg? There's no reason not to use something like makeRelative to try and make something more relative. Patches welcome!

pepeiborra commented 4 years ago

No need, we will just containerise the Hoogle server instead.