Skip to content
This repository was archived by the owner on Aug 3, 2024. It is now read-only.

Add support for labeled module references (updated to ghc-8.10) #1181

Merged
merged 7 commits into from
Jan 15, 2021

Conversation

garetxe
Copy link
Contributor

@garetxe garetxe commented Apr 26, 2020

This is an update of #1078 to the latest version of haddock. (When I opened that one I forgot to make a separate branch, and now I cannot figure out how to fix that, sorry. Feel free to close that one.)

Now that #1179 has been merged, so we can create stable group identifiers, this removes partially the problem brought up in #1078 (comment).

I am copying below for convenience the original message for the pull request (slightly amended since the pull request now includes tests and changes to the manual). Many thanks in advance!

========

This pull request adds support for a markdown-style way of annotating module references. For instance

-- | [label]("Module.Name#anchor")

will create a link that points to the same place as the module
reference "Module.Name#anchor" but the text displayed on the link will
be label.

One motivation for wanting this is detailed in #802 (comment): in the haskell-gi bindings for instance, I will be using this syntax to refer to the documentation for certain sections in the documentation, for example for signals and properties of objects. Something like the following

-- | ... when the button is clicked, the [clicked]("GI.Gtk.Objects.Button#signal:clicked") signal is emitted.

that would render to "... when the button is clicked, the clicked signal is emitted."

I can see this being useful more generally, of course, for instance if one would like to refer to the set of substring methods in Data.Text, one could use [substring methods]("Data.Text#g:17").

I have been using already in the gi-gtk bindings, and seems to work well in practice.

Iñaki García Etxebarria added 4 commits August 6, 2019 06:48
Support a markdown-style way of annotating module references. For instance

-- | [label]("Module.Name#anchor")

will create a link that points to the same place as the module
reference "Module.Name#anchor" but the text displayed on the link will
be "label".
@Kleidukos
Copy link
Member

@garetxe Hi! This is a good PR, thank you very much. I don't think I have much to add, so you just have to resolve the conflicts in Parser.hs and it should be good to merge. :)

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

Thanks for the review @Kleidukos ! I have just fixed the conflicts.

@Kleidukos
Copy link
Member

@garetxe Do you think you could force-push a dummy commit to your branch? Unfortunately we are using GitHub Actions and they don't support manual re-trigger of workflows. The CI failure might have been just a blip.

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

I just pushed a dummy commit, but it is still failing. I can also get the error locally, it seems to be because haddock fails with the following message when working on Bug783:

Haddock coverage:
haddock: While creating Haddock interface for Bug873:

While creating Haddock interface for Bug873:

invalid binary data found in the interface file
CallStack (from HasCallStack):
  error, called at src/Haddock/InterfaceFile.hs:666:20 in haddock-api-2.24.0-inplace:Haddock.InterfaceFile

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

I looked a bit more into this, the reason for the error above is the change in the format in the interface file.

More precisely, when running the tests they are run with --bypass-interface-version-check. But this patch modifies the interface file format (because the DocModule constructor's argument is now a ModLink, instead of a plain String) , so when loading the interface files for the packages installed with ghc one of them (not sure which, this is tricky to debug due to lazyness) triggers the "invalid binary data found in the interface file" error above.

Normally the crash will not be there, thanks to the binaryInterfaceVersionCompatibility checks (which this patch does take into account), but the --bypass-interface-version-check flag turns the warning into an error.

So as far as I can tell there is nothing wrong with the patch, and things should self-correct once the libraries shipped with ghc have the right interface format. But unfortunately I don't see an easy way to correct for this in the CI at the moment. How do you usually deal with interface format changes?

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

For what's worth, Bug783 seems to be the only test that fails, so one option would be to disable this test until the .haddock files included with ghc are in the correct format.

@Kleidukos
Copy link
Member

Yep' that's going to be that. I will merge a patch to the test suite, @garetxe and then you can rebase on it. :)

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

Great, thanks :)

@Kleidukos Kleidukos mentioned this pull request Jan 15, 2021
@Kleidukos
Copy link
Member

@garetxe Alright, you can rebase now :)

@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

Great, this worked, thanks!

@Kleidukos
Copy link
Member

Kleidukos commented Jan 15, 2021

@garetxe Perfect! I'll squash your commits and merge it. Thank you very much for this PR ✨

@Kleidukos Kleidukos merged commit fe30f55 into haskell:ghc-8.10 Jan 15, 2021
@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

Wonderful, thanks a lot!

@garetxe garetxe deleted the labeledRefs branch January 15, 2021 16:31
@garetxe garetxe restored the labeledRefs branch January 15, 2021 16:38
@garetxe
Copy link
Contributor Author

garetxe commented Jan 15, 2021

Just wondering: I notice that there is a ghc-9.0 branch now, but this PR was for ghc-8.10. Can the PR be incorporated into ghc-9.0 too?

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

Hi @Kleidukos ! Would it be possible to merge this PR into the ghc-9.0 branch? I would be happy to adapt anything that needs adapting if it does not merge cleanly.

Thanks!

@Kleidukos
Copy link
Member

@garetxe I'm terribly busy right now but you're right, we need to port that into ghc-9.0. Do you think you could squash your commits into one and rebase your patch on top of ghc-9.0? This would be extremely helpful.

Thanks for pinging!

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

Done, I just opened #1315 with the refreshed version.

I cannot run the tests locally, since Cabal-3.2.1 does not build with ghc-9.0.1, but at least it compiles.

@Kleidukos
Copy link
Member

Yes, use ghcup to grab Cabal-3.4 and ghc-9.0

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

Thanks for the tip, @Kleidukos ! This works, I could run cabal test and everything works in the ghc-9.0.1 version.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants