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

Add per-file overrides using field lists #50

Merged
merged 23 commits into from
Feb 21, 2022

Conversation

ItayZiv
Copy link
Collaborator

@ItayZiv ItayZiv commented Dec 6, 2021

Use field lists to implement per page overrides
Closes #11.
It seems like __init__.py might be due for a slight rewrite after this change.

@ItayZiv ItayZiv marked this pull request as draft December 6, 2021 12:39
@ItayZiv
Copy link
Collaborator Author

ItayZiv commented Dec 12, 2021

Probably should be done at this point, but it really does seem like __init__.py is getting a bit messy, and test_options.py might also be due for a cleanup to make all the tests more similar.

@ItayZiv ItayZiv marked this pull request as ready for review December 12, 2021 13:16
Copy link
Member

@TheTripleV TheTripleV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. It is much needed. I didn't notice it was marked as ready till now, sorry!

The PR seems fine logic wise, there's just a few changes I want to increase readability of the code.

sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
@TheTripleV
Copy link
Member

Regarding the names of the fields, do field lists support underscores? If so, I think using underscores instead of hyphens would be better as the variable names in conf.py and the names of the field list keys would match.

Also, since making this extension support all opengraph tags would be nice. So, transforming all field list entries of the form
:og:X: Y to tags mapping og:X to Y.

@ItayZiv
Copy link
Collaborator Author

ItayZiv commented Jan 24, 2022

Regarding the underscores for names, I think it's supported but field lists everywhere else use hyphens, so I think it'd be better to be consistent with that.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Show resolved Hide resolved
sphinxext/opengraph/descriptionparser.py Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sphinxext/opengraph/__init__.py Outdated Show resolved Hide resolved
sphinxext/opengraph/descriptionparser.py Show resolved Hide resolved
@Daltz333 Daltz333 merged commit d5db5c4 into wpilibsuite:main Feb 21, 2022
@TheTripleV
Copy link
Member

Note, relative image paths in the custom overrides are broken (intentionally) in this PR. A fix should be included as a part of #53.

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.

Refractor and add directive.
3 participants