Conversation
|
📦 Docs artifacts are ready: https://github.com/elixir-lang/ex_doc/actions/runs/21786619987/artifacts/5418799463 |
|
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. |
|
@milmazz I am done with my refactor. Please rebase! The markdown module changed considerably but maybe I dropped a few things :) |
milmazz
force-pushed
the
milton/markdown-formatter-review
branch
from
8d87416 to
6b97029
Compare
|
@josevalim Hey, the new formatter is basically a whole revamp on the previous version :) I just did some minor refactoring:
|
lib/ex_doc/formatter/markdown.ex
Outdated
| groups = | ||
| Map.new([modules: modules, mix_tasks: tasks, extras: extras], fn {k, v} -> | ||
| {k, group_by_group(v)} | ||
| end) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok, seems fair the tiny extra compute for more readability. I will update.
lib/ex_doc/formatter/markdown.ex
Outdated
| generate_api_reference(config, modules, tasks) ++ | ||
| generate_nav(config, groups) ++ | ||
| generate_api_reference(config, groups) ++ | ||
| generate_extras(extras, config) ++ |
There was a problem hiding this comment.
Should we make config the first argument for consistency?
There was a problem hiding this comment.
Yes, I can make that more consistent.
|
One tiny comment and we can fix it! |
|
@josevalim There might be a chance that someone is only interested in the markdown formatter output, the current implementation of the markdown formatter doesn't copy the assets to the |
|
@milmazz yes, sounds like a good idea! |
2 participants
I was curious about this new formatter in
ex_docand I did an initial review. Please let me know what you think.