bavovanachte / sphinx-wavedrom

A sphinx extension that allows including wavedrom diagrams by using its text-based representation
MIT License
34 stars 18 forks source link

Fall back to wavedrompy, or give more helpful error message #34

Closed imphil closed 2 years ago

imphil commented 3 years ago

When I updated to version 3.x I got failing ReadTheDocs builds with wavedrom command 'npx wavedrom-cli' cannot be run, which is due to the default being now wavedrom CLI being used, which cannot be installed on ReadTheDocs.

To improve user experience, would it be possible to

bavovanachte commented 3 years ago

Excellent point. The first suggestion should be easy to implement, as the exception handler is already there. I'll get right on that. Apologies for the inconvenience.

I'd have to think about the fallback option. Even with verbose messaging about the fallback, this can easily get lost in big logs and I want the user to be very aware of the fact that wavedrompy might yield different results than the official wavedrom implementation. My first feeling would be to stick with your first suggestion, but I might change my mind.

bavovanachte commented 3 years ago

Made a proposal in https://github.com/bavovanachte/sphinx-wavedrom/pull/35. Could I ask you to review it?

amykyta3 commented 3 years ago

Just hit this too. Shouldn't render_using_wavedrompy = True be the default? As far as I'm aware, ReadTheDocs doesn't have hooks to install non-python tools. Also the original issue (#23) even mentions that this should be an opt-in feature, keeping wavedrompy be the default.

bavovanachte commented 2 years ago

It doesn't seem like wavedrompy is keeping up with upstream wavedrom changes anymore, so I'd say it's good that it's not the default anymore. Closing this issue