Nest module aliases

[davidsulc]

(Tests and documentation still need to be written, this is just for preliminary feedback.)

A basic implementation for #834

It leverages groups_for_modules to provide the usual UI presentation, and to leverage the current ordering ability. As suggested by José, a nest_module_aliases key was added to the docs config.

This new value is an array of alias configs, each of which is one of:

• ModuleName (equivalent to {ModuleName, as: ModuleName})
• {Super.Duper.Really.Long.ModuleName, as: Some.ModuleName}
• {Super.Duper.Really.Long.ModuleName, as: "My module"}

The as: values are used as group names (as if they had been provided to groups_for_modules) and are removed from the module's displayed title.

Hopefully, this will be familiar to language users as it combine the as: syntax of module aliases with the multiple value formats as can be used when specifying child configs in supervisors.

The objective for this first implementation is to provide enough flexibility while keeping the code changes simple. It is to be noted that the aliasing is very naive: the first match is kept, without attempting to find the longest prefix possible.

With these modifications, nesting could be specified like this (adapted from ExDoc's mix.exs):

      groups_for_modules: [
        "Markdown": [],
        "Formatter API": [
          ExDoc.Config,
          ExDoc.FunctionNode,
          ExDoc.ModuleNode,
          ExDoc.TypeNode
        ],
        "ExDoc.Formatter": []
      ],
      nest_module_aliases: [
        {ExDoc.Markdown, as: Markdown},
        #{ExDoc.Markdown, as: "Foo bar"},
        ExDoc.Formatter
      ]

Adding empty groups matching as: values (or the module name, if absent) in nest_modules_aliases allows controlling the ordering of the blocks of nested modules.

Here's the generated result:

[josevalim]

I think you can do config.nest_module_aliases. :)

[josevalim]

I think we should actually change this to only match modules that are namespaced. So I would change it to something like:

|> Enum.find_value(fn prefix ->
  case String.split(title, prefix) do
    ["", "." <> suffix] -> {prefix, suffix}
    _ -> nil
  end
end)

What do you think?

[davidsulc]

I agree, as I hadn't really considered this being used for a non-nested module.

You could argue that if a (non nested) module's name needs to be changed, it should probably just be renamed in the code base, but there are projects that are essentially Elixir libraries/wrappers for some other project/service/company that has a really long name. In those cases, the root module name (i.e. first section) is usually a variation of the project/service/company name and "ex", which will therefore also be long. So it's possible this decision will have to be revisited in the future, but my preference would be to keep the implementation/concept simple initially (i.e. limit the feature to nested modules), and enhancing it later as issues arise in the community at large.

This leads me to my next question: how helpful should the code error handling be with improper configuration? E.g. trying to alias non nested modules, giving improper values for the alias value, etc. Should it just crash (e.g. failing to find a matching function clause due to type mismatch) and assume the user will refer to the documentation to find out the proper configuration? Or should "catch all" clauses be added where appropriate to return some sort of nice error message?

[josevalim]

So it's possible this decision will have to be revisited in the future, but my preference would be to keep the implementation/concept simple initially (i.e. limit the feature to nested modules), and enhancing it later as issues arise in the community at large.

Exactly. I think it is simpler to start more conservative and expand as necessary. We have the luxury to afford this as we are still pre-1.0. So here are my suggestions:

1. Consider only really nested modules in the group. So if you nest Foo.Bar, Foo.Bar.Baz is part of the group but Foo.Bar isn't

2. As proposed by @whatyouhide, let's not support the :as option right now. The list given to nest_module_aliases is just a simple list of atoms.

I also wouldn't worry about error handling right now. Let's write assertive code, that expects things to be in the proper format, and we can collect feedback in case the API is confusing.

[josevalim]

Thanks @davidsulc! I really like this! I personally think you are in the correct direction! ❤️ I have added two comments.

[teamon]

It took me quite some time to figure out what has been nested and what not. In the screen there is:

# MARKDOWN

ExDoc.Markdown
Cmark
Earmark

which, correct me if I'm wrong is supposed to mean

# MARKDOWN

ExDoc.Markdown
ExDoc.Markdown.Cmark
ExDoc.Markdown.Earmark

This PR provides a great flexibility, yet I am not convinced this is the right approach as it can lead to confusion. If I were writing this doc, it would probably make sense to write it exactly as presented on the screen shot, but from the readers perspective it is not clear.

As the original issue started because of autogenerated library with multiple levels of nesting, I think it would be better to have either groups_for_modules OR just_make_a_nested_tree_out_of_all_modules: true.

I like the idea as as: X, maybe it could be added directly to groups_for_modules ?

[whatyouhide]

I have to say that by reading the rationale and intent of the PR, I am pretty confused by this feature. I don't really understand how to use it or why it's done this way. In #834, the last idea that @josevalim suggests (where you specify the prefix and if a module matches then it's nested) feels reasonable, but I am not sure adding :as makes things better.

[josevalim]

It took me quite some time to figure out what has been nested and what not. In the screen there is:

Right, this is confusing because the feature as currently implemented is mixing both nested and non-nested things, which is why I proposed to only group together nested ones.

As the original issue started because of autogenerated library with multiple levels of nesting, I think it would be better to have either groups_for_modules OR just_make_a_nested_tree_out_of_all_modules: true.

I would prefer to start with a simplest approach with one level hierarchy. I think a tree is just moving the problem around, as the tree is worst to navigate, and a deep tree will be a source of other issues.

I like the idea as as: X, maybe it could be added directly to groups_for_modules ?

The as: X is always required in groups_for_modules and it is the key in the keyword list given to the `:groups_for_modules .

[fertapric]

Right, this is confusing because the feature as currently implemented is mixing both nested and non-nested things, which is why I proposed to only group together nested ones.

It was confusing indeed. Grouping per alias sounds good.

Btw, what do you think about renaming :nest_module_aliases to :group_modules_by_aliases (or something that contains "group" instead of "nest"?

[davidsulc]

@whatyouhide What initially prompted my interest in this issue (https://elixirforum.com/t/exdoc-long-module-names-in-sidebar-getting-clipped-what-to-do/17288/10) is an API wrapper I'm working on (https://github.com/davidsulc/scrapy_cloud_ex). It's name is ScrapyCloudEx (for discoverability reasons as it wraps the "Scrapy Cloud" API provided by scrapinghub.com).

This API has its endpoints split into 2 groups, "app" and "storage", which affects (e.g.) parameter formats a specific endpoint expects (e.g. pagination params are provided differently to endpoints depend on whether they belong to the "app" or "storage" endpoint group).

The reason my initial implementation has an :as option was because it would have been nice to group nested modules under Endpoints.App and Endpoints.Storage group names as it nicely reflects the API organization and API docs (https://doc.scrapinghub.com/scrapy-cloud.html#api-endpoints).

Also, it means the group name wouldn't be ScrapyCloudEx.Endpoints.Storage which would at some point bring us back to "long module names get clipped in the sidebar". Of course, I could introduce modules such as ScrapyCloudEx.Storage that simply defdelegate to ScrapyCloudEx.Endpoints.Storage but I have to say I'm not in love with the idea of code organization being influenced by documentation concerns.

I understand @josevalim's concerns about allowing renaming modules (#834 (comment)), but that could be handled by e.g. checking the alias is a valid prefix of the aliased module.

Finally, I thought it would be nice to be able to group modules under a common name while omitting long common prefixes.

I hope this isn't coming across as argumentative, as I just wanted to shed some light on where I'm coming from and the reasoning behind my initial implementation. In effect, this issue was opened due to deeply nested modules, but my initial concern (https://elixirforum.com/t/exdoc-long-module-names-in-sidebar-getting-clipped-what-to-do/17288/10) was mainly about dealing with long module names.

[josevalim]

Btw, what do you think about renaming :nest_module_aliases to :group_modules_by_aliases?

I like using something that refers to :group_modules. Maybe group_modules_nested_at: [Foo.Bar]?

@davidsulc thanks for the clarification! 👍 Given there is some divergence, let's focus on the MVP and then iterate on that. :) And what do you think about the naming changes?

[davidsulc]

Ok, I will udpate the PR to use only a list of module names as an MVP implementation. Regarding the attribute name, I think group_modules_by_prefix: [Foo.Bar] would fit better. Thoughts?

A lot of newcomers have to learn that module names are just atoms without a hierarchy, so talking about nesting could be counter-productive.

[josevalim]

@davidsulc you originally wrote group_modules_by_nesting and I prefer that over by_prefix because in our case the prefix is not Foo.Bar but Foo.Bar.. So I find something that refers to nesting clearer. Does it make sense?

[davidsulc]

@josevalim Yeah, I edited to change the name when I remembered the "dots don't imply hierarchy" you discover when new to Elixir. But you're right that "foo" is technically a prefix to "foo" itself, so group_modules_by_prefix might not behave exactly as a user would expect. I will rename the attribute to group_modules_by_nesting in the update.

[OvermindDL1]

Yeah in the Markdown example, I would at the very least prefer something that makes it obvious that CMark and such are underneath the ExDoc.Markdown namespace, like using <li> tags inside an unordered item set to show that they are children of the prior one.

[josevalim]

There are also three options we can consider:

1. Impose a dot on the header:

EXDOC.MARKDOWN.
CMark
Earmark

2. Impose a dot on each entry:

EXDOC.MARKDOWN
.CMark
.Earmark

3. Keep it as is:

EXDOC.MARKDOWN
CMark
Earmark

I think 2 is the clearest but it can be considered too noisy? Thoughts?

[OvermindDL1]

2. Impose a dot on each entry:

This, for sure. I don't like the uppercasing at all as it doesn't match the actual module namespacing.

[josevalim]

I don't like the uppercasing at all as it doesn't match the actual module namespacing.

I agree the uppercasing is an issue and I think we can revisit the uppercasing altogether, but let's go with baby steps for now. :D

[tmbb]

It leverages groups_for_modules to provide the usual UI presentation, and to leverage the current ordering ability

I strongly disagree with reusing groups for module nesting. It's visually confusing and mixes two very different things in the dame UI. Module names should always be non-bold and group names should always be bold.

An alternative to nested modules would be to abreviate module names using the first letter of the module. For example: Very.Long.Module.Name.That.Doesnt.Fit could become: V.L.N.T.D.Fit. The long version could be shown on mouseover.

This is just one idea I've thought about now, but the main point is that i really dislike reusing the group system for this.

[davidsulc]

I like @tmbb's idea. Here's what it could look like for (part of) the mentioned lib:

Which would be obtained via a collapse_module_prefixes: [GoogleApi.PubSub.V1] option

The idea would be that (in some future version):

• prefixes common to all modules would get "collapsed" to the first letter of each section (e.g. GoogleApi.PubSub.V1 -> G.P.V. in the example)
• prefixes that are common only to a subset of modules will only be collapsed after being displayed in long form once (e.g. G.P.V.Model. get displayed before being subsequently shortened to G.P.V.M.)

I like this for the following reasons:

• it can address both deeply nested modules, and those with long names
• it doesn't require a lot to be put in the reader's "mental stack": only common prefixes, which usually aren't very significant from an information perspective, and the full module can always be obtained by hovering if necessary (and on the web view)
• determining the meaning of the shortened name for a given module only requires guiding the eye up the series of aligned dots until the full name is encountered (i.e. to determine what M is in G.P.V.M.UpdateSnapshotRequest, follow the M. upward visually until you find it's Model)
• it would also somewhat address @OvermindDL1's concern for actual module namespacing, although that's debatable

This would be driven by a collapse_module_prefixes: [atom] | :auto config option.

As a first implementation, only a list of module prefixes would be accepted, and modules with matching prefixes will be collapsed, except for the module itself (if it exists) which would collapse until the last section.

So given the option collapse_module_prefixes: [Foo.Bar.Baz] and modules Foo.Bar.Baz, Foo.Bar.Baz.Some.Module, and Foo.Bar.Baz.Some.Other.Module, the result in the sidebar would be:

F.B.Baz
F.B.B.Some.Module
F.B.B.Some.Other.Module

You'll note that Some in the 3rd line wasn't collapsed: this would be for a (possible) future version.

Later, depending on feedback, etc. we might introduce the :auto value for :collapse_module_prefixes which would automatically collapse prefixes so that module names will fit in the sidebar (and only if necessary).

@josevalim I think this is in line with your desire to have a straightforward initial implementation that could be enhanced later. Thoughts?

[josevalim]

@josevalim I think this is in line with your desire to have a straightforward initial implementation that could be enhanced later. Thoughts?

If people believe this is a good approach, then please go ahead. :) My only reservations are the same as before:

except for the module itself (if it exists) which would collapse until the last section.

I don't think we should touch the module itself. If you want to collapse it, then you can add the parent of the module itself to the list. I think having to say "it does this for everything except for X" is always a potential source of confusion. :)

So given the option collapse_module_prefixes: [Foo.Bar.Baz]

For the reason above, I think we use the naming "nesting"/"nested" instead of "prefixes".

[davidsulc]

If you want to collapse it, then you can add the parent of the module itself to the list.

I'd like to collapse it, because then the collapsed parts will align with the collapsed parts in the follwing lines (i.e. the nested modules) which makes it easier to follow things visually.

Adding the uncollapsed parent to the list is a good idea, but what if that module doesn't exist?

For the reason above, I think we use the naming "nesting"/"nested" instead of "prefixes".

👍

[josevalim]

Adding the uncollapsed parent to the list is a good idea, but what if that module doesn't exist?

Elixir does not require the parent to exist. So you can say collapse: [Google.PubSub] and you should be fine. :)

[davidsulc]

I know, but that would mean there'd be an entry in the sidebar for a module that doesn't exist. Wouldn't that be confusing? Or am I missing something?

I misunderstood what you meant. I thought by "add it to the list" you meant the sidebar, when you were in fact talking about adding it to the option list. 👍

[tmbb]

Two quick notes:

• The module name abbreviations should use a different font style (italic or something like that), so that it's clear that it's an abbreviation.

• there might be cases in which the first letter might not be enough. For example: App and AppWeb. Maybe it would be better to abbreviate AppWeb as AW. This could be inflected automatically from the module name in the case where an abbreviation would be ambiguous or could be specified by the user.

[davidsulc]

Regarding the 2nd point, I realized that shortly after posting. My inclination is to not handle this in the first (naive) implementation, as disambiguation would need a handled a bit more thoroughly in my opinion. E.g. handle the case where Foo.Bar.One and Foo.Baz.Two are collapsed with a [Foo.Bar, Foo.Baz] option: they would appear as F.B.One and F.B.Two which isn't great (since F.B. refer to different things in each case)...

But as I said, I think I would leave that for a later improvement. In the meantime, users can either live with the ambiguity in their docs, or ensure the collapsing doesn't cause ambiguity (by only collapsing ambiguity-free sections). Also, as this would be opt-in, it can simply not be used in cases where the naive solution doesn't fit, until an enhanced implementation is provided in the future.

[tmbb]

But as I said, I think I would leave that for a later improvement. In the meantime, users can either live with the ambiguity in their docs

The ambiguity can be removed by hovering the mouse over the link, so it shouldn't be too bad. But yeah, it's a problem.

[OvermindDL1]

This is just one idea I've thought about now, but the main point is that i really dislike reusing the group system for this.

I agree with this as well.

I like @tmbb's idea. Here's what it could look like for (part of) the mentioned lib:

After seeing that I am really not a fan of it, it seems exceptionally hard to read to me. I'd prefer it to be more like:

Some.Long.Module.Path
* Something
* More.Thing
* Etc

Or render like this or so:

Some.Long.Module.Path

• Something
• More.Thing
• Etc

[josevalim]

After seeing that I am *really* not a fan of it, it seems exceptionally

hard to read to me. I'd prefer it to be more like: That’s what the group system would provide. Sure, we are relying on the current group implementation but we could easily add change font style on the heading too. -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

[davidsulc]

I'm taking the liberty of closing this in favor of #907 (and I humbly suggest the discussion continue there), but feel free to reopen.