Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add :reference_apps option #1709

Closed

Conversation

viniciusmuller
Copy link
Contributor

This pull request adds the :reference_apps option, which will take a list of dependencies and generate completions in the sidebar that link to their original documentation page.

I invite anyone who might be interested in this feature to give the branch a try and report any issues

Preview

reference_apps

TODO

  • Add tests

Closes #1551

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've bumped the Remix Icon font to its latest version, since the currently used version does not support the external-link icon

lib/ex_doc/formatter/html.ex Outdated Show resolved Hide resolved
lib/ex_doc/formatter/html.ex Outdated Show resolved Hide resolved
@ericmj
Copy link
Member

ericmj commented May 23, 2023

My concern with this is that after you follow the link you will lose the original search context because the new app you landed will not have the same deps.

Something that has come up recently is that it can be hard to navigate the phoenix docs because phoenix has been split out into multiple apps over time. Imagine you are in https://hexdocs.pm/phoenix and search for "render", you get hits for Controller.render and View.render, you follow the link to https://hexdocs.pm/phoenix_view and then search for "render" again, now the Controller.render result will no longer show up even though you are still in a phoenix framework app. I imagine this would be confusing to users.

I think we need another solution that is not based on deps.

@wojtekmach
Copy link
Member

Additionally, the way things are at the moment we need to put into :reference_apps things that are already in deps(), ie it's redundant and instead things from deps should automatically be available?

@viniciusmuller
Copy link
Contributor Author

Hey @ericmj and @wojtekmach, I see what you mean.

The idea would to have something like

phoenix -> reference_apps: [:phoenix_liveview, :phoenix_ecto],
phoenix_liveview -> reference_apps [:phoenix, :phoenix_ecto]

In a way that projects can build "unified" search results across multiple ex_doc instances, without directly depending on them, right?
I think this makes sense, my only doubt is about how we would go about implementing this, should we need to fetch and track each one of the referenced apps and then access its ebin directory when building the docs?

Also, in order for ease of testing, I think we first try to reach the local deps and fallback to pulling it from hex

@wojtekmach
Copy link
Member

One idea would be to have a https://hexdocs.pm/foo/dist/references-<sha>.json (similar to sidebar_items.json, search_items.json) that would contain all references that a package exposes. And then based on :reference_apps we'd be hitting these URLs, either in the browser (relying on browser local cache) or during mix docs prefetch them and save them off somewhere.

If we have a solid list of references we could use them for popovers too:

image

On the flip side, this would potentially be a lot of data though.

@viniciusmuller
Copy link
Contributor Author

Wouldn't search_items.json already be enough? Currently the dependency completions are being built from the same data.

The workflow could be something like

Enum.map([:ecto, :ecto_phoenix], &build_app_docs/1)

where build_app_docs would fetch the search_items.json from the upstream hexdocs page and parse it, rather than building the documentation for the apps locally.

Since it would always be run when building the docs and stored inside the project, I think there would be no cache issues.

Also, should we always link to the latest version? Currently it links to the version used by the project, but I think this will no longer make sense when fetching remote deps

@josevalim
Copy link
Member

Thank you @viniciusmuller! This is amazing but I guess there are good concerns about the feature not being transitive. :(

@josevalim
Copy link
Member

Given the limitations above, we will plan to tackle this in other ways, such as providing hexdocs-wide search. Thank you!

@josevalim josevalim closed this Jul 5, 2023
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.

Add reference_apps
4 participants