Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sphinx: copy_asset_file method templating behaviour feedback #94

Closed
jayaddison opened this issue Mar 1, 2023 · 4 comments
Closed

sphinx: copy_asset_file method templating behaviour feedback #94

jayaddison opened this issue Mar 1, 2023 · 4 comments

Comments

@jayaddison
Copy link

This is a templated and low-importance issue report; I think it's worth mentioning, but I don't mind if you disagree :)

I'm suggesting some changes to sphinx upstream in sphinx-doc/sphinx#11165 and have been looking at the possible downstream effects of using a clearer Jinja template suffix in the copy_asset_file method -- or, in fact, potentially removing the Jinja templating behaviour from that method entirely.

Please let me know if you have any thoughts on whether you feel the templating functionality should be retained, can be migrated from _t suffix to .jinja suffix, or should not be migrated at all.

A completed migration would allow coveragepy to remove a few ambiguous warning messages (see nedbat/coveragepy#1489).

Thanks!
James

@Robpol86
Copy link
Owner

Robpol86 commented Mar 2, 2023

I definitely want the functionality to be retained but I don't mind the migration/renaming.

@jayaddison
Copy link
Author

Thanks, @Robpol86! I haven't opened many of these feedback requests (only two so far) but I'll collect the results and they'll influence the direction of sphinx-doc/sphinx#11165. Closing as you've provided a response (that I interpret as: yes, moving to a .jinja suffix in future is acceptable).

@jayaddison
Copy link
Author

@Robpol86 although the copy_asset_file templating feature likely wouldn't disappear for another ~2 years in Sphinx, with a major version upgrade, it is currently on a path towards deprecation.

Could you help me understand a bit more about the reason you're using the feature currently? It looks to me like it's to ensure that namespacing is applied for bootstrap CSS classes, and that kinda makes sense - although I don't yet understand how namespacing conflicts could occur there. (I'm reading up on it a bit more)

@Robpol86
Copy link
Owner

Robpol86 commented Apr 8, 2023

Thanks for letting me know.

I don't currently remember the reasons why I'm using that method, it's been a while since I worked on this project. A possible reason is for compatibility with Sphinx themes that are based on Bootstrap and for themes without. Also possible is for Sphinx themes that support carousels and those that don't. Since it's all CSS and I was avoiding calling a SCSS preprocessor during Sphinx docs build time to reduce complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants