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

fix URLs for dirhtml builds #1699

Merged
merged 2 commits into from
Feb 16, 2024
Merged

fix URLs for dirhtml builds #1699

merged 2 commits into from
Feb 16, 2024

Conversation

drammock
Copy link
Collaborator

@drammock drammock commented Feb 1, 2024

closes #1675

@kathatherine can you try out this branch to see if it fixes dirhtml builds for you?

@drammock drammock added the kind: bug Something isn't working label Feb 1, 2024
@kathatherine
Copy link

Followed most of the instructions here to set up a dev environment with your branch. Whether I build the pydata-sphinx-theme docs or my own set of docs, I get the following extension error:

Handler <function _fix_canonical_url at 0x10598cae0> for event 'html-page-context' threw an exception (exception: _fix_canonical_url() takes 3 positional arguments but 5 were given)

Uncertain if I've done something wrong or if there's an underlying issue with the branch, but it does appear that the failing tests are giving the same error.

Thanks for attempting to fix this!

Copy link
Collaborator Author

@drammock drammock left a comment

Choose a reason for hiding this comment

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

sorry about that @kathatherine! I forgot to check back to see how the CIs did (and obviously I failed to run the tests locally before pushing 🙈). I think this change should resolve it.

FWIW you shouldn't need to set up a full PST dev environment to test on your own project; changing your project's environment to install the theme from this branch should be enough to test it out. E.g.

pip install -U https://github.com/drammock/pydata-sphinx-theme/archive/refs/heads/canonical-urls.zip

src/pydata_sphinx_theme/__init__.py Outdated Show resolved Hide resolved
@kathatherine
Copy link

Thanks for the tip on that theme install. TIL!

This appears to get the job done for my dirhtml built docs. 🎉

I didn't notice any edge cases when clicking around. Thanks very much!

@drammock
Copy link
Collaborator Author

Team, I'll plan to self merge this one tomorrow unless someone wants extra time to review. I think it's a pretty innocuous change

@drammock drammock merged commit 99f9b1c into pydata:main Feb 16, 2024
18 checks passed
@drammock drammock deleted the canonical-urls branch February 16, 2024 23:03
ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this pull request Jun 5, 2024
* fix URLs for dirhtml builds

Co-authored-by: David Lord <[email protected]>

* Update src/pydata_sphinx_theme/__init__.py

---------

Co-authored-by: David Lord <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canonical URLs wrong in dirhtml build
2 participants