From 272fd21bd7c52897991189bd5e49aaacdd3b9301 Mon Sep 17 00:00:00 2001 From: Morten Piibeleht Date: Mon, 18 Jul 2022 14:44:19 +1200 Subject: [PATCH 1/3] Reduce the PR down to minimal changes --- CHANGELOG.md | 9 + docs/src/lib/internals/utilities.md | 5 + docs/src/lib/public.md | 17 ++ src/CrossReferences.jl | 10 +- src/Documenter.jl | 29 +- src/Documents.jl | 24 +- src/Utilities/Remotes.jl | 254 ++++++++++++++++++ src/Utilities/Utilities.jl | 154 ++++------- src/Writers/HTMLWriter.jl | 6 +- src/Writers/LaTeXWriter.jl | 2 +- test/examples/make.jl | 24 +- .../src/{lib/editurl.md => editurl/bad.md} | 2 + test/examples/src/editurl/good.md | 7 + test/examples/src/editurl/ugly.md | 8 + test/remotes.jl | 72 +++++ test/runtests.jl | 1 + test/utilities.jl | 39 ++- 17 files changed, 513 insertions(+), 150 deletions(-) create mode 100644 src/Utilities/Remotes.jl rename test/examples/src/{lib/editurl.md => editurl/bad.md} (55%) create mode 100644 test/examples/src/editurl/good.md create mode 100644 test/examples/src/editurl/ugly.md create mode 100644 test/remotes.jl diff --git a/CHANGELOG.md b/CHANGELOG.md index 884a809791..370a481c01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ **For upgrading:** To keep using the Markdown backend, refer to the [DocumenterMarkdown package][documentermarkdown]. That package might not immediately support the latest Documenter version, however. * ![Enhancement][badge-enhancement] The `ansicolor` keyword to `HTML()` now defaults to true, meaning that executed outputs from `@example`- and `@repl`-blocks are now by default colored (if they emit colored output). ([#1828][github-1828]) +* ![Enhancement][badge-enhancement] A more general API is now available to configure the remote repository URLs via the `repo` argument of `makedocs` by passing objects that are subtypes of `Remotes.Remote` and implement its interface (e.g. `Remotes.GitHub`). ([#1808][github-1808]) +* ![Enhancement][badge-enhancement] Broken issue references (i.e. links like `[#1234](@ref)`, but when Documenter is unable to determine the remote GitHub repository) now generate `:cross_references` errors that can be caught via the `strict` keyword. ([#1808][github-1808]) + + This is **potentially breaking** as it can cause previously working builds to fail if they are being run in strict mode. However, such builds were already leaving broken links in the generated documentation. + + **For upgrading:** the easiest way to fix the build is to remove the offending `@ref` links. Alternatively, the `repo` argument to `makedocs` can be set to the appropriate `Remotes.Remote` object that implements the `Remotes.issueurl` function, which would make sure that correct URLs are generated. + +* ![Bugfix][badge-bugfix] Documenter now generates the correct source URLs for docstrings from other packages when the `repo` argument to `makedocs` is set (note: the source links to such docstrings only work if the external package is cloned from GitHub and added as a dev-dependency). However, this change **breaks** the case where the `repo` argument is used to override the main package/repository URL, assuming the repository is cloned from GitHub. ([#1808][github-1808]) ## Version `v0.27.21` @@ -1067,6 +1075,7 @@ [github-1805]: https://github.com/JuliaDocs/Documenter.jl/pull/1805 [github-1806]: https://github.com/JuliaDocs/Documenter.jl/pull/1806 [github-1807]: https://github.com/JuliaDocs/Documenter.jl/pull/1807 +[github-1808]: https://github.com/JuliaDocs/Documenter.jl/pull/1808 [github-1810]: https://github.com/JuliaDocs/Documenter.jl/issues/1810 [github-1811]: https://github.com/JuliaDocs/Documenter.jl/pull/1811 [github-1814]: https://github.com/JuliaDocs/Documenter.jl/issues/1814 diff --git a/docs/src/lib/internals/utilities.md b/docs/src/lib/internals/utilities.md index cc940381fe..aeecbf69a1 100644 --- a/docs/src/lib/internals/utilities.md +++ b/docs/src/lib/internals/utilities.md @@ -3,3 +3,8 @@ ```@autodocs Modules = [Documenter.Utilities] ``` + +```@docs +Remotes.URL +Remotes.repofile +``` diff --git a/docs/src/lib/public.md b/docs/src/lib/public.md index bcf0b90c01..d87981142a 100644 --- a/docs/src/lib/public.md +++ b/docs/src/lib/public.md @@ -31,6 +31,23 @@ DocMeta.getdocmeta DocMeta.setdocmeta! ``` +### Remotes + +```@docs +Documenter.Remotes +Documenter.Remotes.GitHub +``` + +The following types and functions and relevant when creating custom +[`Remote`](@ref Documenter.Remotes.Remote) types: + +```@docs +Documenter.Remotes.Remote +Documenter.Remotes.repourl +Documenter.Remotes.fileurl +Documenter.Remotes.issueurl +``` + ## DocumenterTools ```@docs diff --git a/src/CrossReferences.jl b/src/CrossReferences.jl index 3c2ad4efe6..c3649b1e3d 100644 --- a/src/CrossReferences.jl +++ b/src/CrossReferences.jl @@ -13,6 +13,7 @@ import ..Documenter: Utilities.@docerror using DocStringExtensions +using .Utilities: Remotes import Markdown """ @@ -213,8 +214,13 @@ getsig(λ::Union{Function, DataType}, typesig) = Base.tuple_type_tail(which(λ, # ----------------------------- function issue_xref(link::Markdown.Link, num, meta, page, doc) - link.url = isempty(doc.internal.remote) ? link.url : - "https://github.com/$(doc.internal.remote)/issues/$num" + # Update issue links starting with a hash, but only if our Remote supports it + issue_url = Remotes.issueurl(doc.user.remote, num) + if isnothing(issue_url) + @docerror(doc, :cross_references, "unable to generate issue reference for '[`#$num`](@ref)' in $(Utilities.locrepr(page.source)).") + else + link.url = issue_url + end end end diff --git a/src/Documenter.jl b/src/Documenter.jl index f2ad0fa7a2..c123ed29ef 100644 --- a/src/Documenter.jl +++ b/src/Documenter.jl @@ -61,14 +61,14 @@ include("CrossReferences.jl") include("DocChecks.jl") include("Writers/Writers.jl") -import .Utilities: Selectors, git +import .Utilities: Selectors, Remotes, git import .Writers.HTMLWriter: HTML, asset import .Writers.HTMLWriter.RD: KaTeX, MathJax, MathJax2, MathJax3 import .Writers.LaTeXWriter: LaTeX # User Interface. # --------------- -export makedocs, deploydocs, hide, doctest, DocMeta, asset, +export makedocs, deploydocs, hide, doctest, DocMeta, asset, Remotes, KaTeX, MathJax, MathJax2, MathJax3 """ @@ -158,23 +158,16 @@ makedocs( and so any docstring from the module `Documenter` that is not spliced into the generated documentation in `build` will raise a warning. -**`repo`** specifies a template for the "link to source" feature. If you are -using GitHub, this is automatically generated from the remote. If you are using -a different host, you can use this option to tell Documenter how URLs should be -generated. The following placeholders will be replaced with the respective -value of the generated link: +**`repo`** specifies the browsable remote repository (e.g. on github.com). This is used for +generating various remote links, such as the "source" links on docstrings. It can either +be passed an object that implements the [`Remotes.Remote`](@ref) interface (e.g. +[`Remotes.GitHub`](@ref)) or a template string. If a string is passed, it is interpreted +according to the rules described in [`Remotes.URL`](@ref). - - `{commit}` Git branch or tag name, or commit hash - - `{path}` Path to the file in the repository - - `{line}` Line (or range of lines) in the source file - -BitBucket, GitLab and Azure DevOps are supported along with GitHub, for example: - -```julia -makedocs(repo = \"https://gitlab.com/user/project/blob/{commit}{path}#{line}\") # GitLab -makedocs(repo = \"https://dev.azure.com/org/project/_git/repo?path={path}&version={commit}{line}&lineStartColumn=1&lineEndColumn=1\") # Azure DevOps -makedocs(repo = \"https://bitbucket.org/user/project/src/{commit}/{path}#lines-{line}\") # BitBucket -``` +By default, the repository is assumed to be hosted on GitHub, and the remote URL is +determined by first checking the URL of the `origin` Git remote, and then falling back to +checking the `TRAVIS_REPO_SLUG` (for Travis CI) and `GITHUB_REPOSITORY` (for GitHub Actions) +environment variables. If this automatic procedure fails, a warning is printed. **`highlightsig`** enables or disables automatic syntax highlighting of leading, unlabeled code blocks in docstrings (as Julia code). For example, if your docstring begins with an diff --git a/src/Documents.jl b/src/Documents.jl index fb8b24ba09..72e59ba69e 100644 --- a/src/Documents.jl +++ b/src/Documents.jl @@ -16,7 +16,7 @@ import ..Documenter: Plugin, Writer -import ..Documenter.Utilities.Markdown2 +using ..Documenter.Utilities: Remotes, Markdown2 using DocStringExtensions import Markdown using Unicode @@ -244,7 +244,7 @@ struct User strict::Union{Bool,Symbol,Vector{Symbol}} # Throw an exception when any warnings are encountered. pages :: Vector{Any} # Ordering of document pages specified by the user. expandfirst::Vector{String} # List of pages that get "expanded" before others - repo :: String # Template for URL to source code repo + remote :: Union{Remotes.Remote,Nothing} # Remote Git repository information sitename:: String authors :: String version :: String # version string used in the version selector by default @@ -257,7 +257,6 @@ Private state used to control the generation process. """ struct Internal assets :: String # Path where asset files will be copied to. - remote :: String # The remote repo on github where this package is hosted. navtree :: Vector{NavNode} # A vector of top-level navigation items. navlist :: Vector{NavNode} # An ordered list of `NavNode`s that point to actual pages headers :: Anchors.AnchorMap # See `modules/Anchors.jl`. Tracks `Markdown.Header` objects. @@ -300,7 +299,7 @@ function Document(plugins = nothing; modules :: Utilities.ModVec = Module[], pages :: Vector = Any[], expandfirst :: Vector = String[], - repo :: AbstractString = "", + repo :: Union{Remotes.Remote, AbstractString} = "", sitename :: AbstractString = "", authors :: AbstractString = "", version :: AbstractString = "", @@ -320,6 +319,20 @@ function Document(plugins = nothing; version = "git:$(Utilities.get_commit_short(root))" end + remote = if isa(repo, AbstractString) && isempty(repo) + # If the user does not provide the `repo` argument, we'll try to automatically + # detect the remote repository by looking at the Git repository remote. This only + # works if the repository is hosted on GitHub. If that fails, it falls back to + # TRAVIS_REPO_SLUG. + Utilities.getremote(root) + elseif repo isa AbstractString + # Use the old template string parsing logic if a string was passed. + Remotes.URL(repo) + else + # Otherwise it should be some Remote object + repo + end + user = User( root, source, @@ -336,7 +349,7 @@ function Document(plugins = nothing; strict, pages, expandfirst, - repo, + remote, sitename, authors, version, @@ -345,7 +358,6 @@ function Document(plugins = nothing; ) internal = Internal( Utilities.assetsdir(), - Utilities.getremote(root), [], [], Anchors.AnchorMap(), diff --git a/src/Utilities/Remotes.jl b/src/Utilities/Remotes.jl new file mode 100644 index 0000000000..5aa10a8939 --- /dev/null +++ b/src/Utilities/Remotes.jl @@ -0,0 +1,254 @@ +""" +Types and functions for handling repository remotes. +""" +module Remotes + +""" + abstract type Remote + +Abstract supertype for implementing additional remote repositories that Documenter can use +when generating links to files hosted on Git hosting service (such as GitHub, GitLab etc). +For custom or less common Git hosting services, the user can create their own `Remote` +subtype and pass that as the `repo` argument to [`makedocs`](@ref Main.Documenter.makedocs). + +When implementing a new type `T <: Remote`, the following functions must be extended for +that type: + +* [`Remotes.repourl`](@ref) +* [`Remotes.fileurl`](@ref) + +Additionally, it may also extend the following functions: + +* [`Remotes.issueurl`](@ref) +""" +abstract type Remote end + +""" + Remotes.repourl(remote::T) -> String + +An internal Documenter function that **must** be extended when implementing a user-defined +[`Remote`](@ref). It should return a string pointing to the landing page of the remote +repository. E.g. for [`GitHub`](@ref) it returns `"https://github.com/USER/REPO/"`. +""" +function repourl end + +""" + Remotes.fileurl(remote::T, ref, filename, linerange) -> String + +An internal Documenter function that **must** be extended when implementing a user-defined +[`Remote`](@ref). Should return the full remote URL to the source file `filename`, +optionally including the line numbers. + +* **`ref`** is string containing the Git reference, such as a commit SHA, branch name or a tag + name. + +* **`filename`** is a string containing the full path of the file in the repository without any + leading `/` characters. + +* **`linerange`** either specifies a range of integers or is `nothing`. In the former case it + either specifies a line number (if `first(linerange) == last(linerange)`) or a range of + lines (`first(linerange) < last(linerange)`). The line information should be accessed only + with the `first` and `last` functions (no other interface guarantees are made). + + If `linerange` is `nothing`, the line numbers should be omitted and the returned URL + should refer to the full file. + + It is also acceptable for an implementation to completely ignore the value of the + `linerange` argument, e.g. when the remote repository does not support direct links to + particular line numbers. + +E.g. for [`GitHub`](@ref), depending on the input arguments, it would return the following +strings: + +| `ref` | `filename` | `linerange` | returned string | +| ----------- | -------------- | ----------- | :-------------------------------------------------------------- | +| `"master"` | `"foo/bar.jl"` | `nothing` | `"https://github.com/USER/REPO/blob/master/foo/bar.jl"` | +| `"v1.2.3"` | `"foo/bar.jl"` | `12:12` | `"https://github.com/USER/REPO/blob/v1.2.3/foo/bar.jl#L12"` | +| `"xyz/foo"` | `"README.md"` | `10:15` | `"https://github.com/USER/REPO/blob/xyz/foo/README.md#L10-L15"` | +""" +function fileurl end + +""" + Remotes.issueurl(remote::T, issuenumber) + +An internal Documenter function that can be extended when implementing a user-defined +[`Remote`](@ref). It should return a string with the full URL to an issue referenced by +`issuenumber`, or `nothing` if it is not possible to determine such a URL. + +* **`issuenumber`** is a string containing the issue number. + +It is not mandatory to define this method for a custom [`Remote`](@ref). In this case it +just falls back to always returning `nothing`. + +E.g. for [`GitHub`](@ref) when `issuenumber = "123"`, it would return +`"https://github.com/USER/REPO/issues/123"`. +""" +function issueurl end +# Generic fallback always returning nothing +issueurl(::Remote, ::Any) = nothing + +""" + repofile(remote::Remote, ref, filename, linerange=nothing) + +Documenter's internal version of `fileurl`, which sanitizes the inputs before they are passed +to the potentially user-defined `fileurl` implementations. +""" +function repofile(remote::Remote, ref, filename, linerange=nothing) + # sanitize the file name + filename = replace(filename, '\\' => '/') # remove backslashes on Windows + filename = lstrip(filename, '/') # remove leading spaces + # Only pass UnitRanges to user code (even though we require the users to support any + # collection supporting first/last). + fileurl(remote, ref, filename, isnothing(linerange) ? nothing : Int(first(linerange)):Int(last(linerange))) +end + +""" + GitHub(user :: AbstractString, repo :: AbstractString) + GitHub(remote :: AbstractString) + +Represents a remote Git repository hosted on GitHub. The repository is identified by the +names of the user (or organization) and the repository: `GitHub(user, repository)`. E.g.: + +```julia +makedocs( + repo = GitHub("JuliaDocs", "Documenter.jl") +) +``` + +The single-argument constructor assumes that the user and repository parts are separated by +a slash (e.g. `JuliaDocs/Documenter.jl`). +""" +struct GitHub <: Remote + user :: String + repo :: String +end +function GitHub(remote::AbstractString) + user, repo = split(remote, '/') + GitHub(user, repo) +end +repourl(remote::GitHub) = "https://github.com/$(remote.user)/$(remote.repo)" +function fileurl(remote::GitHub, ref::AbstractString, filename::AbstractString, linerange) + url = "$(repourl(remote))/blob/$(ref)/$(filename)" + isnothing(linerange) && return url + lstart, lend = first(linerange), last(linerange) + return (lstart == lend) ? "$(url)#L$(lstart)" : "$(url)#L$(lstart)-L$(lend)" +end +issueurl(remote::GitHub, issuenumber) = "$(repourl(remote))/issues/$issuenumber" + +############################################################################ +# Handling of URL string templates (deprecated, for backwards compatibility) +# +""" + URL(urltemplate, repourl=nothing) + +A [`Remote`](@ref) type used internally in Documenter when the user passes a URL template +string as the `repo` argument. Will return `nothing` from `repourl` if the optional +`repourl` argument is not passed. + +Can contain the following template sections that Documenter will replace: + +* `{commit}`: replaced by the commit SHA, branch or tag name +* `{path}`: replaced by the path of the file, relative to the repository root +* `{line}`: replaced by the line (or line range) reference + +For example, the template URLs might look something like: + +* GitLab: + ``` + https://gitlab.com/user/project/blob/{commit}{path}#{line} + ``` +* Azure DevOps: + ``` + https://dev.azure.com/org/project/_git/repo?path={path}&version={commit}{line}&lineStartColumn=1&lineEndColumn=1 + ``` +* BitBucket: + ``` + https://bitbucket.org/user/project/src/{commit}/{path}#lines-{line} + ``` + +However, an explicit [`Remote`](@ref) object is preferred over using a template string when +configuring Documenter. +""" +struct URL <: Remote + urltemplate :: String + repourl :: Union{String, Nothing} + URL(urltemplate, repourl=nothing) = new(urltemplate, repourl) +end +repourl(remote::URL) = remote.repourl +function fileurl(remote::URL, ref, filename, linerange) + hosttype = repo_host_from_url(remote.urltemplate) + lines = (linerange === nothing) ? "" : format_line(linerange, LineRangeFormatting(hosttype)) + ref = format_commit(ref, hosttype) + # lines = if linerange !== nothing + # end + s = replace(remote.urltemplate, "{commit}" => ref) + # template strings assume that {path} has a leading / whereas filename does not + s = replace(s, "{path}" => "/$(filename)") + replace(s, "{line}" => lines) +end + +# Repository hosts +# RepoUnknown denotes that the repository type could not be determined automatically +@enum RepoHost RepoGithub RepoBitbucket RepoGitlab RepoAzureDevOps RepoUnknown + +# Repository host from repository url +# i.e. "https://github.com/something" => RepoGithub +# "https://bitbucket.org/xxx" => RepoBitbucket +# If no match, returns RepoUnknown +function repo_host_from_url(repoURL::String) + if occursin("bitbucket", repoURL) + return RepoBitbucket + elseif occursin("github", repoURL) || isempty(repoURL) + return RepoGithub + elseif occursin("gitlab", repoURL) + return RepoGitlab + elseif occursin("azure", repoURL) + return RepoAzureDevOps + else + return RepoUnknown + end +end +repo_host_from_url(::GitHub) = RepoGithub +repo_host_from_url(remote::Remote) = repo_host_from_url(Remotes.repourl(remote)) +repo_host_from_url(::Nothing) = RepoUnknown + +function format_commit(commit::AbstractString, host::RepoHost) + if host === RepoAzureDevOps + # if commit hash then preceeded by GC, if branch name then preceeded by GB + if match(r"[0-9a-fA-F]{40}", commit) !== nothing + commit = "GC$commit" + else + commit = "GB$commit" + end + else + return commit + end +end + +struct LineRangeFormatting + prefix::String + separator::String + + function LineRangeFormatting(host::RepoHost) + if host === RepoAzureDevOps + new("&line=", "&lineEnd=") + elseif host == RepoBitbucket + new("", ":") + elseif host == RepoGitlab + new("L", "-") + else + # default is github-style + new("L", "-L") + end + end +end + +function format_line(range::AbstractRange, format::LineRangeFormatting) + if length(range) <= 1 + string(format.prefix, first(range)) + else + string(format.prefix, first(range), format.separator, last(range)) + end +end + +end diff --git a/src/Utilities/Utilities.jl b/src/Utilities/Utilities.jl index df2429d6ff..025f088176 100644 --- a/src/Utilities/Utilities.jl +++ b/src/Utilities/Utilities.jl @@ -10,6 +10,13 @@ import Markdown, LibGit2 import Base64: stringmime import ..ERROR_NAMES +include("Remotes.jl") +using .Remotes: Remote, repourl, repofile +# These imports are here to support code that still assumes that these names are defined +# in the Utilities module. +using .Remotes: RepoHost, RepoGithub, RepoBitbucket, RepoGitlab, RepoAzureDevOps, + RepoUnknown, format_commit, format_line, repo_host_from_url, LineRangeFormatting + """ @docerror(doc, tag, msg, exs...) @@ -406,10 +413,6 @@ function repo_root(file; dbdir=".git") return nothing end -# Repository hosts -# RepoUnknown denotes that the repository type could not be determined automatically -@enum RepoHost RepoGithub RepoBitbucket RepoGitlab RepoAzureDevOps RepoUnknown - """ $(SIGNATURES) @@ -429,88 +432,68 @@ function repo_commit(file) end end -function format_commit(commit::AbstractString, host::RepoHost) - if host === RepoAzureDevOps - # if commit hash then preceeded by GC, if branch name then preceeded by GB - if match(r"[0-9a-fA-F]{40}", commit) !== nothing - commit = "GC$commit" - else - commit = "GB$commit" - end - else - return commit - end -end - -function url(repo, file; commit=nothing) +function edit_url(repo, file; commit=nothing) file = abspath(file) if !isfile(file) @warn "couldn't find file \"$file\" when generating URL" return nothing end file = realpath(file) - remote = getremote(dirname(file)) - isempty(repo) && (repo = "https://github.com/$remote/blob/{commit}{path}") + isnothing(repo) && (repo = getremote(dirname(file))) + isnothing(commit) && (commit = repo_commit(file)) path = relpath_from_repo_root(file) - if path === nothing - nothing - else - hosttype = repo_host_from_url(repo) - repo = replace(repo, "{commit}" => format_commit(commit === nothing ? repo_commit(file) : commit, hosttype)) - # Note: replacing any backslashes in path (e.g. if building the docs on Windows) - repo = replace(repo, "{path}" => string("/", replace(path, '\\' => '/'))) - repo = replace(repo, "{line}" => "") - repo - end + isnothing(path) || isnothing(repo) ? nothing : repofile(repo, commit, path) end -url(remote, repo, doc) = url(remote, repo, doc.data[:module], doc.data[:path], linerange(doc)) +source_url(repo, doc) = source_url(repo, doc.data[:module], doc.data[:path], linerange(doc)) -function url(remote, repo, mod, file, linerange) +function source_url(repo, mod, file, linerange) file === nothing && return nothing # needed on julia v0.6, see #689 remote = getremote(dirname(file)) - isabspath(file) && isempty(remote) && isempty(repo) && return nothing + isabspath(file) && isnothing(remote) && isnothing(repo) && return nothing # make sure we get the true path, as otherwise we will get different paths when we compute `root` below if isfile(file) file = realpath(abspath(file)) end - hosttype = repo_host_from_url(repo) - - # Format the line range. - line = format_line(linerange, LineRangeFormatting(hosttype)) # Macro-generated methods such as those produced by `@deprecate` list their file as # `deprecated.jl` since that is where the macro is defined. Use that to help # determine the correct URL. if inbase(mod) || !isabspath(file) - file = replace(file, '\\' => '/') - base = "https://github.com/JuliaLang/julia/blob" - dest = "base/$file#$line" - if isempty(Base.GIT_VERSION_INFO.commit) - "$base/v$VERSION/$dest" + ref = if isempty(Base.GIT_VERSION_INFO.commit) + "v$VERSION" else - commit = Base.GIT_VERSION_INFO.commit - "$base/$commit/$dest" + Base.GIT_VERSION_INFO.commit end + repofile(julia_remote, ref, "base/$file", linerange) else path = relpath_from_repo_root(file) - if isempty(repo) - repo = "https://github.com/$remote/blob/{commit}{path}#{line}" - end - if path === nothing - nothing - else - repo = replace(repo, "{commit}" => format_commit(repo_commit(file), hosttype)) - # Note: replacing any backslashes in path (e.g. if building the docs on Windows) - repo = replace(repo, "{path}" => string("/", replace(path, '\\' => '/'))) - repo = replace(repo, "{line}" => line) - repo - end + # If we managed to determine a remote for the current file with getremote, + # then we use that information instead of the user-provided repo (doc.user.remote) + # argument to generate source links. This means that in the case where some + # docstrings come from another repository (like the DocumenterTools doc dependency + # for Documenter), then we generate the correct links, since we actually user the + # remote determined from the Git repository. + # + # In principle, this prevents the user from overriding the remote for the main + # repository if the repo is cloned from GitHub (e.g. when you clone from a fork, but + # want the source links to point to the upstream repository; however, this feels + # like a very unlikely edge case). If the repository is cloned from somewhere else + # than GitHub, then everything is fine --- getremote will fail and remote is + # `nothing`, in which case we fall back to using `repo`. + isnothing(remote) && (remote = repo) + commit = repo_commit(file) + isnothing(path) || isnothing(remote) ? nothing : repofile(remote, commit, path, linerange) end end -const GIT_REMOTE_CACHE = Dict{String,String}() +""" +A [`Remote`](@ref) corresponding to the main Julia language repository. +""" +const julia_remote = Remotes.GitHub("JuliaLang", "julia") + +const GIT_REMOTE_CACHE = Dict{String,Union{Remotes.GitHub,Nothing}}() function getremote(dir::AbstractString) return get!(GIT_REMOTE_CACHE, dir) do @@ -520,7 +503,18 @@ function getremote(dir::AbstractString) "" end m = match(LibGit2.GITHUB_REGEX, remote) - return m === nothing ? get(ENV, "TRAVIS_REPO_SLUG", "") : String(m[1]) + if isnothing(m) + # TODO: move this fallback out of getremote + remote = get(ENV, "TRAVIS_REPO_SLUG", nothing) + try + # Remotes.GitHub(remote) can throw if there is no '/' in the string + return isnothing(remote) ? nothing : GitHub.Remote(remote) + catch + return nothing + end + else + return Remotes.GitHub(m[2], m[3]) + end end end @@ -545,24 +539,6 @@ function inbase(m::Module) end end -# Repository host from repository url -# i.e. "https://github.com/something" => RepoGithub -# "https://bitbucket.org/xxx" => RepoBitbucket -# If no match, returns RepoUnknown -function repo_host_from_url(repoURL::String) - if occursin("bitbucket", repoURL) - return RepoBitbucket - elseif occursin("github", repoURL) || isempty(repoURL) - return RepoGithub - elseif occursin("gitlab", repoURL) - return RepoGitlab - elseif occursin("azure", repoURL) - return RepoAzureDevOps - else - return RepoUnknown - end -end - # Find line numbers. # ------------------ @@ -579,32 +555,6 @@ function linerange(text, from) return lines > 0 ? (from:(from + lines + 1)) : (from:from) end -struct LineRangeFormatting - prefix::String - separator::String - - function LineRangeFormatting(host::RepoHost) - if host === RepoAzureDevOps - new("&line=", "&lineEnd=") - elseif host == RepoBitbucket - new("", ":") - elseif host == RepoGitlab - new("L", "-") - else - # default is github-style - new("L", "-L") - end - end -end - -function format_line(range::AbstractRange, format::LineRangeFormatting) - if length(range) <= 1 - string(format.prefix, first(range)) - else - string(format.prefix, first(range), format.separator, last(range)) - end -end - newlines(s::AbstractString) = count(c -> c === '\n', s) newlines(other) = 0 diff --git a/src/Writers/HTMLWriter.jl b/src/Writers/HTMLWriter.jl index d71c279c76..96ce6e6f3c 100644 --- a/src/Writers/HTMLWriter.jl +++ b/src/Writers/HTMLWriter.jl @@ -1180,7 +1180,7 @@ function render_navbar(ctx, navnode, edit_page_link::Bool) # Set the logo and name for the "Edit on.." button. if edit_page_link && (ctx.settings.edit_link !== nothing) && !ctx.settings.disable_git - host_type = Utilities.repo_host_from_url(ctx.doc.user.repo) + host_type = Utilities.repo_host_from_url(ctx.doc.user.remote) if host_type == Utilities.RepoGitlab host = "GitLab" logo = "\uf296" @@ -1208,7 +1208,7 @@ function render_navbar(ctx, navnode, edit_page_link::Bool) # need to set users path relative the page itself pageurl = joinpath(first(splitdir(getpage(ctx, navnode).source)), pageurl) end - Utilities.url(ctx.doc.user.repo, pageurl, commit=edit_branch) + Utilities.edit_url(ctx.doc.user.remote, pageurl, commit=edit_branch) end if url !== nothing edit_verb = (edit_branch === nothing) ? "View" : "Edit" @@ -1569,7 +1569,7 @@ function domify_doc(ctx, navnode, md::Markdown.MD) ret = section(div(domify(ctx, navnode, Utilities.dropheaders(markdown)))) # When a source link is available then print the link. if !ctx.settings.disable_git - url = Utilities.url(ctx.doc.internal.remote, ctx.doc.user.repo, result) + url = Utilities.source_url(ctx.doc.user.remote, result) if url !== nothing push!(ret.nodes, a[".docs-sourcelink", :target=>"_blank", :href=>url]("source")) end diff --git a/src/Writers/LaTeXWriter.jl b/src/Writers/LaTeXWriter.jl index 889b77e198..9553ce4622 100644 --- a/src/Writers/LaTeXWriter.jl +++ b/src/Writers/LaTeXWriter.jl @@ -314,7 +314,7 @@ function latexdoc(io::IO, md::Markdown.MD, page, doc) for (markdown, result) in zip(md.content, md.meta[:results]) latex(io, Utilities.dropheaders(markdown), page, doc) # When a source link is available then print the link. - url = Utilities.url(doc.internal.remote, doc.user.repo, result) + url = Utilities.source_url(doc.user.remote, result) if url !== nothing link = "\\href{$url}{\\texttt{source}}" _println(io, "\n", link, "\n") diff --git a/test/examples/make.jl b/test/examples/make.jl index d7dfc9553a..3934c227ef 100644 --- a/test/examples/make.jl +++ b/test/examples/make.jl @@ -1,10 +1,5 @@ # Defines the modules referred to in the example docs (under src/) and then builds them. # It can be called separately to build the examples/, or as part of the test suite. -# -# It defines a set of variables (`examples_*`) that can be used in the tests. -# The `examples_root` should be used to check whether this file has already been included -# or not and should be kept unique. -isdefined(@__MODULE__, :examples_root) && error("examples_root is already defined\n$(@__FILE__) included multiple times?") # The `Mod` and `AutoDocs` modules are assumed to exist in the Main module. (@__MODULE__) === Main || error("$(@__FILE__) must be included into Main.") @@ -182,7 +177,6 @@ htmlbuild_pages = Any[ "Library" => [ "lib/functions.md", "lib/autodocs.md", - "lib/editurl.md", ], hide("Hidden Pages" => "hidden/index.md", Any[ "Page X" => "hidden/x.md", @@ -199,11 +193,16 @@ htmlbuild_pages = Any[ "example-output.md", "fonts.md", "linenumbers.md", + "EditURL" => [ + "editurl/good.md", + "editurl/bad.md", + "editurl/ugly.md", + ], ] -function html_doc(build_directory, mathengine) +function html_doc(build_directory, mathengine; htmlkwargs=(;), kwargs...) @quietly withassets("images/logo.png", "images/logo.jpg", "images/logo.gif") do - makedocs( + makedocs(; debug = true, root = examples_root, build = "builds/$(build_directory)", @@ -212,7 +211,7 @@ function html_doc(build_directory, mathengine) pages = htmlbuild_pages, expandfirst = expandfirst, doctest = false, - format = Documenter.HTML( + format = Documenter.HTML(; assets = [ "assets/favicon.ico", "assets/custom.css", @@ -225,7 +224,9 @@ function html_doc(build_directory, mathengine) mathengine = mathengine, highlights = ["erlang", "erlang-repl"], footer = "This footer has been customized.", - ) + htmlkwargs... + ), + kwargs... ) end end @@ -244,6 +245,7 @@ examples_html_doc = if "html" in EXAMPLE_BUILDS ), ), )), + htmlkwargs = (; edit_link = :commit), ) else @info "Skipping build: HTML/deploy" @@ -323,7 +325,7 @@ examples_html_local_doc = if "html-local" in EXAMPLE_BUILDS sitename = "Documenter example", pages = htmlbuild_pages, expandfirst = expandfirst, - + repo = "https://dev.azure.com/org/project/_git/repo?path={path}&version={commit}{line}&lineStartColumn=1&lineEndColumn=1", linkcheck = true, linkcheck_ignore = [r"(x|y).md", "z.md", r":func:.*"], format = Documenter.HTML( diff --git a/test/examples/src/lib/editurl.md b/test/examples/src/editurl/bad.md similarity index 55% rename from test/examples/src/lib/editurl.md rename to test/examples/src/editurl/bad.md index 5d88d24713..fcb1993eec 100644 --- a/test/examples/src/lib/editurl.md +++ b/test/examples/src/editurl/bad.md @@ -3,3 +3,5 @@ ```@meta EditURL = "/foo/bar/baz/I/Should/Not/Exist" ``` + +Edit link should be missing, since it's set to a bad value. diff --git a/test/examples/src/editurl/good.md b/test/examples/src/editurl/good.md new file mode 100644 index 0000000000..755e173449 --- /dev/null +++ b/test/examples/src/editurl/good.md @@ -0,0 +1,7 @@ +# Good EditURL + +```@meta +EditURL = "../../../../CHANGELOG.md" +``` + +Good `EditURL`: a relative path pointing to the CHANGELOG (for no particular reason). diff --git a/test/examples/src/editurl/ugly.md b/test/examples/src/editurl/ugly.md new file mode 100644 index 0000000000..805743364f --- /dev/null +++ b/test/examples/src/editurl/ugly.md @@ -0,0 +1,8 @@ +# Absolute EditURL + +```@meta +EditURL = "https://github.com/JuliaDocs/Documenter.jl/pulls" +``` + +Should point to the pull request tab of the Documenter repository. +Also, not actually ugly. diff --git a/test/remotes.jl b/test/remotes.jl new file mode 100644 index 0000000000..be5129323a --- /dev/null +++ b/test/remotes.jl @@ -0,0 +1,72 @@ +module RemoteTests +using Test +using Documenter +using .Remotes: repofile, repourl, issueurl, URL, GitHub + +@testset "RepositoryRemote" begin + let r = URL("https://github.com/FOO/BAR/blob/{commit}{path}#{line}") + @test repourl(r) === nothing + @test repofile(r, "master", "src/foo.jl") == "https://github.com/FOO/BAR/blob/master/src/foo.jl#" + @test repofile(r, "master", "src/foo.jl", 5:5) == "https://github.com/FOO/BAR/blob/master/src/foo.jl#L5" + @test repofile(r, "master", "src/foo.jl", 10) == "https://github.com/FOO/BAR/blob/master/src/foo.jl#L10" + @test repofile(r, "master", "src/foo.jl", 5:15) == "https://github.com/FOO/BAR/blob/master/src/foo.jl#L5-L15" + @test issueurl(r, "123") === nothing + end + + # Default linerange formatting is GitHub-style + let r = URL("http://example.org/{commit}/x{path}?lines={line}", "https://example.org/X") + @test repourl(r) == "https://example.org/X" + @test repofile(r, "123abc", "src/foo.jl") == "http://example.org/123abc/x/src/foo.jl?lines=" + @test repofile(r, "123abc", "src/foo.jl", 5:5) == "http://example.org/123abc/x/src/foo.jl?lines=L5" + @test repofile(r, "123abc", "src/foo.jl", 5:15) == "http://example.org/123abc/x/src/foo.jl?lines=L5-L15" + @test issueurl(r, "123") === nothing + end + + # Different line range formatting for URLs containing 'bitbucket' + let r = URL("https://bitbucket.org/foo/bar/src/{commit}{path}#lines-{line}") + @test repourl(r) === nothing + @test repofile(r, "mybranch", "src/foo.jl") == "https://bitbucket.org/foo/bar/src/mybranch/src/foo.jl#lines-" + @test repofile(r, "mybranch", "src/foo.jl", 5:5) == "https://bitbucket.org/foo/bar/src/mybranch/src/foo.jl#lines-5" + @test repofile(r, "mybranch", "src/foo.jl", 5:15) == "https://bitbucket.org/foo/bar/src/mybranch/src/foo.jl#lines-5:15" + @test issueurl(r, "123") === nothing + end + + # Different line range formatting for URLs containing 'gitlab' + let r = URL("https://gitlab.mydomain.eu/foo/bar/-/blob/{commit}{path}#{line}", "https://gitlab.mydomain.eu/foo/bar/") + @test repourl(r) == "https://gitlab.mydomain.eu/foo/bar/" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl") == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl", 5:5) == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#L5" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl", 5:15) == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#L5-15" + @test issueurl(r, "123") === nothing + end + + # Different line range formatting for URLs containing 'azure' + let r = URL("https://gitlab.mydomain.eu/foo/bar/-/blob/{commit}{path}#{line}", "https://gitlab.mydomain.eu/foo/bar/") + @test repourl(r) == "https://gitlab.mydomain.eu/foo/bar/" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl") == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl", 5:5) == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#L5" + @test repofile(r, "v1.2.3-rc3+foo", "src/foo.jl", 5:15) == "https://gitlab.mydomain.eu/foo/bar/-/blob/v1.2.3-rc3+foo/src/foo.jl#L5-15" + @test issueurl(r, "123") === nothing + end + + # GitHub remote + let r = GitHub("JuliaDocs", "Documenter.jl") + @test repourl(r) == "https://github.com/JuliaDocs/Documenter.jl" + @test repofile(r, "mybranch", "src/foo.jl") == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl" + @test repofile(r, "mybranch", "src/foo.jl", 5) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5" + @test repofile(r, "mybranch", "src/foo.jl", 5:5) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5" + @test repofile(r, "mybranch", "src/foo.jl", 5:8) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5-L8" + @test issueurl(r, "123") == "https://github.com/JuliaDocs/Documenter.jl/issues/123" + end + + let r = GitHub("JuliaDocs/Documenter.jl") + @test repourl(r) == "https://github.com/JuliaDocs/Documenter.jl" + @test repofile(r, "mybranch", "src/foo.jl") == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl" + @test repofile(r, "mybranch", "src/foo.jl", 5) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5" + @test repofile(r, "mybranch", "src/foo.jl", 5:5) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5" + @test repofile(r, "mybranch", "src/foo.jl", 5:8) == "https://github.com/JuliaDocs/Documenter.jl/blob/mybranch/src/foo.jl#L5-L8" + @test issueurl(r, "123") == "https://github.com/JuliaDocs/Documenter.jl/issues/123" + end +end + +end # module diff --git a/test/runtests.jl b/test/runtests.jl index fb9e43a22e..68bf61872c 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -19,6 +19,7 @@ include("TestUtilities.jl"); using .TestUtilities include("except.jl") include("utilities.jl") include("markdown2.jl") + include("remotes.jl") # DocChecks tests include("docchecks.jl") diff --git a/test/utilities.jl b/test/utilities.jl index 8a30ff3dfc..7ff548db9a 100644 --- a/test/utilities.jl +++ b/test/utilities.jl @@ -156,7 +156,7 @@ end @test Documenter.Utilities.format_commit("test", Documenter.Utilities.RepoAzureDevOps) == "GBtest" # URL building - filepath = string(first(methods(Documenter.Utilities.url)).file) + filepath = string(first(methods(Documenter.Utilities.source_url)).file) Sys.iswindows() && (filepath = replace(filepath, "/" => "\\")) # work around JuliaLang/julia#26424 let expected_filepath = "/src/Utilities/Utilities.jl" Sys.iswindows() && (expected_filepath = replace(expected_filepath, "/" => "\\")) @@ -164,6 +164,7 @@ end end mktempdir() do path + remote = Documenter.Utilities.Remotes.URL("//blob/{commit}{path}#{line}") path_repo = joinpath(path, "repository") mkpath(path_repo) cd(path_repo) do @@ -181,8 +182,10 @@ end # Run tests commit = Documenter.Utilities.repo_commit(filepath) - @test Documenter.Utilities.url("//blob/{commit}{path}#{line}", filepath) == "//blob/$(commit)/src/SourceFile.jl#" - @test Documenter.Utilities.url(nothing, "//blob/{commit}{path}#{line}", Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" + @test Documenter.Utilities.edit_url(remote, filepath) == "//blob/$(commit)/src/SourceFile.jl#" + # The '//blob/..' remote conflicts with the github.com origin.url of the repository and source_url() + # picks the wrong remote currently () + @test_broken Documenter.Utilities.source_url(remote, Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" # repo_root & relpath_from_repo_root @test Documenter.Utilities.repo_root(filepath) == dirname(abspath(joinpath(dirname(filepath), ".."))) # abspath() keeps trailing /, hence dirname() @@ -203,8 +206,8 @@ end # Run tests commit = Documenter.Utilities.repo_commit(filepath) - @test Documenter.Utilities.url("//blob/{commit}{path}#{line}", filepath) == "//blob/$(commit)/src/SourceFile.jl#" - @test Documenter.Utilities.url(nothing, "//blob/{commit}{path}#{line}", Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" + @test Documenter.Utilities.edit_url(remote, filepath) == "//blob/$(commit)/src/SourceFile.jl#" + @test_broken Documenter.Utilities.source_url(remote, Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" # repo_root & relpath_from_repo_root @test Documenter.Utilities.repo_root(filepath) == dirname(abspath(joinpath(dirname(filepath), ".."))) # abspath() keeps trailing /, hence dirname() @@ -238,8 +241,8 @@ end @test isfile(filepath) - @test Documenter.Utilities.url("//blob/{commit}{path}#{line}", filepath) == "//blob/$(commit)/src/SourceFile.jl#" - @test Documenter.Utilities.url(nothing, "//blob/{commit}{path}#{line}", Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" + @test Documenter.Utilities.edit_url(remote, filepath) == "//blob/$(commit)/src/SourceFile.jl#" + @test Documenter.Utilities.source_url(remote, Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" # repo_root & relpath_from_repo_root @test Documenter.Utilities.repo_root(filepath) == dirname(abspath(joinpath(dirname(filepath), ".."))) # abspath() keeps trailing /, hence dirname() @@ -249,6 +252,28 @@ end @test Documenter.Utilities.repo_root(tempname()) == nothing @test Documenter.Utilities.relpath_from_repo_root(tempname()) == nothing end + + # This tests the case where the origin.url is some unrecognised Git hosting service, in which case we are unable + # to parse the remote out of the origin.url value and we fallback to the user-provided remote. + path_repo_github = joinpath(path, "repository-not-github") + mkpath(path_repo_github) + cd(path_repo_github) do + # Create a simple mock repo in a temporary directory with a single file. + @test trun(`$(git()) init`) + @test trun(`$(git()) config user.email "tester@example.com"`) + @test trun(`$(git()) config user.name "Test Committer"`) + @test trun(`$(git()) remote add origin git@this-is-not-github.com:JuliaDocs/Documenter.jl.git`) + mkpath("src") + filepath = abspath(joinpath("src", "SourceFile.jl")) + write(filepath, "X") + @test trun(`$(git()) add -A`) + @test trun(`$(git()) commit -m"Initial commit."`) + + # Run tests + commit = Documenter.Utilities.repo_commit(filepath) + @test Documenter.Utilities.edit_url(remote, filepath) == "//blob/$(commit)/src/SourceFile.jl#" + @test Documenter.Utilities.source_url(remote, Documenter.Utilities, filepath, 10:20) == "//blob/$(commit)/src/SourceFile.jl#L10-L20" + end end import Documenter.Documents: Document, Page, Globals From d63b0196bb689c02f3454b8f00b5c962e0d6b572 Mon Sep 17 00:00:00 2001 From: Morten Piibeleht Date: Mon, 18 Jul 2022 22:06:46 +1200 Subject: [PATCH 2/3] Don't include editurl tests in LaTeX example --- test/examples/make.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/examples/make.jl b/test/examples/make.jl index 3934c227ef..4074fb1731 100644 --- a/test/examples/make.jl +++ b/test/examples/make.jl @@ -408,7 +408,6 @@ examples_latex_doc = if "latex" in EXAMPLE_BUILDS "Library" => [ "lib/functions.md", "lib/autodocs.md", - "lib/editurl.md", ], "Expandorder" => [ "expandorder/00.md", From 544d53094083418cea00647d10a98253988b2e5b Mon Sep 17 00:00:00 2001 From: Morten Piibeleht Date: Mon, 18 Jul 2022 22:16:34 +1200 Subject: [PATCH 3/3] one more.. --- test/examples/make.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/test/examples/make.jl b/test/examples/make.jl index 4074fb1731..b300582a7c 100644 --- a/test/examples/make.jl +++ b/test/examples/make.jl @@ -491,7 +491,6 @@ examples_latex_texonly_doc = if "latex_texonly" in EXAMPLE_BUILDS "Library" => [ "lib/functions.md", "lib/autodocs.md", - "lib/editurl.md", ], "Expandorder" => [ "expandorder/00.md",