Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for labeled module references #1315

Closed
wants to merge 2 commits into from

Conversation

garetxe
Copy link
Contributor

@garetxe garetxe commented Feb 6, 2021

Support a markdown-style way of annotating module references. For instance

-- | label

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".

(This is a rebase of #1181 for ghc-9.0, as requested in #1181 (comment))

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".
@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

Just to clarify: this was already merged in the ghc-8.10 branch, but was not brought forward to ghc-9.0. I rebased so it is easier to merge in ghc-9.0, as @Kleidukos suggested.

@Kleidukos
Copy link
Member

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

Yes, this should be fixed by 0fd9131 : the DocModule constructor now takes a ModLink argument, and not a plain String. The error was due to OverloadedStrings trying to make sense out of this.

@Kleidukos
Copy link
Member

Alright now we're hitting

2021-02-06T20:55:25.2278472Z hoogle-test: hoogle-test/out/Bug873: removeDirectoryRecursive:getSymbolicLinkStatus: does not exist (No such file or directory)
2021-02-06T20:55:25.2280079Z Failed to run Haddock on test package 'Bug873'

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

There is still a failing test, but this is the same Bug873 as last time, due to the change on the interface version (see comments #1181 (comment) and #1296 (comment)). If I remove that test the tests pass locally for me.

Please let me know if you would like me to add a commit removing that test from my branch.

@Kleidukos
Copy link
Member

@garetxe This is also the symptom of something deeper. I have a call with Alex tomorrow and we'll decide of a solution. Otherwise, it's all good. I'll merge the PR when it's all green. :)

@garetxe
Copy link
Contributor Author

garetxe commented Feb 6, 2021

OK, great, thanks a lot :)

@Kleidukos
Copy link
Member

Kleidukos commented Feb 7, 2021

@garetxe Alright, I've had the call with @alexbiehl and we decided to make some changes to the PR to ensure backward compatibility.
Either you can upstream those changes in your own PR, or another PR can be opened so that you're not bothered anymore. We shall put your name in the "Authored by" field in the commit.
Thanks a lot! ✨

@garetxe
Copy link
Contributor Author

garetxe commented Feb 7, 2021

Great, thanks a lot! :)

@garetxe
Copy link
Contributor Author

garetxe commented Feb 7, 2021

Oh, and I don't care about the authorship part, as long as the patch goes in in some form I am happy!
So feel free to directly commit your changes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants