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.

Sign up for GitHub

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

Jump to bottom

Rename html_theme_options keys "pygment_light_style" and "pygment_dark_style" #1594

Closed
cmarqu opened this issue Dec 14, 2023 · 7 comments · Fixed by #1614
Closed

Rename html_theme_options keys "pygment_light_style" and "pygment_dark_style" #1594

cmarqu opened this issue Dec 14, 2023 · 7 comments · Fixed by #1614

Comments

@cmarqu
Copy link
Contributor

cmarqu commented Dec 14, 2023

The keys html_theme_options["pygment_light_style"] and html_theme_options["pygment_dark_style"] are (well-entrenched) misnomers - since the project name is "Pygments" with an "s", they should be called
html_theme_options["pygments_light_style"] and html_theme_options["pygments_dark_style"].

(This is obviously a very minor issue, I admit :))
@drammock
Copy link
Collaborator

I don't disagree. However, Sphinx itself has a config option pygments_dark_style already: https://www.sphinx-doc.org/en/master/development/theming.html#creating-themes

...which I think (?) was the reason why @12rambau chose to (mis)name the theme-specific options here. If that's right, we could I suppose change them to something else like highlight_style_light and highlight_style_dark, but IMO that just creates unnecessary work for downstream theme users (who, if they've customized the highlighting theme, will all need to change their conf.pys just to keep things working). To me that's not worth it... but I'll wait for @12rambau to weigh in before closing.

@12rambau
Copy link
Collaborator

12rambau commented Jan 2, 2024

I know myself and the simplest explanation is the right one: it's a typo....
@drammock I checked, and Furo uses a dedicated pygments_style_light and pygments_style_dark parameter to control the rendeing if highlighted code. So I guess we could change it back with a deprecation process.

@drammock
Copy link
Collaborator

drammock commented Jan 2, 2024

I still think that changing the name

creates unnecessary work for downstream theme users (who, if they've customized the highlighting theme, will all need to change their conf.pys just to keep things working). To me that's not worth it

@12rambau
Copy link
Collaborator

12rambau commented Jan 2, 2024

Even with a super long deprecation cycle (like 1.0) ?
I don't really like to have typos included in our parameters.

@cmarqu
Copy link
Contributor Author

cmarqu commented Jan 2, 2024

Feels too small to open an issue or PR, so I'll mention it here: the file src/pydata_sphinx_theme/pygment.py is also missing an "s".

@drammock
Copy link
Collaborator

drammock commented Jan 2, 2024

Even with a super long deprecation cycle (like 1.0) ? I don't really like to have typos included in our parameters.

Fair enough. Ok with me to change, with deprecation warning.

@drammock
Copy link
Collaborator

drammock commented Jan 2, 2024

Feels too small to open an issue or PR, so I'll mention it here: the file src/pydata_sphinx_theme/pygment.py is also missing an "s".

@12rambau let's fix this at the same time

@12rambau 12rambau mentioned this issue Jan 3, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: deprecate pygments related variable to correct typos 12rambau/pydata-sphinx-theme
3 participants
@cmarqu @drammock @12rambau