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

Failed investigation: de-duplicate setting up the Favicon #324

Closed
Eric-Arellano opened this issue May 12, 2023 · 0 comments
Closed

Failed investigation: de-duplicate setting up the Favicon #324

Eric-Arellano opened this issue May 12, 2023 · 0 comments

Comments

@Eric-Arellano
Copy link
Collaborator

Eric-Arellano commented May 12, 2023

Possible follow up to #262.

Right now, users need to copy in favicon.ico in their _static folder and set html_icon = "_static/favicon.ico". I wanted to see if we can deduplicate some of this.

My first attempt was to provide favicon.ico in pytorch_base/static/images. But setting html_icon = "static/images/favicon.ico fails the Sphinx build. Even though it's normally okay for a project to reference something from the theme's static/images. ChatGPT offers this explanation (although note it may be misinformed):

My prompt

In a Sphinx extension, is there a way for me to auto-set a config value in the setup function? Normally, users would set this in their conf.py:

html_favicon = "images/favicon.ico"

I would like to instead have my extension automatically configure the option html_favicon.

ChatGPT answer

Yes, it's possible to set or modify Sphinx configuration values from within an extension's setup function. This function is called as part of the Sphinx build process, after the configuration file conf.py has been read, so you can override user configuration if necessary.

Here's an example of how you could do this:

def setup(app):
app.add_config_value('html_favicon', 'images/favicon.ico', 'html')

This setup function adds a new configuration value html_favicon. The first argument is the name of the configuration option, the second argument is the default value, and the third argument is the type of configuration option, in this case 'html'.

However, I would advise against this approach as a general practice. Overriding user settings can create confusion and unexpected behavior, which is not user-friendly. A better approach would be to provide sensible defaults and clear instructions for how to change those defaults in the extension documentation.

If you do need to override a configuration value, make sure that this behavior is clearly documented and there's a way for users to opt out. For instance, you could add a configuration option to control whether the html_favicon is overridden:

def setup(app):
app.add_config_value('override_html_favicon', False, 'html')

def builder_inited(app):
    if app.config.override_html_favicon:
        app.config.html_favicon = 'images/favicon.ico'

app.connect('builder-inited', builder_inited)

In this example, the html_favicon is only overridden if the override_html_favicon configuration option is set to True. The builder-inited event is used to modify html_favicon after all configuration values have been initialized.

I agree that we don't want to do something this magical. For example, this won't work properly if the user doesn't have html_static_path set up how we want.

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

1 participant