CadQuery / sphinxcadquery

An extension to visualize CadQuery 3D files in your Sphinx documentation
BSD 3-Clause "New" or "Revised" License
13 stars 4 forks source link

changed add_javescript to add_js_file #21

Closed shimwell closed 3 years ago

shimwell commented 3 years ago

As reported in

20

I noticed the error report in a recent build of using this package had the error

AttributeError: 'Sphinx' object has no attribute 'add_javascript'

Screenshot from 2021-05-09 23-30-03

Then I found the deprecated API infor in Sphinx than mentioned add_javascript() has been replaced by add_js_file() and will be removed in v4.0.

https://www.sphinx-doc.org/en/master/extdev/deprecated.html?highlight=add_javascript#deprecated-apis

Screenshot from 2021-05-09 23-32-33

So I've replaced occurrences of add_javascript with add_js_file in the sphinxcadquery/sphinxcadquery.py file

I think this will continue to work with old version and new versions of Sphinx. But I've not found a nice way to test it.

Peque commented 3 years ago

@Shimwell Thanks for the pull request and for making some investigations on the issue! :blush:

As you may have already noticed, there is no CI setup for this project (no tests), so could you please test the changes locally to verify everything works as expected? If you test it with Sphinx 4 only, that would be fine. If you test it with Sphinx 3 too, that would be perfect. :stuck_out_tongue_winking_eye:

I suspect more changes may be needed for Sphinx 4 compatibility. In example, add_stylesheet seems that it was also deprecated in 1.8 and removed in 4.0.

Peque commented 3 years ago

@Shimwell Looking good! :blush:

Would you mind squashing the changes into a single commit with title "Fix Sphinx 4 compatibility issues"?

shimwell commented 3 years ago

@Shimwell Looking good! blush

Would you mind squashing the changes into a single commit with title "Fix Sphinx 4 compatibility issues"?

Not sure if I got that correct (first time I've tried a git squash) but I shall make a cleaner PR and then you can take your pick :-)

Peque commented 3 years ago

@Shimwell Don't worry, I can do that for you if you want. If you'd rather try again yourself, I'll wait! :blush:

By the way, I meant "Fix Sphinx 4 compatibility issues", but without the quotes around. :stuck_out_tongue_winking_eye:

shimwell commented 3 years ago

@Shimwell Don't worry, I can do that for you if you want. If you'd rather try again yourself, I'll wait! blush

By the way, I meant "Fix Sphinx 4 compatibility issues", but without the quotes around. stuck_out_tongue_winking_eye

Yep I wasn't sure about those quote marks were needed in the Nano commit message. Thanks for training me up here.

Peque commented 3 years ago

Closing after https://github.com/Peque/sphinxcadquery/pull/25.