Skip to content

Use Hackage for Documentation and Source links#4746

Open
Saizan wants to merge 4 commits intohaskell:masterfrom
Saizan:link-to-hackage
Open

Use Hackage for Documentation and Source links#4746
Saizan wants to merge 4 commits intohaskell:masterfrom
Saizan:link-to-hackage

Conversation

@Saizan
Copy link

@Saizan Saizan commented Oct 25, 2025

This is enabled by the new linkToHackage configuration option.

This is useful if you don't have local documentation or sources built, or other minor downsides of using local files. The vscode-haskell provides similar functionality but by trying to mangle the local file links, which is error prone because the path structure has changed between ghc versions.

If local documentation/source exists it gets used as a hint for which module to link to, which generally gives better results in case of re-exports. Otherwise we pick the module where the name is defined, and generate the link using its package name and version.

I am quite unsure if how I wired in the linkToHackage config option is appropriate. But I didn't want to do any refactor without some feedback first.

Initially I figured linkToHackage should be a plugin config for ghcide-hover-and-symbols, but the flag affects the GetDocMap rule which structurally appears to be more part of the core? The first way I wrote it I got import cycles so I backed off.

I am happy to have the flag where it is or move it if desired. And in any case I will split it into linkDocToHackage and linkSrcToHackage for flexibility.

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion and can we add a regression test case, somehow to make sure this doesn't accidentally break?

Otherwise, LGTM :)

@fendor fendor added the status: needs review This PR is ready for review label Nov 17, 2025
@Saizan Saizan force-pushed the link-to-hackage branch 2 times, most recently from 3f5aa28 to 4977a57 Compare February 14, 2026 17:43
@fendor fendor force-pushed the link-to-hackage branch from a5c9041 to 0ae5db3 Compare March 5, 2026 09:53
@fendor fendor requested a review from vidit-od March 5, 2026 09:55
@Saizan
Copy link
Author

Saizan commented Mar 5, 2026

@fendor I got a bit lost trying to add regression tests, would it look something like the ones in FindDefinitionAndHoverTests ?

@Saizan Saizan force-pushed the link-to-hackage branch from 0ae5db3 to 965a180 Compare March 5, 2026 13:03
Just foundFile -> T.replace "-" sep $ T.pack foundFile
pure $!
mconcat $
[ "http://hackage.haskell.org/package/"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use https here

Comment on lines +110 to +118
LinkToHackage ->
toHackageDocUriText env mod (takeFileName <$> doc)
LinkToLocal ->
toFileUriText doc
src_link = case linkSource of
LinkToHackage ->
toHackageSrcUriText env mod (takeFileName <$> src)
LinkToLocal ->
toFileUriText src
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

( personal nitpick , this is also fine)
can make these one lines
i.e

    LinkToHackage -> toHackageDocUriText env mod (takeFileName <$> doc)
    LinkToLocal -> toFileUriText doc
src_link = case linkSource of
    LinkToHackage ->  toHackageSrcUriText env mod (takeFileName <$> src)
    LinkToLocal -> toFileUriText src

@vidit-od
Copy link
Collaborator

vidit-od commented Mar 5, 2026

@Saizan Overall looks good,
regarding regression tests,

what you linked is a good pattern to follow, but from what i could see,
FindDefinitionAndHoverTests will use LinkToLocal by default. You'll need to write a separate test function that uses testithconfig with a custom Testconfig that sets to LinkToHackage maybe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants