Skip to content

Conversation

@milmazz
Copy link
Member

@milmazz milmazz commented Jan 11, 2026

I was curious about this new formatter in ex_doc and I did an initial review. Please let me know what you think.

@github-actions
Copy link

@josevalim
Copy link
Member

Hi @milmazz! There are some other PRs and I have also refactored some of this code locally, so let's please hold on before merging this, unless there is a bug fix. :)

|> Enum.dedup()
|> Enum.intersperse("\n")

File.mkdir_p!(Path.dirname(build))
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed because the config.output directory is always created in the output_setup private helper that is applied before this one.

Comment on lines +154 to +156
config.output
|> Path.join(filename)
|> File.write(content)
Copy link
Member Author

Choose a reason for hiding this comment

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

Please let me know if we should include a small private helper like:

defp write_content(config, filename, content) do
  config.output
  |> Path.join(filename)
  |> File.write(content)
end

Because this is repeated a few times in this module.

Comment on lines +172 to +175
Enum.map(nodes_map.modules, fn module_node ->
synopsis = synopsis(module_node.doc)

["- [#{module_node.title}](#{module_node.id}.md): ", synopsis, "\n"]
Copy link
Member Author

Choose a reason for hiding this comment

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

I think is more readable to work with lists in this case, and it should be a bit more performant for bigger projects.


tasks_info =
if length(nodes_map.tasks) > 0 do
if Enum.any?(nodes_map.tasks) do
Copy link
Member Author

Choose a reason for hiding this comment

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

We just care if there is at least one element in the list, we don't mind about the total number of entries.


extras_info =
if is_list(config.extras) and length(config.extras) > 0 do
if is_list(config.extras) and Enum.any?(config.extras) do
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here, we just care if there is at least one element in the list, we don't mind about the total number of entries.

end
end

defp extract_plain_text(_), do: "No documentation available"
Copy link
Member Author

Choose a reason for hiding this comment

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

ExDoc.DocAST.synopsis/1 always returns a binary, so, this block was unreachable.

@milmazz
Copy link
Member Author

milmazz commented Jan 11, 2026

Hi @milmazz! There are some other PRs and I have also refactored some of this code locally, so let's please hold on before merging this, unless there is a bug fix. :)

No worries, I'm so inactive in this project that I wasn't planning on merging code without any maintainer review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants