API for plugin assets

[LazyScholar]

First time melding here. Hope i did not break anything.

[fredrikekre]

Thanks! Can you make 2 PRs instead? The first commit seems like a bugfix and could be merged as is I think while the second commit might need some discussion?

[LazyScholar]

I could do it. Although the first PR is a very minor fix. So it can wait (unless you want to keep PRs for issues strictly separated).

[mortenpi]

I could do it. Although the first PR is a very minor fix. So it can wait (unless you want to keep PRs for issues strictly separated).

Generally we like to keep unrelated changes in different PRs, so it would be awesome if you could open a separate one for #1575.

[mortenpi]

Re #1612 PR: why are you looping over the builder pipeline when trying to check for assets? Wouldn't looping over the plugins field of Document would make more sense?

[LazyScholar]

I could do it. Although the first PR is a very minor fix. So it can wait (unless you want to keep PRs for issues strictly separated).

Generally we like to keep unrelated changes in different PRs, so it would be awesome if you could open a separate one for #1575.

Done.

[LazyScholar]

I deleted some dabbling comments i made while experimenting with other solutions. I settled with those two commits:

• f6d17a0 : asset registration by asset_path::String, asset_list::Vector{HTMLAsset} fields passed by the plugin struct on call of makedocs -> needs some notes in the docs
• 77b7baf : asset addition by type registered functions called by looping over plugins from within HTMLWriter.jl

[LazyScholar]

Thanks for iterating on this! This exposes a completely new API for plugin writers, so it's worth taking some time to get this right.

I agree.

* I think it would be better to rely on multiple dispatch, rather than forcing the plugin writer to implement particular fields/properties. How about a function, like `extra_html_assets(::Plugin)`, that each plugin could implement, and would return the list of `HTMLAssets`?

Like those functions? They should do that only the names should be changed or are they not what you have in mind? https://github.com/JuliaDocs/Documenter.jl/blob/77b7baf5911b3155332565e11020106afca49a8a/src/Writers/HTMLWriter.jl#L964-L983 Although the plugin struct fields could be optional. The moment fields with the same names would be used then there would it be necessary to check collisions/false input.

* Could you also add a new documentation page where we could tart documenting this API? I think it might be worth documenting it first to get the design down, and then focus on the implementation details.

A extra "Plug-in API" page Or a "How to Plug-in" page? I could start a page but i originally just wanted to add this and maybe refactor the asset addition structure for issue 1433. I will start it but i wont polish/finish it, sorry.

TODO: I should also do asset input checking like here: https://github.com/JuliaDocs/Documenter.jl/blob/77b7baf5911b3155332565e11020106afca49a8a/src/Writers/HTMLWriter.jl#L440-L444

[mortenpi]

Like those functions?

Ah, yes. I think I was looking at an old commit the other day. But hmm. A couple thoughts:

• For full flexibility, I think it might be better if the user specifies explicitly each and every asset that gets copied, rather than just overlaying a whole directory onto build/. I reckon in most cases it will be like 1 JS file and 1 CSS file that the plugin wants to add anyway.
• I think it would be cleaner and easier for the plugin writer if this was a single function to implement, rather than multiple.

We could maybe expand HTMLAsset in a way that you can also specify a source and destination file.

struct HTMLAsset
    class :: Symbol
    uri :: String
    source :: Union{String, Nothing}
...

where source is a full path to a local asset files, and a nothing value would correspond to local = false. HTMLAsset could then have a new constructor:

HTMLAsset(class::Symbol, src::String, dst::String = joinpath("assets", basename(src)))

The most likely case for the plugin writer would be doing something like HTMLAsset(:js, "path/to/myfancy.js"), which will end up copying the files to assets/myfancy.js (I addition to including remote assets). And all the information will be contained in a single array of HTMLAssets.

Do you have any thoughts or see any issues with this design?

A extra "Plug-in API" page Or a "How to Plug-in" page?

Could just have a page called "Plugins" before between "Showcase" and "Library". Just a brief example showing how to implement your own plugin would be enough.

[LazyScholar]

Do you have any thoughts or see any issues with this design?

I think that would be perfect. So registering the HTMLAsset will push them into a vector. Which can be looped over to gather all assets. And with that static vector one can build the asset links of each page much cleaner.

Edit: And for the lazy ones we could allow to pass a directory path where we test if it is a dir and add the allowed assets from there (which will be converted to individual assets in the vector).

Which leads me to the next points i would change. Adding a additional field like index::AbstractFloat (defaulting to 0.0) allowing to sort the assets. And refactoring the asset() calls in order to prepare for issue #1433 and set indices in order to place them in a certain order on the HTML header/footer (index >= 0 are on header and index < 0 are on the footer).

A extra "Plug-in API" page Or a "How to Plug-in" page?

Could just have a page called "Plugins" before between "Showcase" and "Library". Just a brief example showing how to implement your own plugin would be enough.

Okay, so a "How to Plug-In" page. I will write something up on how far i understand it.

[LazyScholar]

Changes on 4407a4c:

• moved the checks of HTMLAsset to asset() and added checks for new uri&src fields
• added documentation on HTMLAsset and asset()
• now the plugin assets get collected into HTMLContext.asset_list
• after collecting all plugin assets the files listed on that list will be copied over to the build directory
• the HTMLContext.asset_list is used to create the additional DOM entries the same way as the user provided HTML.assets list

[LazyScholar]

Still pondering on how to write a short overview on "How to Plug-in" for the docs. I am afraid it is no easy task. Looking at how DocumenterCitations.jl does the hook into Documenter.jlit is necessary to include quite a few sub modules and functions one wants to hook into?

A example of the current way a plugin can pass his assets is (see: https://github.com/ali-ramadhan/DocumenterCitations.jl/blob/116dac41942dc012cf3af3b5702530c805b50ac2/src/citations.jl#L5-L10):

const asset_dir = normpath(abspath(joinpath(@__DIR__, "..", "assets")))
const asset_list = [ asset(joinpath(asset_dir,"citations.css"),class=:css,islocal=true) ]

function HTMLWriter.add_plugin_assets(::Type{CitationBibliography})::Vector{HTMLWriter.HTMLAsset}
  return DocumenterCitations.asset_list
end

[LazyScholar]

Adding a additional field like index::AbstractFloat (defaulting to 0.0) allowing to sort the assets.

I am not a huge fan of this idea -- it doesn't really reliably guarantee anything. In principle, the assets from different plugins should not interact with each other. Of course, we don't live in an ideal world.. and this approach would sometimes allow for a workaround.

Do you think it would be enough if we guarantee that the assets get inserted in the order in which the plugins are passed to makedocs?

I had a look how DocumenterCitations.jlhooked into Documenter.jl and it was done with a indexed based ordering too.

https://github.com/JuliaDocs/Documenter.jl/blob/cacf739b425b088e285af14a214cf4865bfb0019/src/Builder.jl#L74-L80

I think it is a robust system where one can set indices and others can chose their own numbers to position their elements in relation. And with my sparse knowledge on web dev i assume it is quite common that certain .js files must be loaded in a certain order. Of course a system where one can say "switch out fileA with fileB and/or fileC has to be loaded after fileA" would be nicer.

I think the HTMLContext.asset_list can be used to implement it (push the other Documenter.jl assets in as well and use whatever rule to order it and manage it) or not. And one ordering rule could be "elements with the same index shall remain in the same relation to each other" (order in the list should correlate with the order of appending).

Non interacting plugins... that would be a sad world comparable to non interacting Julia packages.

I just wanted to see if i can put a check mark on another issue with the knowledge/time i had to acquire/use in order to get my issue solved.

[LazyScholar]

I added a test in #1663 which is now broken, because absolute paths are now okay. With that in mind, could you add tests that:

1. Test the absolute path case, including that `assetlink` has correct paths (in the vein of the new tests)?

Yes. src has to be a relative path for Documenter.jl users and a absolute path for Documenter.jl plugin authors.

• In case for Documenter.jl users src is only used to copy the src string into the HTML header without checks (no file operations are done in consequence of using asset() as far as i can see).
• In case for plugin authors the src+dst strings are used to place the file in the build path and to generate the string for the HTML header (so the src path has to point to a valid file).

That might indeed lead to problems for Documenter.jl users later on. As the paths copied into the HTML header might not be valid. It would be nice to have a way to detect if asset() has been called by a user or a plugin. Or to introduce a _asset() function which gets wrapped by asset() and a new plugin_asset() function and give those functions dedicated checks (or even just a second function without wrapping)?

Update: A further way to limit the Documenter.jl users src to relative paths is to place checks in the constructor of HTML.

2. A test for the case when `src` is a remote URL (`islocal = false`), but `dst` is set. I think it should error, but I am not sure if it does?

~~It does not. I will put some checks in place for 2. (although it might be useful to gather online source files locally, which in turn is a security risk).~~ Done

[LazyScholar]

@mortenpi is the current implementation okay so far? Possible changes would be:

• place a absolute path check in HTML struct constructor
• split the function asset() function into a function for users and plugin authors

You mentioned the PR in a issue related to the location/ordering of script/html assets shall i add a indexing number similar to Documenter.jl/src/Builder.jl (mentioned above)?

[mortenpi]

@LazyScholar: apologies, I was not active here for a while. I still think a feature like this would be good. Do you think you would have time to revisit this PR in the near future? If yes, I could do another review round now.

[LazyScholar]

@LazyScholar: apologies, I was not active here for a while. I still think a feature like this would be good. Do you think you would have time to revisit this PR in the near future? If yes, I could do another review round now.

I will try, although tight on time. Will rebase and start from scratch.

[LazyScholar]

@mortenpi found some time. Have a look.

[mortenpi]

Thank you @LazyScholar! And sorry for the delay here. I am actually having a little bit of trouble reviewing this, since I don't have a clear vision of how exactly this will be used.

1. Could we list a couple of brief, but specific use cases for this API? I.e. what problem exactly and how this will solve? We could then benchmark the current implementation against those use case descriptions, to see if we should improve on it or not.
2. In the long term, it would be handy to have a brief manual page explaining how plugin writers should use these APIs. I think this could help with hashing out the API too. But on the other hand this is quite a bit of extra writing work, so this can also come later, and maybe also something I can tackle.

[LazyScholar]

No problem @mortenpi, it is not urgent.

I will describe what i aimed for and how i imagine it to work. To demo it try it with https://github.com/ali-ramadhan/DocumenterCitations.jl/pull/52 .

The main aim is to pass HTML-assets directly from one plug-in. As Documenter.jl already provides a solution for users via asset (create HTML-asset element) and the HTML argument assets (to register special assets) i tried to reuse most of it.

To use it the plug-in author has to use

function HTMLWriter.plugin_html_assets(::Type{"<name of the plugin>"})::Tuple{String, Vector{HTMLWriter.HTMLAsset}}
  return normpath(abspath("<here path of directory containing the assets (only the assets)>")), [ asset("<relative path of the asset to be included (has to include the name of the asset directory), same rules as for user assets>", class=:css, islocal=true) ]
end

Example:

const asset_dir = normpath(abspath(joinpath(@__DIR__, "..", "citations_assets")))
const asset_list = [ asset(joinpath("citations_assets","citations.css"),class=:css,islocal=true) ]

function HTMLWriter.plugin_html_assets(::Type{CitationBibliography})::Tuple{String, Vector{HTMLWriter.HTMLAsset}}
  return DocumenterCitations.asset_dir, DocumenterCitations.asset_list
end

How it should work:

1. The new function plugin_assets!(ctx::HTMLContext) is called within render(doc::Documents.Document, settings::HTML=HTML()).
2. This function loops through the plug-in list and calls the new function plugin_html_assets(::Documenter.Plugin).
3. If one plugin registered this function with its own name it can return the absolute path to a asset directory and a vector of HTML assets.
4. In the case that a directory path has been passed the directory will fully copied to the <build directory>/plugin directory.
5. The islocal=true asset paths will be prepended with plugin and added to the new attribute asset_list::Vector{HTMLAsset} of struct HTMLContext.
6. Every time render_head(ctx, navnode) is called this vector asset_list::Vector{HTMLAsset} will be passed to asset_links(src, ctx.asset_list) which modifies and adds those plug-in assets to the head.

I tried to hold it very simple as my first attempt seemed too complicated. Provided documentation to the functions created by me. I wont create a own documentation page. I did not add test cases (no good idea on how to).

Possible improvements (not doing this):

• Refactor and generalize the existing function which copies assets to the build directory (arguments: list of filetypes to copy, source dir, target dir, overwrite flag) in order to reuse it in the new function plugin_assets!(ctx::HTMLContext).
• Selectively add plug-in assets only to headers of sites where those where used.
• Allow users and plug-in authors to somehow dictate the order in which those should be included to the headers (floating point numbers would be the easiest like https://github.com/JuliaDocs/Documenter.jl/blob/0d088e4b42a972bf710c158bed2d803500a6d31d/src/Builder.jl#L74-L80)
• check if and how complicated plug-in assets work if the files have different directory depths (simple paths are already amended)
• check for already existing plug-in directories rename them automatically and amend the corresponding asset paths - or throw errors

[LazyScholar]

https://github.com/JuliaDocs/Documenter.jl/issues/1943#issuecomment-1262989188

Sure, feel free to add tests. The Structure for the plug-in should be roughly as follows:

.
├── docs
│   ├── make.jl
│   └── src
│       └── index.md
├── mermaid_assets
│   └── mermaid_init.js
├── Project.toml
└── src
    └── Mermaid.jl

With those contents:

src/Mermaid.jl:

module Mermaid

using Documenter
using Documenter.Expanders
using Documenter.Selectors
using Documenter.Writers.HTMLWriter

# register plugin
struct MermaidPlugin <: Documenter.Plugin
  some_val::Bool
end
export MermaidPlugin
function MermaidPlugin()
  return MermaidPlugin(true)
end

# add html assets
const asset_dir = normpath(abspath(joinpath(@__DIR__, "..", "mermaid_assets")))
const asset_list = [ asset("https://cdn.jsdelivr.net/npm/mermaid/dist/mermaid.min.js", class=:js, islocal=false)
                     asset(joinpath("mermaid_assets","mermaid_init.js"), class=:js, islocal=true) ]
function HTMLWriter.plugin_html_assets(::Type{MermaidPlugin})::Tuple{String, Vector{HTMLWriter.HTMLAsset}}
  return Mermaid.asset_dir, Mermaid.asset_list
end

# register expander
abstract type MermaidExpander <: Expanders.ExpanderPipeline end
Selectors.order(::Type{MermaidExpander}) = 11.05
Selectors.matcher(::Type{MermaidExpander}, node, page, doc) = Expanders.iscode(node, r"^@mermaid")

# register transformation
function Selectors.runner(::Type{MermaidExpander}, x, page, doc)
  raw_html = """<div class="mermaid">$(x.t.code)</div>"""
  x.t = Documenter.Documents.RawNode(:html, raw_html)
end

end # module Mermaid

mermaid_assets/mermaid_init.js:

mermaid.initialize({startOnLoad:true});

docs/make.jl:

# get path of docs folders
docdir = dirname(@__FILE__);

# check if run by CI
CIflag = get(ENV,"CI","") != "";

# add look up paths
push!(LOAD_PATH, joinpath(docdir, ".."));

using Documenter;
using Mermaid;

makedocs(MermaidPlugin(),
         sitename = "Mermaid",
         authors = "???",
         format = Documenter.HTML(edit_link = CIflag ? :commit : "main" ),
         doctest = false,
         clean = true,
         pages = ["Home" => "index.md"],
        );

docs/src/index.md:

# Mermaid Test

```@mermaid
graph TD;
    A-->B;
    A-->C;
    B-->D;
    C-->D;
```

Which provides this:

After deleting some JS lines to circumvent those current errors:

[MilesCranmer]

Could this be merged soon-ish? Would be super useful!

[MilesCranmer]

Pinging this again. Would be very useful to have mermaid.js enabled so I can generate flowcharts in my docs. What is left to be done before merging?