From c045169edc4e482e1778389706cd1274001a72b8 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 08:42:49 -0400 Subject: [PATCH 01/11] improvement: add `redirects` config, and documentation explaining it --- README.md | 29 ++++++++++++++++++++++++++ lib/ex_doc/config.ex | 2 ++ lib/ex_doc/formatter/html.ex | 40 ++++++++++++++++++++++++++++-------- mix.exs | 10 +++++++++ 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ad7093234..4e8140063 100644 --- a/README.md +++ b/README.md @@ -448,6 +448,35 @@ Similarly to the example above, if your Markdown includes Mermaid graph specific For more details and configuration options, see the [Mermaid usage docs](https://mermaid-js.github.io/mermaid/#/usage). +## Changing documentation over time + +As your project grows, your documentation may very likely change, even structurally. There are a few important things to consider in this regard: + +- Links to your *extras* will break if you change or move file names. +- Links to your *modules, and mix tasks* will change if you change your code. +- Links to *functions* are actually links to modules with anchor links. If you change the function name, the link does + not break but will leave users at the top of the module's documentation. + +Because these docs are static files, when a user gets to a page that is not found, they will see a generic 404 page. +They will not be redirected to your packages home page. This can potentially be jarring for users. + +With this in mind, it is a good idea to preserve links to your old documentation. We do this with the `redirects` configuration. +This can solve for everything but the function names changing. + +For this example, we've changed the module `MyApp.MyModule` to `MyApp.My.Module`, and the extra `get-started.md` to `quickstart.md` + +```elixir +defp docs do + [ + ..., + redirects: %{ + MyApp.MyModule => MyApp.My.Module, + "get-started" => "quickstart" + } + ] +end +``` + ## Contributing The easiest way to test changes to ExDoc is to locally rebuild the app and its own documentation: diff --git a/lib/ex_doc/config.ex b/lib/ex_doc/config.ex index b20debced..8352b0752 100644 --- a/lib/ex_doc/config.ex +++ b/lib/ex_doc/config.ex @@ -39,6 +39,7 @@ defmodule ExDoc.Config do package: nil, proglang: :elixir, project: nil, + redirects: %{}, retriever: ExDoc.Retriever, skip_undefined_reference_warnings_on: &__MODULE__.skip_undefined_reference_warnings_on/1, @@ -78,6 +79,7 @@ defmodule ExDoc.Config do output: nil | Path.t(), package: :atom | nil, project: nil | String.t(), + redirects: %{optional(String.t()) => String.t()}, retriever: atom(), skip_undefined_reference_warnings_on: (String.t() -> boolean), skip_code_autolink_to: (String.t() -> boolean), diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index bbc0226b0..8bb460f1e 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -47,7 +47,8 @@ defmodule ExDoc.Formatter.HTML do generate_search(nodes_map, config) ++ generate_not_found(nodes_map, config) ++ generate_list(nodes_map.modules, nodes_map, config) ++ - generate_list(nodes_map.tasks, nodes_map, config) ++ generate_index(config) + generate_list(nodes_map.tasks, nodes_map, config) ++ + generate_redirects(config, ".html") generate_build(Enum.sort(all_files), build) config.output |> Path.join("index.html") |> Path.relative_to_cwd() @@ -187,13 +188,6 @@ defmodule ExDoc.Formatter.HTML do File.write!(build, entries) end - defp generate_index(config) do - index_file = "index.html" - main_file = "#{config.main}.html" - generate_redirect(index_file, config, main_file) - [index_file] - end - defp generate_not_found(nodes_map, config) do filename = "404.html" config = set_canonical_url(config, filename) @@ -390,6 +384,36 @@ defmodule ExDoc.Formatter.HTML do |> Enum.sort_by(fn extra -> GroupMatcher.group_index(groups, extra.group) end) end + def generate_redirects(config, ext) do + config.redirects + |> Map.put_new("index", config.main) + |> Enum.map(fn {from, to} -> + source = stringify_redirect_item(from) <> ext + destination = stringify_redirect_item(to) <> ext + generate_redirect(source, config, destination) + + source + end) + end + + defp stringify_redirect_item(item) when is_binary(item) do + item + end + + defp stringify_redirect_item(item) when is_atom(item) do + inspected = inspect(item) + + case to_string(item) do + "Elixir." <> ^inspected -> inspected + other -> other + end + end + + defp stringify_redirect_item(item) do + raise ArgumentError, + "Redirect source and destination must be a string or an atom, got: #{inspect(item)}" + end + defp disambiguate_id(extra, discriminator) do Map.put(extra, :id, "#{extra.id}-#{discriminator}") end diff --git a/mix.exs b/mix.exs index 6b9478f7c..bae56277c 100644 --- a/mix.exs +++ b/mix.exs @@ -87,6 +87,7 @@ defmodule ExDoc.Mixfile do "Cheatsheet.cheatmd", "CHANGELOG.md" ] ++ test_dev_examples(Mix.env()), + redirects: redirects(Mix.env()), source_ref: "v#{@version}", source_url: @source_url, groups_for_modules: [ @@ -107,6 +108,15 @@ defmodule ExDoc.Mixfile do defp test_dev_examples(:dev), do: Path.wildcard("test/examples/*") defp test_dev_examples(_), do: [] + defp redirects(:dev) do + %{ + "old-admonition" => "admonition", + Exdoc.OldMarkdown => ExDoc.Markdown + } + end + + defp redirects(_), do: %{} + defp clean_test_fixtures(_args) do File.rm_rf("test/tmp") end From 8713a5b18663e77a002038e8969af237c2890321 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 08:47:32 -0400 Subject: [PATCH 02/11] chore: update docs to be more clear --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4e8140063..8ed8f5430 100644 --- a/README.md +++ b/README.md @@ -453,7 +453,7 @@ For more details and configuration options, see the [Mermaid usage docs](https:/ As your project grows, your documentation may very likely change, even structurally. There are a few important things to consider in this regard: - Links to your *extras* will break if you change or move file names. -- Links to your *modules, and mix tasks* will change if you change your code. +- Links to your *modules, and mix tasks* will change if you change their name. - Links to *functions* are actually links to modules with anchor links. If you change the function name, the link does not break but will leave users at the top of the module's documentation. From 64ad8f2fdfc86ccb35990caef165154412453e82 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 08:48:21 -0400 Subject: [PATCH 03/11] chore: grammar --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8ed8f5430..a1d97413d 100644 --- a/README.md +++ b/README.md @@ -458,7 +458,7 @@ As your project grows, your documentation may very likely change, even structura not break but will leave users at the top of the module's documentation. Because these docs are static files, when a user gets to a page that is not found, they will see a generic 404 page. -They will not be redirected to your packages home page. This can potentially be jarring for users. +They will not be redirected to your package's home page. This can potentially be jarring for users. With this in mind, it is a good idea to preserve links to your old documentation. We do this with the `redirects` configuration. This can solve for everything but the function names changing. From 3917268a5991e8cb99a128c1e374b506b90f73d7 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 12:10:06 -0400 Subject: [PATCH 04/11] Update README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a1d97413d..9765360d3 100644 --- a/README.md +++ b/README.md @@ -457,8 +457,8 @@ As your project grows, your documentation may very likely change, even structura - Links to *functions* are actually links to modules with anchor links. If you change the function name, the link does not break but will leave users at the top of the module's documentation. -Because these docs are static files, when a user gets to a page that is not found, they will see a generic 404 page. -They will not be redirected to your package's home page. This can potentially be jarring for users. +Because these docs are static files, the behavior of a missing page will depend on where they are hosted. +In particular, [hexdocs.pm](https://hexdocs.pm) will show a 404 page. With this in mind, it is a good idea to preserve links to your old documentation. We do this with the `redirects` configuration. This can solve for everything but the function names changing. From 897b7b69baced54675395956bd6f088e57f2a7f4 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 12:11:02 -0400 Subject: [PATCH 05/11] Update README.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 9765360d3..f502b6ee5 100644 --- a/README.md +++ b/README.md @@ -460,10 +460,10 @@ As your project grows, your documentation may very likely change, even structura Because these docs are static files, the behavior of a missing page will depend on where they are hosted. In particular, [hexdocs.pm](https://hexdocs.pm) will show a 404 page. -With this in mind, it is a good idea to preserve links to your old documentation. We do this with the `redirects` configuration. -This can solve for everything but the function names changing. - -For this example, we've changed the module `MyApp.MyModule` to `MyApp.My.Module`, and the extra `get-started.md` to `quickstart.md` +You can improve the developer experience on everything but function names changing +by using the `redirects` configuration. For example, if you changed the module `MyApp.MyModule` +to `MyApp.My.Module` and the extra `get-started.md` to `quickstart.md`, you can +setup the following redirects: ```elixir defp docs do From 09ba90a7cc17040f2c022811199e43b252c6c88d Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 12:12:13 -0400 Subject: [PATCH 06/11] Update lib/ex_doc/formatter/html.ex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/ex_doc/formatter/html.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index 8bb460f1e..11e301931 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -411,7 +411,7 @@ defmodule ExDoc.Formatter.HTML do defp stringify_redirect_item(item) do raise ArgumentError, - "Redirect source and destination must be a string or an atom, got: #{inspect(item)}" + "redirect source and destination must be a string or an atom, got: #{inspect(item)}" end defp disambiguate_id(extra, discriminator) do From 83551f84a7f2b1be689cfdd24b61edc9a60cfc8b Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 08:48:21 -0400 Subject: [PATCH 07/11] PR Feedback --- README.md | 31 ++++++++++++++++++++--------- lib/ex_doc/config.ex | 2 +- lib/ex_doc/formatter/html.ex | 24 ++++------------------ mix.exs | 10 ---------- test/ex_doc/formatter/html_test.exs | 21 +++++++++++++++++++ 5 files changed, 48 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index f502b6ee5..db0306ecf 100644 --- a/README.md +++ b/README.md @@ -464,19 +464,32 @@ You can improve the developer experience on everything but function names changi by using the `redirects` configuration. For example, if you changed the module `MyApp.MyModule` to `MyApp.My.Module` and the extra `get-started.md` to `quickstart.md`, you can setup the following redirects: + + +### Elixir + +For this example, we've changed the module `MyApp.MyModule` to `MyApp.My.Module`, and the extra `get-started.md` to `quickstart.md` ```elixir -defp docs do - [ - ..., - redirects: %{ - MyApp.MyModule => MyApp.My.Module, - "get-started" => "quickstart" - } - ] -end +redirects: %{ + "MyApp.MyModule" => "MyApp.My.Module", + "get-started" => "quickstart" +} ``` +### Erlang + +For this example, we've changed the module `:my_module` to `:my_module2`, and the extra `get-started.md` to `quickstart.md` + +```erlang +{redirects, [ + {"my_module", "my_module2"}, + {"get-started", "quickstart"} +]}. +``` + + + ## Contributing The easiest way to test changes to ExDoc is to locally rebuild the app and its own documentation: diff --git a/lib/ex_doc/config.ex b/lib/ex_doc/config.ex index 8352b0752..1646d8b38 100644 --- a/lib/ex_doc/config.ex +++ b/lib/ex_doc/config.ex @@ -79,7 +79,7 @@ defmodule ExDoc.Config do output: nil | Path.t(), package: :atom | nil, project: nil | String.t(), - redirects: %{optional(String.t()) => String.t()}, + redirects: %{optional(String.t()) => String.t()} | [{String.t(), String.t()}], retriever: atom(), skip_undefined_reference_warnings_on: (String.t() -> boolean), skip_code_autolink_to: (String.t() -> boolean), diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index 11e301931..32f33dc0d 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -388,32 +388,16 @@ defmodule ExDoc.Formatter.HTML do config.redirects |> Map.put_new("index", config.main) |> Enum.map(fn {from, to} -> - source = stringify_redirect_item(from) <> ext - destination = stringify_redirect_item(to) <> ext + unless is_binary(from), do: raise "expected a string for the source of a redirect, got: #{inspect(from)}" + unless is_binary(to), do: raise "expected a string for the destination of a redirect, got: #{inspect(to)}" + source = from <> ext + destination = to <> ext generate_redirect(source, config, destination) source end) end - defp stringify_redirect_item(item) when is_binary(item) do - item - end - - defp stringify_redirect_item(item) when is_atom(item) do - inspected = inspect(item) - - case to_string(item) do - "Elixir." <> ^inspected -> inspected - other -> other - end - end - - defp stringify_redirect_item(item) do - raise ArgumentError, - "redirect source and destination must be a string or an atom, got: #{inspect(item)}" - end - defp disambiguate_id(extra, discriminator) do Map.put(extra, :id, "#{extra.id}-#{discriminator}") end diff --git a/mix.exs b/mix.exs index bae56277c..6b9478f7c 100644 --- a/mix.exs +++ b/mix.exs @@ -87,7 +87,6 @@ defmodule ExDoc.Mixfile do "Cheatsheet.cheatmd", "CHANGELOG.md" ] ++ test_dev_examples(Mix.env()), - redirects: redirects(Mix.env()), source_ref: "v#{@version}", source_url: @source_url, groups_for_modules: [ @@ -108,15 +107,6 @@ defmodule ExDoc.Mixfile do defp test_dev_examples(:dev), do: Path.wildcard("test/examples/*") defp test_dev_examples(_), do: [] - defp redirects(:dev) do - %{ - "old-admonition" => "admonition", - Exdoc.OldMarkdown => ExDoc.Markdown - } - end - - defp redirects(_), do: %{} - defp clean_test_fixtures(_args) do File.rm_rf("test/tmp") end diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index 97396c59a..ba2761a14 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -326,6 +326,27 @@ defmodule ExDoc.Formatter.HTMLTest do end end + describe "generates redirects" do + test "redirects are generated based on the configuration", %{tmp_dir: tmp_dir} = context do + generate_docs(doc_config(context, extras: ["test/fixtures/LICENSE"], redirects: %{ + "/old-license" => "license" + })) + + assert File.read!(tmp_dir <> "/html/old-license.html") == """ + + + + + Elixir v1.0.1 — Documentation + + + + + + """ + end + end + describe "generates extras" do @extras [ "test/fixtures/LICENSE", From 000c028efed5a68e76867852597f6520d479b8e8 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 12:30:30 -0400 Subject: [PATCH 08/11] chore: format --- lib/ex_doc/formatter/html.ex | 8 +++++-- test/ex_doc/formatter/html_test.exs | 33 +++++++++++++++++------------ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index 32f33dc0d..9e9db5e70 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -388,8 +388,12 @@ defmodule ExDoc.Formatter.HTML do config.redirects |> Map.put_new("index", config.main) |> Enum.map(fn {from, to} -> - unless is_binary(from), do: raise "expected a string for the source of a redirect, got: #{inspect(from)}" - unless is_binary(to), do: raise "expected a string for the destination of a redirect, got: #{inspect(to)}" + unless is_binary(from), + do: raise("expected a string for the source of a redirect, got: #{inspect(from)}") + + unless is_binary(to), + do: raise("expected a string for the destination of a redirect, got: #{inspect(to)}") + source = from <> ext destination = to <> ext generate_redirect(source, config, destination) diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index ba2761a14..2a7ab4b50 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -328,22 +328,27 @@ defmodule ExDoc.Formatter.HTMLTest do describe "generates redirects" do test "redirects are generated based on the configuration", %{tmp_dir: tmp_dir} = context do - generate_docs(doc_config(context, extras: ["test/fixtures/LICENSE"], redirects: %{ - "/old-license" => "license" - })) + generate_docs( + doc_config(context, + extras: ["test/fixtures/LICENSE"], + redirects: %{ + "/old-license" => "license" + } + ) + ) assert File.read!(tmp_dir <> "/html/old-license.html") == """ - - - - - Elixir v1.0.1 — Documentation - - - - - - """ + + + + + Elixir v1.0.1 — Documentation + + + + + + """ end end From a5dc8963da783609807b8566b30a7d5542bbe1f0 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 14:01:28 -0400 Subject: [PATCH 09/11] fix: handle lists in redirects properly, add corresponding test --- lib/ex_doc/formatter/html.ex | 1 + test/ex_doc/formatter/html_test.exs | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/ex_doc/formatter/html.ex b/lib/ex_doc/formatter/html.ex index 9e9db5e70..c4fcb818d 100644 --- a/lib/ex_doc/formatter/html.ex +++ b/lib/ex_doc/formatter/html.ex @@ -386,6 +386,7 @@ defmodule ExDoc.Formatter.HTML do def generate_redirects(config, ext) do config.redirects + |> Map.new() |> Map.put_new("index", config.main) |> Enum.map(fn {from, to} -> unless is_binary(from), diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index 2a7ab4b50..5ceb009e1 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -350,6 +350,30 @@ defmodule ExDoc.Formatter.HTMLTest do """ end + + test "redirects accept a list", %{tmp_dir: tmp_dir} = context do + generate_docs( + doc_config(context, + extras: ["test/fixtures/LICENSE"], + redirects: [ + {"/old-license", "license"} + ] + ) + ) + + assert File.read!(tmp_dir <> "/html/old-license.html") == """ + + + + + Elixir v1.0.1 — Documentation + + + + + + """ + end end describe "generates extras" do From 6fec117f40faa0de1b444ed13d07c88fcd764a14 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 14:02:25 -0400 Subject: [PATCH 10/11] chore: add whitespace to readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index db0306ecf..9fe792a59 100644 --- a/README.md +++ b/README.md @@ -464,6 +464,7 @@ You can improve the developer experience on everything but function names changi by using the `redirects` configuration. For example, if you changed the module `MyApp.MyModule` to `MyApp.My.Module` and the extra `get-started.md` to `quickstart.md`, you can setup the following redirects: + ### Elixir From 2083915b0e7af996a74b3ca5a2bb489fe4000982 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Tue, 17 Sep 2024 14:04:27 -0400 Subject: [PATCH 11/11] chore: format --- test/ex_doc/formatter/html_test.exs | 42 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/ex_doc/formatter/html_test.exs b/test/ex_doc/formatter/html_test.exs index 5ceb009e1..b9a93416f 100644 --- a/test/ex_doc/formatter/html_test.exs +++ b/test/ex_doc/formatter/html_test.exs @@ -352,28 +352,28 @@ defmodule ExDoc.Formatter.HTMLTest do end test "redirects accept a list", %{tmp_dir: tmp_dir} = context do - generate_docs( - doc_config(context, - extras: ["test/fixtures/LICENSE"], - redirects: [ - {"/old-license", "license"} - ] - ) - ) + generate_docs( + doc_config(context, + extras: ["test/fixtures/LICENSE"], + redirects: [ + {"/old-license", "license"} + ] + ) + ) - assert File.read!(tmp_dir <> "/html/old-license.html") == """ - - - - - Elixir v1.0.1 — Documentation - - - - - - """ - end + assert File.read!(tmp_dir <> "/html/old-license.html") == """ + + + + + Elixir v1.0.1 — Documentation + + + + + + """ + end end describe "generates extras" do