Skip to content

Markdown formatter refactor#2181

Open
milmazz wants to merge 4 commits intomainfrom
milton/markdown-formatter-review
Open

Markdown formatter refactor#2181
milmazz wants to merge 4 commits intomainfrom
milton/markdown-formatter-review

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

github-actions bot commented Jan 11, 2026

@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. :)

@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.

@josevalim
Copy link
Member

@milmazz I am done with my refactor. Please rebase! The markdown module changed considerably but maybe I dropped a few things :)

@milmazz milmazz force-pushed the milton/markdown-formatter-review branch from 8d87416 to 6b97029 Compare February 7, 2026 17:45
@milmazz
Copy link
Member Author

milmazz commented Feb 7, 2026

@josevalim Hey, the new formatter is basically a whole revamp on the previous version :)

I just did some minor refactoring:

  • The optional description in the llms.txt should be a block quote according to the docs from: https://llmstxt.org
  • Group the resources just once and pass that as argument to private helpers
  • Use some pipelines to denote the sequence of data transformation
  • Use File.write!/2 for everything, as in the html formatter.

@milmazz milmazz requested a review from josevalim February 7, 2026 18:00
groups =
Map.new([modules: modules, mix_tasks: tasks, extras: extras], fn {k, v} ->
{k, group_by_group(v)}
end)
Copy link
Member

Choose a reason for hiding this comment

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

I decided explicitly to not go down this route so we can more clearly see what is passed to what and what depends on what.

generate_api_reference(config, modules, tasks) ++
generate_nav(config, groups) ++
generate_api_reference(config, groups) ++
generate_extras(extras, config) ++
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 make config the first argument for consistency?

@josevalim
Copy link
Member

One tiny comment and we can fix it!

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.

2 participants