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

Use a generic description for non-GitHub hosts #804

Merged
merged 2 commits into from
Aug 29, 2018

Conversation

tribut
Copy link
Contributor

@tribut tribut commented Aug 17, 2018

The HTMLWriter currently uses

GitHub-Icon Edit on GitHub

for 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

File-Icon Edit

in case no known host is detected.

@mortenpi mortenpi added this to the 0.20.0 milestone Aug 19, 2018
@mortenpi
Copy link
Member

This is a good idea. The current implementation will take away all GitHub links though, since the default repo template is defined in the url function. It should probably be moved away from there and made the default argument for doc.user.repo. I don't know if doing that will change any other default behaviour though.

function url(repo, file; commit=nothing)
file = realpath(abspath(file))
remote = getremote(dirname(file))
isempty(repo) && (repo = "https://github.com/$remote/blob/{commit}{path}")

One other minor thing -- having just the work "Edit" looks somehow too narrow. What do you think about having something like "Edit source"?

@tribut
Copy link
Contributor Author

tribut commented Aug 19, 2018

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

url = Utilities.url(ctx.doc.user.repo, pageurl, commit=ctx.doc.user.html_edit_branch)

and

url = Utilities.url(ctx.doc.internal.remote, ctx.doc.user.repo, result)

In both cases I cannot see how the repo argument would be different after my changes?

@mortenpi
Copy link
Member

I was a bit imprecise: GitHub links get the generic icon. They don't disappear.

And the reason is that url gets called after

host_type = Utilities.repo_host_from_url(ctx.doc.user.repo)

and so at that point ctx.doc.user.repo is an empty string and does not match the GitHub repo.

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 Utilities.repo_host_from_url into the following:

elseif occursin("github", repoURL) || isempty(repoURL)
    return RepoGithub

Utilities.url seems to use the same heuristic to determine that it should create GitHub URLs.

@tribut
Copy link
Contributor Author

tribut commented Aug 20, 2018

Oh, yes. Empty repo would have had that effect, yes. I've updated the PR as you suggested.

@mortenpi
Copy link
Member

LGTM, thanks!

@mortenpi mortenpi merged commit a388bd1 into JuliaDocs:master Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants