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

Allow for duplicate docstrings in @docs and @autodocs, issue #1079 #1570

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/Documents.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import ..Documenter.Utilities.Markdown2
using DocStringExtensions
import Markdown
using Unicode

import Base64: base64encode
# Pages.
# ------

Expand Down Expand Up @@ -94,7 +94,8 @@ struct IndexNode
build :: String # Path to the file where this index will appear.
source :: String # Path to the file where this index was written.
elements :: Vector # (object, doc, page, mod, cat)-tuple for constructing links.

include_ids :: Vector{<:AbstractString} # list of `id_str`s to include into index block (to avoid duplicates)

function IndexNode(;
# TODO: Fix difference between uppercase and lowercase naming of keys.
# Perhaps deprecate the uppercase versions? Same with `ContentsNode`.
Expand All @@ -103,9 +104,13 @@ struct IndexNode
Order = [:module, :constant, :type, :function, :macro],
build = error("missing value for `build` in `IndexNode`."),
source = error("missing value for `source` in `IndexNode`."),
include_ids = [""],
others...
)
new(Pages, Modules, Order, build, source, [])
new(
Pages, Modules, Order, build, source, [],
[base64encode(str_id) for str_id ∈ include_ids]
)
end
end

Expand Down Expand Up @@ -419,9 +424,10 @@ function populate!(index::IndexNode, document::Document)
for (object, doc) in document.internal.objects
page = relpath(doc.page.build, dirname(index.build))
mod = object.binding.mod
id_str = object.id_str
# Include *all* signatures, whether they are `Union{}` or not.
cat = Symbol(lowercase(Utilities.doccat(object.binding, Union{})))
if _isvalid(page, index.pages) && _isvalid(mod, index.modules) && _isvalid(cat, index.order)
if _isvalid(id_str, index.include_ids) && _isvalid(page, index.pages) && _isvalid(mod, index.modules) && _isvalid(cat, index.order)
push!(index.elements, (object, doc, page, mod, cat))
end
end
Expand Down
10 changes: 6 additions & 4 deletions src/Expanders.jl
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ Selectors.order(::Type{RawBlocks}) = 11.0

Selectors.matcher(::Type{TrackHeaders}, node, page, doc) = isa(node, Markdown.Header)
Selectors.matcher(::Type{MetaBlocks}, node, page, doc) = iscode(node, "@meta")
Selectors.matcher(::Type{DocsBlocks}, node, page, doc) = iscode(node, "@docs")
Selectors.matcher(::Type{AutoDocsBlocks}, node, page, doc) = iscode(node, "@autodocs")
Selectors.matcher(::Type{DocsBlocks}, node, page, doc) = iscode(node, r"^@docs")
Selectors.matcher(::Type{AutoDocsBlocks}, node, page, doc) = iscode(node, r"^@autodocs")
Selectors.matcher(::Type{EvalBlocks}, node, page, doc) = iscode(node, "@eval")
Selectors.matcher(::Type{IndexBlocks}, node, page, doc) = iscode(node, "@index")
Selectors.matcher(::Type{ContentsBlocks}, node, page, doc) = iscode(node, "@contents")
Expand Down Expand Up @@ -273,6 +273,7 @@ end
# -----

function Selectors.runner(::Type{DocsBlocks}, x, page, doc)
id_str = match(r"^@docs[ ]?(.*)$", first(split(x.language, ';', limit = 2)))[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the syntax should be

```@docs id="something"
```

instead? I think that pattern is used elsewhere for e.g. doctest filters etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you ever care about the id or just that it is unique?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the current approach follows the idea that is used in example blocks? there the id/name for continued examples is also just added with a space.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea that's true.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do you ever care about the id or just that it is unique?

If I remember correctly, I need the id_str for the filtering mechanism in @index blocks.
This filtering should allow to

  • exclude docstrings with ids that are not empty from the listing
  • and to list only a subset of docstrings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ever useful to have index with duplicates though? Perhaps there should be a canonical @docs block or something?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ever useful to have index with duplicates though?

I guess not, and I am not sure myself anymore what I wanted this feature for.
I think it was primarily meant for exclusion of docstring subsets, but most of the use cases that come to mind can also be achieved with the Pages kw.
The only (exotic) example I can think of is a single documentation page that has subsections (possibly containing duplicate docstrings in @docs blocks) and an @index is meant to be created for each subsection.

nodes = Union{DocsNode,Markdown.Admonition}[]
curmod = get(page.globals.meta, :CurrentModule, Main)
lines = Utilities.find_block_in_file(x.code, page.source)
Expand Down Expand Up @@ -308,7 +309,7 @@ function Selectors.runner(::Type{DocsBlocks}, x, page, doc)
end
typesig = Core.eval(curmod, Documenter.DocSystem.signature(ex, str))

object = Utilities.Object(binding, typesig)
object = Utilities.Object(binding, typesig, id_str)
# We can't include the same object more than once in a document.
if haskey(doc.internal.objects, object)
push!(doc.internal.errors, :docs_block)
Expand Down Expand Up @@ -370,6 +371,7 @@ end
const AUTODOCS_DEFAULT_ORDER = [:module, :constant, :type, :function, :macro]

function Selectors.runner(::Type{AutoDocsBlocks}, x, page, doc)
id_str = match(r"^@autodocs[ ]?(.*)$", first(split(x.language, ';', limit = 2)))[1]
curmod = get(page.globals.meta, :CurrentModule, Main)
fields = Dict{Symbol, Any}()
lines = Utilities.find_block_in_file(x.code, page.source)
Expand Down Expand Up @@ -417,7 +419,7 @@ function Selectors.runner(::Type{AutoDocsBlocks}, x, page, doc)
if filtered
for (typesig, docstr) in multidoc.docs
path = normpath(docstr.data[:path])
object = Utilities.Object(binding, typesig)
object = Utilities.Object(binding, typesig, id_str)
if isempty(pages)
push!(results, (mod, path, category, object, isexported, docstr))
else
Expand Down
10 changes: 7 additions & 3 deletions src/Utilities/Utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ using Base.Meta
import Base: isdeprecated, Docs.Binding
using DocStringExtensions
import Markdown, LibGit2
import Base64: stringmime
import Base64: stringmime, base64encode
import ..ERROR_NAMES

# escape characters that has a meaning in regex
Expand Down Expand Up @@ -218,9 +218,11 @@ struct Object
binding :: Binding
signature :: Type

function Object(b::Binding, signature::Type)
id_str :: AbstractString

function Object(b::Binding, signature::Type, i_s :: AbstractString = "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function Object(b::Binding, signature::Type, i_s :: AbstractString = "")
function Object(b::Binding, signature::Type, i_s::AbstractString = "")

m = nameof(b.mod) === b.var ? parentmodule(b.mod) : b.mod
new(Binding(m, b.var), signature)
new(Binding(m, b.var), signature, base64encode(i_s) )
end
end

Expand Down Expand Up @@ -256,7 +258,9 @@ end
function Base.print(io::IO, obj::Object)
print(io, obj.binding)
print_signature(io, obj.signature)
print_id(io, obj.id_str)
end
print_id(io::IO, id :: AbstractString ) = isempty(id) ? nothing : print(io, '-', id)
print_signature(io::IO, signature::Union{Union, Type{Union{}}}) = nothing
print_signature(io::IO, signature) = print(io, '-', signature)

Expand Down