search

spatialaudio / nbsphinx

:ledger: Sphinx source parser for Jupyter notebooks
https://nbsphinx.readthedocs.io/
MIT License
451 stars 130 forks source link

Fix attachment regex when no markdown text #526

Closed alanhdu closed 3 years ago

alanhdu commented 3 years ago

Right now, the regex is suited for things like:

This is an image ![logo-64x64.png](attachment:cbe9739f-7fcd-4ed5-866f-80d693d30fe8.png)

which gets translated into:

This is an image: |logo-64x64.png|

.. |logo-64x64.png| image:: attachment:cbe9739f-7fcd-4ed5-866f-80d693d30fe8.png

However, if you have an image without an text in front of it like:

![logo-64x64.png](attachment:cbe9739f-7fcd-4ed5-866f-80d693d30fe8.png)

Then the rst looks like:

.. image:: attachment:cbe9739f-7fcd-4ed5-866f-80d693d30fe8.png
   :alt: logo-64x64.png

Crucially, this means that there is no |<filename.png>| to match, which causes the regex to fail. This is a simple solution which just makes the alt-text matching optional in the regex.

Fixes https://github.com/spatialaudio/nbsphinx/issues/525

alanhdu commented 3 years ago

I'm not exactly sure the right way to test this -- I checked manually that this fixes rendering my specific notebook. I added an example of this behavior that to the markdown-cells.ipynb notebook -- is that the best way to test this change? I haven't been able to build the docs locally based on some bibtex configuration error.

mgeier commented 3 years ago

Thanks for this PR!

is that the best way to test this change?

The "best way" is quite a strong claim, I'm not sure about that, but I think it's a good enough way to test it.

I haven't been able to build the docs locally based on some bibtex configuration error.

I've just merged #524 which should fix (or rather postpone) the error, can you please rebase?

alanhdu commented 3 years ago

Rebased!

mgeier commented 3 years ago

Thanks, the rendered pages is available at https://nbsphinx--526.org.readthedocs.build/en/526/markdown-cells.html#Cell-Attachments.

alanhdu commented 3 years ago

I believe the test failure is a flaky one https://github.com/spatialaudio/nbsphinx/pull/526/checks?check_run_id=1658998687#step:8:69:

(line 1573) broken    https://developer.mozilla.org/en-US/docs/Tools/Web_Console#Opening_the_Web_Console - Anchor 'Opening_the_Web_Console' not found

but when I click on the link, it seems to work fine.

mgeier commented 3 years ago

Thanks a lot!

Yes, the test failure is unrelated, I'm trying to fix it in #528.