Skip to content

Fix broken links when using dirhtml builder#1060

Merged
larsoner merged 7 commits intosphinx-gallery:masterfrom
mgoulao:master
Jan 3, 2023
Merged

Fix broken links when using dirhtml builder#1060
larsoner merged 7 commits intosphinx-gallery:masterfrom
mgoulao:master

Conversation

@mgoulao
Copy link
Copy Markdown
Contributor

@mgoulao mgoulao commented Dec 30, 2022

The idea of this PR is to develop a better solution to #921.

  • Remove dirhtml blocking and associated test
  • Add condition to directives.visit_imgsg_html to support dirhtml special case

I'm unaware of any other problem related to using the dirhtml builder.

@larsoner
Copy link
Copy Markdown
Contributor

larsoner commented Jan 2, 2023

I'm unaware of any other problem related to using the dirhtml builder.

I think in order to un-block it like this we need to test it to make sure it succeeds without error. The easiest way to add such a test would probably be to run the tinybuild HTML build using the dirhtml builder.

@mgoulao
Copy link
Copy Markdown
Contributor Author

mgoulao commented Jan 3, 2023

I have added a simple test_error_messages_dirhtml test which passed. Is this enough? Or should I duplicate every test using the sphinx_app fixture?

@larsoner
Copy link
Copy Markdown
Contributor

larsoner commented Jan 3, 2023

Or should I duplicate every test using the sphinx_app fixture?

No I don't think that's necessary. I'll try to push a commit to make CIs happy, at the very least CircleCI is protecting against unregistered/new users...

@larsoner larsoner enabled auto-merge (squash) January 3, 2023 18:07
@larsoner larsoner merged commit 4006662 into sphinx-gallery:master Jan 3, 2023
@larsoner
Copy link
Copy Markdown
Contributor

larsoner commented Jan 3, 2023

All green after a few style fixes and making the code more DRY, thanks @mgoulao !

@larsoner larsoner added the bug label Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants