Ex_doc: Deeply nested modules do not fit in sidebar

[teamon]

Google has recently released multiple libraries for their API.
They contain generated elixir structs for every protobuf model which leads to very deeply nested module names that do not fit in the sidebar. You can see example here https://hexdocs.pm/google_api_pub_sub/api-reference.html#modules

What do you think about (optionally) putting modules list in a tree?

(left - current state, right - my   in inspector attempt to present the idea)

[DarkMarmot]

I could also use this -- and the ability to toggle/hide groups would be an awesome addition.

[mhanberg]

I have also run into this using the Timex library.

My proposal would to make the sidebar re-sizable and/or having the sidebar automatically size to the width of the longest module name.

[tmbb]

On one hand, this solves the pragmatic issue of not enough space for module names in the sidebar.
On the other hand, this obscures the fact that module names are just atoms. There aren't nested modules in Elixir!

In any case, that's a problem that needs to be solved. I suggest something like this:

GoogleApi.PubSub.V1.{
  Api.Projects,
  Connection,
  Deserializer,
  Model.{
    AcknowledgeRequest,
    Binding,
    Empty
  }
}

It uses the syntax for aliasing modules, which is familiar, and although it takes up a little more vertical space, I think the resemblance to source code is worth it.

[OvermindDL1]

It uses the syntax for aliasing modules, which is familiar, and although it takes up a little more vertical space, I think the resemblance to source code is worth it.

Ooo, big +1 on this, I like this idea quite a bit!

[josevalim]

It uses the syntax for aliasing modules, which is familiar, and although it takes up a little more vertical space, I think the resemblance to source code is worth it.

It looks great in code but it is hard to do visually. Mockups are appreciated. :)

Another idea to leverage this with the current infrastructure is to use the groups feature. Here is a mockup:

We would need only two features for this to work: Introduce a :default_group and :strip_prefix options. Thoughts?

[tmbb]

Mockups are appreciated. :)

Something like this:

The default font for the curly brace and period doesn't work too well, but that's fixable.
A big fat period would work better, I guess, as well as a wider curly brace.

[tmbb]

Another idea to leverage this with the current infrastructure is to use the groups feature.

I don't like this very much because I'd like group names to be more descriptive than just a module name.

[josevalim]

@tmbb i like your mockup, in case somebody wants to give it a try, although I think it needs to be opt-in per nesting.

[tmbb]

I propose changing this:

def groups_for_modules() do
  [
    [
      "Endpoint And Plugs": [
        Phoenix.CodeReloader,
        Phoenix.Endpoint,
        Phoenix.Endpoint.CowboyAdapter,
        Phoenix.Endpoint.Cowboy2Adapter,
        Phoenix.Logger,
      ],
  ]
end

into this (Phoenix.Endpoint is clickable, while Phoenix.{ isn't; the only way to know is by seeing the cursor change on mouseover):

def groups_for_modules() do
  [
    [
      "Endpoint And Plugs": """
      Phoenix.{
        CodeReloader,
        Endpoint,
          Endpoint{
          CowboyAdapter,
          Cowboy2Adapter
        },
        Logger
      }
      AnotherModule
      YetAnotherModule
      """
  ]
end

or this (More explicit; Phoenix.Endpoint must be in a separate line so that the user knows it's clickable):

def groups_for_modules() do
  [
    [
      "Endpoint And Plugs": """
      Phoenix.{
        CodeReloader,
        Endpoint,
        Endpoint{
          CowboyAdapter,
          Cowboy2Adapter
        },
        Logger
      }
     AnotherModule
     YetAnotherModule
      """
  ]
end

Some advantages:

1. It's backwards-compatible (if the argument is a string, do this; if it's a list do the old thing)
2. It's exactly the same syntax as defining a module alias
3. It's a context-free language, which can be parsed by NimbleParsec, on which Exdoc already depends because of the new syntax highlighter (if you don't want to get fancy, you can use Code.string_to_quoted/2 to parse it; it will generate atoms dynamically, but this is a build-time feature anyway)
4. It's as opt in as it gets

[josevalim]

@tmbb we definitely should not use strings for configuration. It means all of the rich mechanisms we have to traverse and handle data structures will be lost. Imagine for example if your whole def project was a string and you had to do everything as string interpolation/composition. It is not an example we should set and we should definitely avoid as it is very unnatural in Elixir. The reason why we have macros in the first place is to not rely on strings for meta-programming.

To me the configuration is the simplest of all of this: we will have a key such as alias_sidebar: [Google.PubSub.V1] and that will "alias" everything that matches Google.PubSub.V1.

Furthermore, I believe we should handle this completely orthogonal to groups.

[tmbb]

Ok, use a data structure instead, or a macro that consumes Elixir syntax instead. The main idea is that you're explicit in specifying how the nesting happens by writing the layout you want for the table of contents.

You might want to split a module in some places but not others. This means that nesting should't be orthogonal to groups.

By making nesting independent of grouping, you're in fact imposing a rigid structure where there is no need for one...

The problem is mainly a visual one (lack of space), so the user should be free to manually manipulate the level of nesting in each case.

In one group you might want:

Module.A.B.C1
Module.A.B.C2

And in another:

Module.{
  A.{
    B.{
      VeryLongModuleNameC1,
      VeryLongModuleNameC2,
      VeryLongModuleNameC3
    }
  }
}

My general approach (mimicking the structure of the table of contents) is strictly more flexible and in most cases probably less verbose.

Using a string was merely an implementation detail (it was the shortest DSL I came up in 5mins but I admit there might be better approaches).

[josevalim]

Folks, I am pretty sure we can make the most complex mechanism for handling nesting. But for cases like google pubsub, we only need a simple nesting based on the prefix. That's what we should focus for now. As other use cases appear, we can adept accordingly.

[josevalim]

My suggestion is to have a configuration called nest_module_aliases: [Foo.Bar, Bar.BazBat] and if a module matches any of the prefix in the list, it will be nested under the first matching prefix. Note that they won't cascade.

In terms of layout, we have two options, use the nesting that @teamon proposed, or use the existing groups_for_modules layout structure.

[davidsulc]

At José's suggestion in https://elixirforum.com/t/exdoc-long-module-names-in-sidebar-getting-clipped-what-to-do/17288, I've opened https://github.com/elixir-lang/ex_doc/pull/906 for preliminary feedback. If I'm on the right track, I'll add docs and tests.

[josevalim]

@teamon, @tmbb, @DarkMarmot, @OvermindDL1, @mhanberg, your feedback is deeply appreciated. :heart:

[fertapric]

Here are a few ideas:

1. Include the :as option (as suggested by @davidsulc), but in the :groups_for_modules. That would allow a lot of flexibility (which might not be what we want).

   Here is an example for Google:

   def groups_for_modules() do
     [
       [
         "GoogleAPI.PubSub.V1": [
           {GoogleAPI.PubSub.V1.Connection, as: "Connection"},
           {GoogleAPI.PubSub.V1.Deserializer, as: "Deserializer"},
           {GoogleAPI.PubSub.V1.Model.AcknowledgeRequest, as: "Model.AcknowledgeRequest"},
           ...
         ],
         ...
       ]
     ]
   end
   

2. Include an :alias option (similar to what @josevalim suggested), but per module group. The syntax may vary:

   def groups_for_modules() do
     [
       [
         "GoogleAPI.PubSub.V1": {
           [alias: GoogleAPI.PubSub.V1],
           [
             GoogleAPI.PubSub.V1.Connection,
             GoogleAPI.PubSub.V1.Deserializer,
             GoogleAPI.PubSub.V1.Model.AcknowledgeRequest,
             ...
           ]
         },
         ...
       ]
     ]
   end
   

   If the group contains a module that cannot be aliased, it would be left as is.

Thoughts?

[josevalim]

Oh, now I see what @teamon wanted in regards to the :as option. Honestly, I would like to be careful in regards to allowing people to change the module names in the sidebar. I think the entry there should always be the module name and we are right now opening only a single exception for nesting. I am definitely not comfortable with generalizing it and I would prefer to continue with a restrictive approach for now.

[tmbb]

I can't test it right now but I'll take a look as soon as possible

[davidsulc]

FYI https://github.com/elixir-lang/ex_doc/pull/906#issuecomment-430524961

[davidsulc]

I think this can be closed with https://github.com/elixir-lang/ex_doc/pull/907 having been merged.