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 listing outside URLs in extras #2103

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion assets/js/sidebar/sidebar-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function initialize () {
const items = []
const hasHeaders = Array.isArray(node.headers)
const translate = hasHeaders ? undefined : 'no'
const href = node?.url || `${node.id}.html`
Copy link
Member

Choose a reason for hiding this comment

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

Should we add an icon, such as this one, for URLs?

If so, you can upload this bundle to remixicon.com, add external link, and get the new font back: https://github.com/elixir-lang/ex_doc/blob/main/assets/fonts/RemixIconCollection.remixicon


// Group header.
if (node.group !== group) {
Expand All @@ -78,7 +79,7 @@ export function initialize () {
}

items.push(el('li', {}, [
el('a', {href: `${node.id}.html`, translate}, [node.nested_title || node.title]),
el('a', {href: href, translate}, [node.nested_title || node.title]),
...childList(`node-${node.id}-headers`,
hasHeaders
? renderHeaders(node)
Expand Down

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions lib/ex_doc/formatter/epub.ex
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,17 @@ defmodule ExDoc.Formatter.EPUB do
end

defp generate_extras(config) do
for {_title, extras} <- config.extras do
Enum.each(extras, fn %{id: id, title: title, title_content: title_content, content: content} ->
output = "#{config.output}/OEBPS/#{id}.xhtml"
html = Templates.extra_template(config, title, title_content, content)

if File.regular?(output) do
Utils.warn("file #{Path.relative_to_cwd(output)} already exists", [])
end
for {_title, extras} <- config.extras,
%{id: id, title: title, title_content: title_content, content: content} <- extras,
not is_map_key(extras, :url) do
output = "#{config.output}/OEBPS/#{id}.xhtml"
html = Templates.extra_template(config, title, title_content, content)

if File.regular?(output) do
Utils.warn("file #{Path.relative_to_cwd(output)} already exists", [])
end

File.write!(output, html)
end)
File.write!(output, html)
end
end

Expand Down
36 changes: 26 additions & 10 deletions lib/ex_doc/formatter/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ defmodule ExDoc.Formatter.HTML do
defp generate_extras(extras, config) do
generated_extras =
extras
|> Enum.reject(&is_map_key(&1, :url))
|> with_prev_next()
|> Enum.map(fn {node, prev, next} ->
filename = "#{node.id}.html"
Expand Down Expand Up @@ -349,6 +350,7 @@ defmodule ExDoc.Formatter.HTML do

extras =
config.extras
|> Enum.map(&normalize_extras/1)
|> Task.async_stream(
&build_extra(&1, groups, language, autolink_opts, source_url_pattern),
timeout: :infinity
Expand Down Expand Up @@ -384,10 +386,21 @@ defmodule ExDoc.Formatter.HTML do
end)
end

defp normalize_extras(base) when is_binary(base), do: {base, %{}}
defp normalize_extras({base, opts}), do: {base, Map.new(opts)}

defp disambiguate_id(extra, discriminator) do
Map.put(extra, :id, "#{extra.id}-#{discriminator}")
end

defp build_extra({input, %{url: _} = input_options}, groups, _lang, _auto, _url_pattern) do
input = to_string(input)
title = input_options[:title] || filename_to_title(input)
group = GroupMatcher.match_extra(groups, input)

%{group: group, id: Utils.text_to_id(title), title: title, url: input_options[:url]}
end

defp build_extra({input, input_options}, groups, language, autolink_opts, source_url_pattern) do
input = to_string(input)
id = input_options[:filename] || input |> filename_to_title() |> Utils.text_to_id()
Expand Down Expand Up @@ -447,10 +460,6 @@ defmodule ExDoc.Formatter.HTML do
}
end

defp build_extra(input, groups, language, autolink_opts, source_url_pattern) do
build_extra({input, []}, groups, language, autolink_opts, source_url_pattern)
end

defp normalize_search_data!(nil), do: nil

defp normalize_search_data!(search_data) when is_list(search_data) do
Expand Down Expand Up @@ -595,14 +604,21 @@ defmodule ExDoc.Formatter.HTML do
end

defp extra_paths(config) do
Map.new(config.extras, fn
path when is_binary(path) ->
Enum.reduce(config.extras, %{}, fn
path, acc when is_binary(path) ->
base = Path.basename(path)
{base, Utils.text_to_id(Path.rootname(base))}

{path, opts} ->
base = path |> to_string() |> Path.basename()
{base, opts[:filename] || Utils.text_to_id(Path.rootname(base))}
Map.put(acc, base, Utils.text_to_id(Path.rootname(base)))

{path, opts}, acc ->
if Keyword.has_key?(opts, :url) do
acc
else
base = path |> to_string() |> Path.basename()
name = Keyword.get_lazy(opts, :filename, fn -> Utils.text_to_id(Path.rootname(base)) end)

Map.put(acc, base, name)
end
end)
end
end
60 changes: 30 additions & 30 deletions lib/ex_doc/formatter/html/search_data.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,8 @@ defmodule ExDoc.Formatter.HTML.SearchData do
["searchData=" | ExDoc.Utils.to_json(data)]
end

defp extra(map) do
if custom_search_data = map[:search_data] do
extra_search_data(map, custom_search_data)
else
Comment on lines -22 to -24
Copy link
Author

Choose a reason for hiding this comment

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

Added a clause for this rather than a conditional in the function body.

{intro, sections} = extract_sections_from_markdown(map.source)

intro_json_item =
encode(
"#{map.id}.html",
map.title,
:extras,
intro
)

section_json_items =
for {header, body} <- sections do
encode(
"#{map.id}.html##{Utils.text_to_id(header)}",
header <> " - #{map.title}",
:extras,
body
)
end

[intro_json_item | section_json_items]
end
end

defp extra_search_data(map, custom_search_data) do
Enum.map(custom_search_data, fn item ->
defp extra(%{search_data: search_data} = map) when is_list(search_data) do
Enum.map(search_data, fn item ->
link =
if item.anchor === "" do
"#{map.id}.html"
Expand All @@ -59,6 +31,34 @@ defmodule ExDoc.Formatter.HTML.SearchData do
end)
end

defp extra(%{url: url} = map) do
[encode("#{map.id}", map.title, :extras, url)]
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I should have caught it the first time around, but I am thinking this likely should not show up on search? I.e. we should skip URLs here too.

Copy link
Member

Choose a reason for hiding this comment

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

And let's add a test for whatever decision we pick here. :)

end

defp extra(map) do
{intro, sections} = extract_sections_from_markdown(map.source)

intro_json_item =
encode(
"#{map.id}.html",
map.title,
:extras,
intro
)

section_json_items =
for {header, body} <- sections do
encode(
"#{map.id}.html##{Utils.text_to_id(header)}",
header <> " - #{map.title}",
:extras,
body
)
end

[intro_json_item | section_json_items]
end

defp module(%ExDoc.ModuleNode{} = node) do
{intro, sections} = extract_sections(node.doc_format, node)

Expand Down
19 changes: 9 additions & 10 deletions lib/ex_doc/formatter/html/templates.ex
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,9 @@ defmodule ExDoc.Formatter.HTML.Templates do

defp sidebar_extras(extras) do
for extra <- extras do
%{id: id, title: title, group: group, content: content} = extra
%{id: id, title: title, group: group} = extra

item =
%{
id: to_string(id),
title: to_string(title),
group: to_string(group),
headers: extract_headers(content)
}
item = %{id: to_string(id), title: to_string(title), group: to_string(group)}

case extra do
%{search_data: search_data} when is_list(search_data) ->
Expand All @@ -79,10 +73,15 @@ defmodule ExDoc.Formatter.HTML.Templates do
}
end)

Map.put(item, :searchData, search_data)
item
|> Map.put(:headers, extract_headers(extra.content))
|> Map.put(:searchData, search_data)

%{url: url} when is_binary(url) ->
Map.put(item, :url, url)

_ ->
item
Map.put(item, :headers, extract_headers(extra.content))
end
end
end
Expand Down
12 changes: 7 additions & 5 deletions lib/mix/tasks/docs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,12 @@ defmodule Mix.Tasks.Docs do
Markdown and plain text pages; default: "PAGES". Example: "GUIDES"

* `:extras` - List of paths to additional Markdown (`.md` extension), Live Markdown
(`.livemd` extension), Cheatsheets (`.cheatmd` extension) and plain text pages to
add to the documentation. You can also specify keyword pairs to customize the
generated filename, title and source file, and search content of each extra page; default: `[]`. Example:
`["README.md", "LICENSE", "CONTRIBUTING.md": [filename: "contributing", title: "Contributing", source: "CONTRIBUTING.mdx"]]`
See the Customizing Extras section for more.
(`.livemd` extension), Cheatsheets (`.cheatmd` extension), external urls (`:url` option),
and plain text pages to add to the documentation. You can also specify keyword pairs to
customize the generated filename, title and source file, and search content of each extra page;
default: `[]`. Example: `["README.md", "LICENSE", "CONTRIBUTING.md": [filename: "contributing",
title: "Contributing", source: "CONTRIBUTING.mdx"]]` See the Customizing Extras section for
more.

* `:favicon` - Path to a favicon image file for the project. Must be PNG, JPEG or SVG. When
specified, the image file will be placed in the output "assets" directory, named
Expand Down Expand Up @@ -378,6 +379,7 @@ defmodule Mix.Tasks.Docs do
title but keep the source file unchanged.
* `:search_data` - A list of terms to be indexed for autocomplete and search. If not provided, the content
of the extra page will be indexed for search. See the section below for more.
* `:url` - An external url to link to from the sidebar.
Copy link
Member

Choose a reason for hiding this comment

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

I am thinking we could break it apart like this:

## Customizing Extras

There are two sources for extras, filenames and urls.

For filenames, the allowed configuration is:

  * ...

For urls:

  * title
  * url


### Customizing Search Data

Expand Down
11 changes: 11 additions & 0 deletions test/ex_doc/formatter/epub_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,17 @@ defmodule ExDoc.Formatter.EPUBTest do
assert content =~ ~r{<li><a href="readme.xhtml">README</a></li>}
end

test "ignores any external url extras", %{tmp_dir: tmp_dir} = context do
config =
context
|> doc_config()
|> Keyword.put(:extras, [elixir: [url: "https://elixir-lang.org"]])

generate_docs_and_unzip(context, config)

refute File.exists?(tmp_dir <> "/epub/OEBPS/elixir.xhtml")
end

test "uses samp as highlight tag for markdown", %{tmp_dir: tmp_dir} = context do
generate_docs_and_unzip(context, doc_config(context))

Expand Down
18 changes: 18 additions & 0 deletions test/ex_doc/formatter/html_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,24 @@ defmodule ExDoc.Formatter.HTMLTest do
assert LazyHTML.text(bar_content["h1"]) == "README bar"
end

test "extras defined as external urls", %{tmp_dir: tmp_dir} = context do
config =
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add a test that shows grouping work. Imagine you want all URLs in a section in the sidebar called Important Links. The Template.sidebar_extras kinda implies URLs work with groups but we need to be sure it works "end to end".

Copy link
Member

Choose a reason for hiding this comment

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

It seems groups expect the filename but there is none here, so we probably have to pass the filename or the url (and update its docs accordingly).

doc_config(context,
extras: [
"README.md",
"Elixir": [url: "https://elixir-lang.org"]
]
)

File.write!("#{tmp_dir}/README.md", "README")

generate_docs(config)

content = File.read!(tmp_dir <> "/html/README.html")

assert content =~ "https://elixir-lang.org"
end

test "warns when generating an index.html file with an invalid redirect",
%{tmp_dir: tmp_dir} = context do
output =
Expand Down