Ex_doc: Group option for modules

[LostKobrakai]

I've just tried the group option for markdown docs, like it's used for the new phoenix docs. I'd find it useful to have such a grouping possibility for modules as well. With phoenix contexts the number of modules does quickly become quite big and I'd like to document not only the public interface, but also how the internals work. Grouping the modules by the context name would solve that issue quite simply and it seems the necessary frontend stuff could be reused.

[josevalim]

I agree. Can you please submit a PR?

[LostKobrakai]

I could, but I'm not sure what would be the best way to determine the context group. I've thought of the following options:

1. Custom module attribute
   Would be tedious work to add to an already big codebase. Also I'm not sure if custom module attributes are really desirable in general.
2. Just take the second module namespace level
   Like MyApp.Namespace.Module => Namespace. But this would require MyApp.Namespace to be handled differently if there are deeper nested modules or not. Also this is quite the preconception about the users codebase.
3. Some kind of mapping in the mix.exs
   I'm not sure what would be the most effortless option to assign groups to modules, which does not break as soon as a developer is adding a new module to the project. Maybe something like this [MyApp.Namespace, MyApp.AnotherNamespace], which would group all the modules of that name and nested ones. Maybe with the additional boolean option of "include top level module in groups".

After writing this out I feel like 3. would probably be the best way.

[josevalim]

Yup, definitely 3. For pages those are specified alongside each page:

      "introduction/learning.md": [group: "Introduction"],
      "introduction/community.md": [group: "Introduction"],

      "up_and_running.md": [group: "Guides"],
      "adding_pages.md": [group: "Guides"],

But I am not sure this approach would work for modules exactly because of what you said. We can add a groups: option that accepts a list of regexes and module names:

groups: [
  "Super important": ~r/^MyApp.Foo.?/,
  "Another group": [MyApp.Bar, MyApp.Baz]
]

I am not sure I like the inconsistency with how pages work though but given their different contexts, I guess it is a reasonable choice to make?

/cc @Gazler @jeregrine @eksperimental @milmazz

[LostKobrakai]

Maybe naming the key module_groups separates it a bit more from groups of pages? The context is certainly different. I like the idea of a concrete list and using regex for implicit grouping of modules. Maybe also "Some group": [~r/^MyApp.Foo.?/, ~r/^MyAppWeb.Foo.?/], which should cover most cases from my pov.

[Gazler]

For consistency, perhaps we should consider deprecating the current way it is handled on pages in favor of:

module_groups: [
  "Super important": ~r/^MyApp.Foo.?/,
  "Another group": [MyApp.Bar, MyApp.Baz]
],
page_groups: [
  "Introduction": [~r/introduction\/?/]
  "Guides": ["up_and_running.md", "adding_pages.md"]
]

[josevalim]

@Gazler that sounds good to me. Could you please try Phoenix' mix.exs and let us know how it looks alike? I want to make sure the Phoenix team will be happy with the result since they are currently the primary users of this feature.

[Gazler]

@josevalim if such a feature existed, then we would do something like this:

https://github.com/phoenixframework/phoenix/compare/master...Gazler:guides/mix-changes?expand=1

We can't use a regex match though, as ordering is important. It's a pretty minor change, but the new data structure makes more sense in my opinion, it means renaming a category only requires one change instead of one per page. It also enforces grouping. Previously you could have:

"page1.md": [group: "foo"]
"page2.md": [group: "bar"]
#...
"page100.md": [group: "foo"]

[josevalim]

@Gazler cool, thank you. I think the version you sent is indeed better, so let's migrate to that direction as well.

[LostKobrakai]

Ordering within groups is also a good keyword, especially on how to handle regex matches vs explicit matches and combined versions.

In my current implementation it would be possible to have the following.

[
  "Group": [MyApp.SomeExplicitModule, ~r/MyApp.Grouped.?/]
]

Probably group explicit modules/pages by the order of definition and move all regex matches to the end and order them alphabetically?

[josevalim]

Oh, I see. I am not sure if we should order within the groups. In general, I am not sure if specifying the extras_groups should replace specifying the extra pages inside extras itself.

So you would probably specify the pages in extras and then the groups. Then at least you can use a regex to match on the groups or something like Path.wildcard or File.ls!.

[LostKobrakai]

So order pages by order in extras and use the groups setup only to match them together. And for modules simply keep alphabetical order within groups?

[josevalim]

@LostKobrakai that's what I would say, yeah. You should get this behaviour by sorting twice in both cases, first order them alphabetically (or as specified in extra) and then order based on the group. Elixir's sort is a stable sort.

[Gazler]

Here it is using both extras and groups https://github.com/phoenixframework/phoenix/compare/master...Gazler:guides/mix-changes-with-extras?expand=1

[LostKobrakai]

Ok, I'll look into the extras/pages related changes later today. I think the changes for grouping modules should be mostly done in the PR.

[LostKobrakai]

Also what about extra_groups and module_groups. page_groups feels disconnected in it's naming.

[josevalim]

Maybe it should be extras_groups and modules_groups? Is that valid english speaking wise?

[LostKobrakai]

I'm not a native speaker as well so thats for others to answer.

[josevalim]

Let's wait for @Gazler to validate it then. :)

[Gazler]

It's a little clumsy with the double plural. Maybe groups_for_extas and groups_for_modules?

[LostKobrakai]

I've added the changes for grouping extra pages to the PR. I'm not sure if or how I should handle the deprecation of the old option of applying groups to extras from the code side. Also I didn't update the config key naming changes by now. I'll leave that until we're settled on that one.

[josevalim]

Thanks @LostKobrakai! I will do a complete review soon but to answer your more immediate questions:

1. No need to worry about backwards compatibility. We are pre-1.0 and this feature is relatively new. I will take care of the CHANGELOG.

2. Let's go with groups_for_extras and groups_for_modules

[josevalim]

Thank you @LostKobrakai! I think this is a fantastic feature and I will see if I use it on Elixir itself.

@Gazler, can you also please give it a try on Phoenix? Once it works for both Phoenix and Elixir, I will release 0.17. Notice the old style is no longer supported as of master. I have pushed some documentation here: f5fd932f63576eeda85ba6ce7294d6dd7d7660fa

[josevalim]

I have reopened this to make sure we collect everyone's feedback before closing and releasing 0.17.

[josevalim]

The Elixir PR is here: https://github.com/elixir-lang/elixir/pull/6621

[Gazler]

@josevalim working as expected here on 0.17 - only difference is the "Protocols" page is missing, that's unrelated to this PR though.

[josevalim]

Yup, I removed protocols section. It gives the sidebar more space and it
often has only a module or two.

Protocols now show will all modules. A project can use the new group
feature if they want to keep them somewhat apart still.

--

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Director of R&D

[josevalim]

v017.0 is out. Thank you @LostKobrakai, this is a fantastic feature!