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

Take default og:site_name from sphinx project config value #83

Merged
merged 5 commits into from Nov 22, 2022

Conversation

rkdarst
Copy link
Contributor

@rkdarst rkdarst commented Nov 8, 2022

  • The standard project Sphinx config value seems like a reasonable
    default value for the site_name.
  • This uses that as a default, if ogp_site_name is not set.
  • This will change the default, and add og:site_name for many
    projects where it previously wasn't set.

Perhaps I should have asked before doing this, since the last point
may be a blocker. But it was easier to do than think too much...

- The standard `project` Sphinx config value seems like a reasonable
  default value for the site_name.
- This uses that as a default, if `ogp_site_name` is not set.
- This will change the default, and add `og:site_name` for many
  projects where it previously wasn't set.
README.md Outdated Show resolved Hide resolved
@rkdarst
Copy link
Contributor Author

rkdarst commented Nov 8, 2022

The reason I think this is useful is that I can add this extension to any site and it's closer to being functional even without any configuration.

Would we want a way to tell it to disable the og:site_name property? Right now, site_name can't be unset (assuming most sphinx projects have project set).

rkdarst and others added 2 commits November 8, 2022 15:36
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@TheTripleV
Copy link
Member

Yes, there should be a way to turn this off, similar to ogp_image_alt.

Default ogp_site_name to None.

if ogp_site_name is None:
  use project
elif ogp_site_name == False:
  omit og:site_name
else:
  use config site_name

@rkdarst
Copy link
Contributor Author

rkdarst commented Nov 8, 2022

Updated and works in my local test.

@TheTripleV TheTripleV merged commit 7469f2b into wpilibsuite:main Nov 22, 2022
@rkdarst rkdarst deleted the site_name-from-project branch November 22, 2022 20:42
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

Successfully merging this pull request may close these issues.

None yet

3 participants