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.

Sign up for GitHub

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

fix: Make docs links passthrough #1085

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −1
Open

fix: Make docs links passthrough #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions plugins/plotly-express/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
# options for sphinx_autodoc_typehints
always_use_bars_union = True

myst_all_links_external = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think instead of this we should add a URL scheme for absolute paths. See docs. I think we could just add '/' as a scheme to match any URL starting with a slash and effectively skip checking all absolute URLs without skipping checking internal URLs

Copy link
Collaborator

@mattrunyon mattrunyon Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nevermind that might require a colon like /:path to work. The other option would be we add a custom scheme like dh: to prefix external links and start failing on myst failures. Probably should so we don't have bad links internally

Copy link
Collaborator Author

@jnumainville jnumainville Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I did consider a custom scheme but seemed a bit annoying to have to keep track of if it can be avoided. But if we want to do that as a way to validate I'd be fine with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ya it's a hard pick because salmon doesn't rewrite any absolute links. So absolute links can't be to actual pages within the project because they won't work on salmon. /components/button.md would resolve to deephaven.io/components/button which won't exist.

In my ideal scenario absolute links always fail and require the schema. But it didn't look like myst worked that way and I'm not sure it's worth the effort it might take to enforce that.

I'd still prefer internal links be validated. We have broken links in the current docs I think, but they don't fail doc builds. They should fail doc builds


from deephaven_server import Server

# need a server instance to pull types from the autodocs
Expand Down
2 changes: 2 additions & 0 deletions plugins/ui/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,7 @@

from deephaven_server import Server

myst_all_links_external = True

# need a server instance to pull types from the autodocs
Server(port=10075)
2 changes: 1 addition & 1 deletion plugins/ui/docs/hooks/use_row_list.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,5 @@ table_row_list = ui_table_row_list(
## API reference

```{eval-rst}
.. dhaufunction:: deephaven.ui.use_row_list
.. dhautofunction:: deephaven.ui.use_row_list
```
Loading