coderefinery / sphinx-lesson

Sphinx extension for creating CodeRefinery lessons
https://coderefinery.github.io/sphinx-lesson/
MIT License
15 stars 20 forks source link

Copy button on code blocks broken in main page #50

Closed robertodr closed 3 years ago

robertodr commented 3 years ago

It seems that the copy button is broken (i.e. not rendering correctly), but only for code blocks on the main page: https://enccs.github.io/intermediate-mpi/#graphical-and-text-conventions

rkdarst commented 3 years ago

After some investigation...

Looking at the source, this is the element of the copy button itself. It seems the icon is not showing, and instead the alt text is, which :

<img src="#_static/copy-button.svg" alt="Copy to clipboard">

Note the extra # on it, so the image link doesn't work. My sites don't have that # there. If I add the # on my site, I get the same behavior as you see there.

I have no idea yet where that # can come from. I downloaded that repository and built it myself, and it worked. Has it always been this way? I notice this directly has a code-block inside of a signature class, and I couldn't find any other examples of that. Could you try varying different things and see if you can find any other patterns of it occuring. Does it happen when building locally?

My current hope is there was some bug in some upstream version of libraries, which is fixed now, so this will go away by itself...

robertodr commented 3 years ago

I have tried to move the code block outside the signature directive and it still shows the alt-text. It happens only for code blocks (or literal includes) in index.rst anywhere else in the document the icon is shown correctly. It also happens when building through the GitHub action :(

rkdarst commented 3 years ago

Just to be clear: when you build it locally, it works correctly?

I just made a PR with more debugging information (versions of installed packages)

robertodr commented 3 years ago

Nope, it does not work locally either.

rkdarst commented 3 years ago

Update!

Also index.rst is the only document which keeps its name: all other get renamed to index.html and put into a sub-directory, to have pretty URLs. This isn't the first error of an executablebooks project for the dirhtml builder, but I can't yet see how it could be a problem of sphinx-copybutton itself... I will investigate more.

rkdarst commented 3 years ago

Reply from https://github.com/ENCCS/intermediate-mpi/pull/42, since I commented on the wrong issue:

A-ha! Thanks for the detective work! So if we renamed index.rst to something else it would fix the problem?

rkdarst commented 3 years ago

It seems like yes, but you really would want it to be called index.rst, so that the root would work. I am making another test repository to narrow down the problem (it happens with only sphinx+sphinx-copybotton, so after I narrow down some more, I will report it to sphinx-copybutton while I continue investigating)

robertodr commented 3 years ago

Many thanks for digging!

rkdarst commented 3 years ago

I think it is a Sphinx problem. Debugging found here: https://github.com/executablebooks/sphinx-copybutton/issues/110

rkdarst commented 3 years ago

Fixes for Sphinx and sphinx_rtd_theme:

But these require an upstream release to properly fix. Next up, local hackish command to do it.

rkdarst commented 3 years ago

My workaround for fixing is this is currently

sed -i 's/url_root="#"/url_root=""/' _build/dirhtml/index.html || true

Let's see if it works for intermediate-mpi before I apply it here.

robertodr commented 3 years ago

It worked! Thanks!

rkdarst commented 3 years ago

This is now fixed upstream in both sphinx and sphinx_rtd_theme. sphinx-book-theme is not affected. Other themes may possible need fixing, if they have copied the wrong code.

I think this workaround should be maintained for quite a while, though, since things will be slow to update everywhere, and we don't want to strongly tie to any particular versions.

Thanks for reporting this!