divi255 / sphinxcontrib.asciinema

Easily embed asciinema videos into Sphinx documentation
MIT License
17 stars 11 forks source link

Self hosted, wrong (fixed) player JS location #26

Open alperyazar opened 7 months ago

alperyazar commented 7 months ago

Hi,

https://github.com/divi255/sphinxcontrib.asciinema/blob/f1882d400c08a264c920973511acab2bfc504f72/sphinxcontrib/asciinema/asciinema.py#L40

I think, this commit assumes that the position of the page with asciinema directive is fixed to _static directory. This assumption makes many pages broken.

It looks like the previous version is fine: https://github.com/divi255/sphinxcontrib.asciinema/blob/75a71688118f4ca4f67c3faea673e7f686b72b74/sphinxcontrib/asciinema/asciinema.py#L25

.css file is included correctly thanks to: https://github.com/divi255/sphinxcontrib.asciinema/blob/f1882d400c08a264c920973511acab2bfc504f72/sphinxcontrib/asciinema/asciinema.py#L12

Thanks!

mike-matera commented 6 months ago

I can confirm that 0.4.0 does not display local cast files. Reverting to 0.3.8 fixed the problem.

divi255 commented 6 months ago

will look into this. please stay on 0.3 until fixed

divi255 commented 6 months ago

fixed both local and remote. sorry guys, was too busy to check the player upgrade commit. it broke completely everything.

0.4.1 works well

alperyazar commented 4 months ago

@divi255 Thanks for the fix, BUT

I tested the new 0.4.1 version today but for my website the self-hosted player looks broken, no player even shows up. I don't know the exact problem and don't know to debug but reverting back to 0.3.8 solves temporarily.

I have noticed a difference between 0.3.8 and 0.4.1. Both versions embed the cast file in HTML with Base64 which may not be the optimum solution in terms of size but it's fine for me. However, 0.4.1 embed the same Base64 string 3 times, one of just for div id. This potentially increases (unnecessarily ?) the HTML file if the recording is a long file.

0.3.8:

https://github.com/divi255/sphinxcontrib.asciinema/blob/d3fb128f6b245369d53768086a24c90855a1aa76/sphinxcontrib/asciinema/asciinema.py#L29

0.4.1:

https://github.com/divi255/sphinxcontrib.asciinema/blob/714264ed7fe100cc68257781af88cd21619610a9/sphinxcontrib/asciinema/asciinema.py#L40

divi255 commented 3 months ago

thanks for reporting. can you please upload somewhere an example where the player is broken? It was tested in both self-hosted and embedded mode and I found no problems with that. Maybe I'm missing some variation.

divi255 commented 3 months ago

about div-id, agree, it should be compacted

divi255 commented 3 months ago

div-id fixed in 0.4.2