fix haddock documentation links (#2542)

[lawbel]

This doesn't completely resolve the linked issue (#2542), but it does seem to solve the documentation link problems. I've looked through the haddock codebase and tried to mirror the way that it creates links/anchors, and it looks to be working. In the process of doing this I realized that there is another issue with the current links - they don't work for function names like foldl' that have apostrophes in them. This commit also fixes that problem.

Getting the source links to behave is proving more difficult. I can't figure out how the module names are supposed to be formatted. I had thought, when writing the issue, that they should always be separated with dashes (like GHC-Base.html). But it seems that sometimes dots are needed (like GHC.Base.html). I'm at a loss trying to understand it. Any help would be welcome here.

[jneira]

Many thanks for the pr and the fixes

Getting the source links to behave is proving more difficult. I can't figure out how the module names are supposed to be formatted. I had thought, when writing the issue, that they should always be separated with dashes (like GHC-Base.html). But it seems that sometimes dots are needed (like GHC.Base.html). I'm at a loss trying to understand it. Any help would be welcome here.

There is an issue in haddock about: haskell/haddock#1447, opened from the hls issue #2508

[jneira]

It would be great to add a regression test to check haddock doc links are not going to be broken. The test suite for hover is here:

haskell-language-server/ghcide/test/exe/Main.hs

Line 3867 in 3de244e

findDefinitionAndHoverTests :: TestTree

feel free to ask any question about

[lawbel]

Thanks for the pointer, I will do that and try to write some suitable tests. Should I do that in a separate PR?

From reading those links about the source links, it seems that the issue I was experiencing with them is not the fault of HLS, but rather some combination of haddock changes and the vscode HLS extension?

[jneira]

Thanks for the pointer, I will do that and try to write some suitable tests. Should I do that in a separate PR?

AS you consider, both options are fine

From reading those links about the source links, it seems that the issue I was experiencing with them is not the fault of HLS, but rather some combination of haddock changes and the vscode HLS extension?

yeah, a combination of, i think haddock is inconsistent in its url generation, hls follow one schema and the vscode tried to fix it standalone 🤦

[lawbel]

Okay, I have added a couple tests to check that names in the documentation links are getting encoded properly. If that looks good to you, then I think we could close the original issue, as the business with source links already has open issues in the appropriate places?

[michaelpj]

Looks like one of the ghcide tests fails now?

[lawbel]

Yeah, at first glance it looks like some (older?) versions of GHC and/or HLS don't generate the Documentation links, which is making the tests fail. I'll see if I can add some conditionals to just run the tests for some versions.

[lawbel]

Sorry for the delay, it was a bit of a slog to look through the logs. It seems like many of the builds got timed out, and others skipped the ghcide tests? Looking at the builds that got far enough to run the two tests I added (which only happened for ubuntu-latest), the results are as follows:

ghc	def operator	def function	hover operator	hover function
9.0.1	ok	ok	fail	fail
8.10.7	ok	ok	ok	ok
8.8.4	ok	ok	ok	ok
8.6.5	ok	ok	fail	ok

I admit to being completely lost as to why the def cases are all passing, but a selection of the hover cases are failing.

Looking at the results, it would seem sensible to keep the def tests as-is, and just skip over all the hover tests. I've pushed a commit that does exactly that, so we'll see whether the tests pass this time or not.

[michaelpj]

Possibly dumb question: could we actually depend on haddock and pull in the functions that do this stuff?

[lawbel]

Perhaps that would be a better approach. I'm not very familiar with the internals of haddock, or HLS for that matter, so I just tried to hack something together that works. I suppose I had been implicitly assuming that the way haddock does this is relatively stable, i.e. it's not something that's likely to change often if at all.

The function that I added called makeDocAnchor is a minor adaptation of Haddock.Utils.makeAnchorId from haddock-api, link here for the source code on github. I think it is the exact function that creates the documentation links in the first place. (Though I might be mistaken, there's a lot of code in haddock, I may have missed something.) It is not exported as part of the public API of haddock-api though, as far I as can tell... I'm not sure whether equivalent functionality is exposed through some of the other parts that are exported.

[michaelpj]

I'm not so much worried about Haddock changing how it does it, as there just being more awkward corners we hadn't thought about. If they don't expose it, though, nothing we can do. Maybe open an issue in haddock recording that it would be useful to expose functions for creating links?

[michaelpj]

Also, the tests didn't time out this time. Are you planning to investigate what's up with the disabled tests or just leave it for now? At least we should include a comment as to why they're disabled if we do that.

[pepeiborra]

Sad that this PR was closed without merging. Reopening in case someone can resubmit it, or at least salvage the tests