-
Notifications
You must be signed in to change notification settings - Fork 482
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
Use a generic description for non-GitHub hosts #804
Conversation
This is a good idea. The current implementation will take away all GitHub links though, since the default repo template is defined in the Documenter.jl/src/Utilities/Utilities.jl Lines 386 to 389 in c58e7e0
One other minor thing -- having just the work "Edit" looks somehow too narrow. What do you think about having something like "Edit source"? |
54d95cc
to
2a5a751
Compare
"Edit source" seems fine, I've update the PR accordingly. However, I do not understand how my changes would take away Github links. Looking through the code, the url function is called twice, namely: Documenter.jl/src/Writers/HTMLWriter.jl Line 445 in b598cb0
and Documenter.jl/src/Writers/HTMLWriter.jl Line 725 in b598cb0
In both cases I cannot see how the repo argument would be different after my changes? |
I was a bit imprecise: GitHub links get the generic icon. They don't disappear. And the reason is that Documenter.jl/src/Writers/HTMLWriter.jl Line 427 in b598cb0
and so at that point The URL handling is a bit hectic and should probably be refactored at some point. But for now, to work around this issue, just make the GitHub part of elseif occursin("github", repoURL) || isempty(repoURL)
return RepoGithub
|
2a5a751
to
69b9e79
Compare
Oh, yes. Empty repo would have had that effect, yes. I've updated the PR as you suggested. |
LGTM, thanks! |
The
HTMLWriter
currently usesfor the source link, even if the repo points to a private host (such as a self-hosted GitLab instance). This PR changes the default to simply
in case no known host is detected.