Skip to content

Conversation

@eksperimental
Copy link
Contributor

Closes #951

@eksperimental eksperimental force-pushed the dont_link_to_moduledoc_false_current_project branch from 360ddb2 to 6b14616 Compare May 23, 2019 04:19
@josevalim
Copy link
Member

Thanks @eksperimental!

Unfortunately, if we go through the retriever every time we autolink, that's going to be very expensive. I suggest for us to try to read the "Docs" chunk from the .beam file directly without going through the retriever. We should likely also implement a cache, to avoid doing the same lookup over and over again.

@eksperimental
Copy link
Contributor Author

How do we keep state? :ets

@eksperimental eksperimental force-pushed the dont_link_to_moduledoc_false_current_project branch from 6b14616 to 43360b0 Compare May 27, 2019 04:36
@eksperimental
Copy link
Contributor Author

Please have a look again, no need to cache things.

@eksperimental eksperimental force-pushed the dont_link_to_moduledoc_false_current_project branch from 43360b0 to 822d3cd Compare May 27, 2019 05:02
@eksperimental eksperimental mentioned this pull request May 27, 2019
@eksperimental eksperimental force-pushed the dont_link_to_moduledoc_false_current_project branch from 822d3cd to 711f3fc Compare May 27, 2019 05:09
@josevalim
Copy link
Member

Thanks @eksperimental, this looks much better! I think it can still be simpler though.

Since we only keep @moduledoc false for the current project, we actually don't need to know if the module is hidden or not since, if the module does not have the documentation and the module exists in the project, then it is by definition with doc false.

So my suggestion is to break the retriever pipeline, which does: docs_from_dir |> docs_from_files |> docs_from_modules into something like this:

all_modules = Retriever.modules_from_dir(config.source_beam, config)
docs = Retriever.docs_from_modules(all_modules, config)

I prefer this approach because it is a less invasive change and it means we don't have to change docs_from_modules and friends to return a tuple. Can you please investigate? Thank you.

@eksperimental eksperimental force-pushed the dont_link_to_moduledoc_false_current_project branch from 711f3fc to 12155d2 Compare May 30, 2019 05:19
@eksperimental
Copy link
Contributor Author

eksperimental commented May 30, 2019

Thanks @eksperimental, this looks much better! I think it can still be simpler though.

Since we only keep @moduledoc false for the current project, we actually don't need to know if the module is hidden or not since, if the module does not have the documentation and the module exists in the project, then it is by definition with doc false.

So my suggestion is to break the retriever pipeline, which does: docs_from_dir |> docs_from_files |> docs_from_modules into something like this:

all_modules = Retriever.modules_from_dir(config.source_beam, config)
docs = Retriever.docs_from_modules(all_modules, config)

I prefer this approach because it is a less invasive change and it means we don't have to change docs_from_modules and friends to return a tuple. Can you please investigate? Thank you.

Ok. no need to create any new function, but if you still want docs_from_modules I can provide it in another PR.
yes, this was the way to to go, Apparently I stuck to the tuple return value, probably since when I was trying to support Elixir modules, or even Erlang.

Have a look and tell me what you think.
This is ready to be merged

UPDATE: Kernel.ParallelRequire.files/2 is still being linked. I'm working on a solution

@michalmuskala
Copy link
Member

I wonder - should it produce a warning when this situation happens? It seems to me like this can be a "bug" in the documentation when you refer to a private module.

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.

Linking to non available functions

3 participants