Add favicons option for defining multiple favicons #359
Conversation
|
I think this is a great idea - please ping when it’s ready for a look over! |
|
@choldgraf I'd certainly appreciate some feedback at this point - I haven't written any tests yet, but I'd like to make sure this is headed in the right direction! |
jorisvandenbossche
left a comment
There was a problem hiding this comment.
I think this looks good!
|
I think the implementation looks great as well. My one thought is that this would be a useful extension to live outside of this theme, so that other people could use it as well. I think of this as similar to the sphinext-opengraph extension which adds metadata to I know that creating a sphinx extension is extra work, but I think this could have broader use from the community. Any interest? |
|
We are using sphinext-opengraph for Bokeh (and I have even contributed to the repo myself). That is a great idea and I'd love to do that, but I won't have the time to get that going in the next few weeks. We would like to use the favicon feature for Bokeh, so it would be great if we could keep it simple for now and just have this as a feature in the theme. But I'll plan on turning this into a dedicated extension and ping you once it is ready! |
|
@tcmetzger do I have your permission to make a little Sphinx extension that does this and push it to PyPI by tomorrow? :-) I'd put it in the otherwise, I'm fine just merging this in here, though FWIW I think that it'll be a faster road to PyPI if we publish it as a lightweight extension, than if we wait for the pydata theme to make another release 🙃 |
|
@choldgraf Sure, that is no doubt the quickest solution. I'd appreciate it if you could list me as an author! |
|
or upstream to sphinx .. ;) (but of course that's only for the long term) |
|
@jorisvandenbossche that's a good point - let's just get this one in here quickly and open an issue in Sphinx to see if they'd be interested in it. I think @tcmetzger wanna add a quick test to make sure the functionality works? Other than that LGTM |
|
@choldgraf Quick test has been added. Great idea with the Sphinx issue! |
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Thanks for the tests!
|
There is a sphinx PR that tries to do something similar, but specific for apple: sphinx-doc/sphinx#8249 (might be worth chiming in there, I think the sphinx maintainer already proposed to make it more generic, similar as done here) |
As discussed in bokeh/bokeh#10971, I have been experimenting with adding favicons (beyond the standard sphinx options).