-
Notifications
You must be signed in to change notification settings - Fork 351
feat: add Markdown formatter #2148
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Yordis Prieto <[email protected]>
@@ -0,0 +1,133 @@ | |||
defmodule ExDoc.Formatter.MARKDOWNTest do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defmodule ExDoc.Formatter.MARKDOWNTest do | |
defmodule ExDoc.Formatter.MarkdownTest do |
I feel markdown should be Markdown
thou, no?
📦 Docs artifacts are ready: https://github.com/elixir-lang/ex_doc/actions/runs/18206396187/artifacts/4169817236 |
@josevalim any thoughts here thus far? |
Sorry, a bit busy with Elixir v1.19-rc and today I was out of focus. It is in my inbox and I will review it as soon as I can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @yordis that's amazing! Let me share some ideas that could be useful.
I was inspecting the generated Markdown files and thinking if we could apply some changes. See the differences here https://gist.github.com/leandrocp/ee4f0ba8325b410b8650ccd26b9b2351 (CompiledWithDocs.md vs CompiledWithDocs_PROPOSAL.md)
- Use frontmatter block to describe global values/notes
- Use the format "Summary / Functions" similar to HTML pages (so Functions become a level 2 heading)
- Include source links
- Reorganize metadata, for eg: add (deprecated) and doc in summary
- Remove links to hexdocs.pm because 1) it breaks the content and I guess that would make it harder for LLMs to parse; 2) I'm not sure it should link to html pages
You can see some examples on https://shopify.dev/docs/api/liquid/basics.md and https://vercel.com/docs/rest-api/reference/sdk.md
def to_markdown_string({:code, _attrs, inner, _meta} = ast, fun) do | ||
result = """ | ||
``` | ||
#{inner} | ||
``` | ||
""" |
There was a problem hiding this comment.
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 the lang here:
def to_markdown_string({:code, _attrs, inner, _meta} = ast, fun) do | |
result = """ | |
``` | |
#{inner} | |
``` | |
""" | |
def to_markdown_string({:code, attrs, inner, _meta} = ast, fun) do | |
class = attrs[:class] || "" | |
result = """ | |
```#{class} | |
#{inner} | |
``` | |
""" |
EEx.function_from_file( | ||
:def, | ||
:nav_template, | ||
Path.expand("templates/nav_template.eex", __DIR__), | ||
[:config, :nodes], | ||
trim: true | ||
) | ||
|
||
EEx.function_from_file( | ||
:defp, | ||
:nav_item_template, | ||
Path.expand("templates/nav_item_template.eex", __DIR__), | ||
[:name, :nodes], | ||
trim: true | ||
) | ||
|
||
EEx.function_from_file( | ||
:defp, | ||
:nav_grouped_item_template, | ||
Path.expand("templates/nav_grouped_item_template.eex", __DIR__), | ||
[:nodes], | ||
trim: true | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me suggest using using ~MD as an alternative :)
Since ~MD
does compile EEx strings, you could replace all template files and these functions with just:
import MDEx.Sigil
import ExDoc.Utils, only: [before_closing_body_tag: 2, h: 1, text_to_id: 1]
defp nav_template(assigns) do
~MD"""
# <%= @config.project %> v<%= @config.version %> - Documentation - Table of contents
<%= nav_grouped_item_template %{nodes: @config.extras} %>
<%= unless Enum.empty?(@nodes.modules) do %>
## Modules
<%= nav_grouped_item_template %{nodes: @nodes.modules} %>
<% end %>
<%= nav_item_template %{name: "Mix Tasks", nodes: @nodes.tasks} %>
<%= before_closing_body_tag(@config, :markdown) %>
"""MD
end
defp nav_grouped_item_template(assigns) do
~MD"""
<%= for {title, nodes} <- @nodes do %>
<%= if title do %>
- <%=h to_string(title) %>
<% end %>
<%= for node <- nodes do %>
- [<%=h node.title %>](<%= URI.encode node.id %>.md)
<% end %>
<% end %>
"""MD
end
defp nav_item_template(assigns) do
~MD"""
<%= unless Enum.empty?(@nodes) do %>
- <%= @name %>
<%= for node <- @nodes do %>
- [<%=h node.title %>](<%= URI.encode node.id %>.md)
<% end %>
<% end %>
"""MD
end
# ... rest
Note that MDEx can do more than just templating so it could be useful for manipulating documents too as for eg https://github.com/elixir-lang/ex_doc/pull/1992/files#diff-ca88e8bb57ba3af8237d2b7edb130ca72744fe6e0772bcd73a28690e958e6cc5R148-R169
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try our best to avoid adding dependencies to ExDoc because they may impose further constraints on users of ExDoc, given ExDoc is used by almost every library. Furthermore, if we use MDEx as a dependency, then you need to write some doc publishing helpers in MDEx, cause you will no longer be able to depend on ExDoc. :D So it is best to postpone this as much as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try our best to avoid adding dependencies to ExDoc because they may impose further constraints on users of ExDoc, given ExDoc is used by almost every library. Furthermore, if we use MDEx as a dependency, then you need to write some doc publishing helpers in MDEx, cause you will no longer be able to depend on ExDoc. :D So it is best to postpone this as much as possible.
Makes sense :D
Signed-off-by: Yordis Prieto [email protected]