-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support containers in LaTeX output. #9166
Conversation
Support containers in the LaTeX writer by outputting the start and end of a LaTeX group, wrapping the beginning and end of a LaTeX environment. The class name(s) are passed as an argument to the environment.
In docutils, the container node will be rendered as It would be better to follow the concept. @jfbu Do you have opinion for this? |
I'm a little confused. I agree that a macro would be better than an environment, but the links you provide seem to define a I am completely happy to refactor this to insert a macro as suggested, but that doesn't actually seem consistent with what docutils are doing. Please let me know what you would prefer. |
Ah, sorry. It's my bad. I'd like to mention the DUclass "environment". |
OK, so I am a little unclear as to what you are asking for. Would you like me to rename the environment as |
My large concern is generating an environment for each class, not for the whole of the node. For example, a container node having "red bold" will be converted to
About naming, I don't have opinion for this. So I think either '\DUclass |
Also rename sphinxcontainer to sphinxclass.
I've changed the code to one environment per class as requested, and renamed that environment to
|
With docutils' style, the following code will be converted the latter one internally
So users can change the behavior for each class by defining I think it's important to allow customizing each class. It enables that one extension creates "red" class and its macro, and another extension creates "bold" class and its macro. Additionally, could you add a testcase for this? Note: I confirmed the docutils' macro via this code:
|
Ah, sorry. My last comment was wrong.
No argument is passed to the |
OK will do. I am a little unclear on how a test of LaTeX output is supposed to work. Can you point me at an example. |
The requested code changes are in. I've browsed the tests and am still slightly mystified as to how they work, so would appreciate some guidance. |
In this case, please add a test case into
|
Thanks for the advice. A test is now included. |
Just emerged from marking hell. I see an administrator has triggered the tests and they have passed. Is there anything more I should do in order to get this merged? Many thanks, David |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits!
Thanks for nice contribution and sorry for delay
I assumed during reviewing this is only block-level mark-up as per docutils doc so we don't have to worry about space tokens.
% environment, and branching on the value of the argument. | ||
|
||
\ifx\sphinxclass\undefined % poor man's "provideenvironment" | ||
\newenvironment{sphinxclass}[1]{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I wonder if #1
needs some sanitizing of any sort. Accented letters or other special chars are not usable as LaTeX environment names and could cause breakage. But digits and spaces will be ok.
In our latex codebase we have this sphinx/sphinx/texinputs/sphinxlatexobjects.sty Lines 167 to 180 in e0b1e10
which was inherited many years ago from DocUtils I presume. Notice it uses But then we would need to provide some documented interface like this \newsphinxclassenv{red}{\color{red}}{} which would facilitate Sphinx user define the The definition would simply be \newcommand\newsphinxclassenv[3][{\newenvironment{\detokenize{#1}}{#2}{#3}} but then we have to use This can be added later after this will be merged once updated. EDIT: and then we can do a The |
OK. I think I claim I have dealt with all the requested changes. I haven't touched the handling of special characters as I believe you suggest this should be postponed to future work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for contribution.
We can add later handling of non-ascii class names.
Waiting for testing to complete, then I will push to this branch a CHANGES entry and merge. Thanks for great addition! |
rather I merge now and will push the CHANGES entry later once I fix may github email privacy settings... |
Looks good to me too! @jfbu Thank you for reviewing :-) |
This is my first contribution to the main sphinx project. Thank you for making it a very positive experience. I hope to have reason to contribute more in the future. |
LaTeX: add some documentation for container support (#9166)
Purpose
Support containers in the LaTeX writer by outputting the start and end of a LaTeX group, wrapping the beginning and end of a LaTeX environment. The class name(s) are passed as an argument to the
environment.
Detail
Container directives currently result in no output in the LaTeX writer, which is unfortunate because they are a very useful mechanism for adding an essentially arbitrary formatting hook in a document. A previous solution was proposed in #1336 which would have emitted an environment with the name of the directive class. This was not merged because emitting previously undefined environment names will break the
latexpdf
build unless the user also defines the environment, for example inlatex_elements
.This proposed solution avoids that problem by always emitting a
sphinxcontainer
environment. The class name(s) are passed into the environment as an argument. The environment emitted is wrapped in\bgroup
and\egroup
so that it also constitutes a LaTeX group. It is argued that this best mirrors the html behaviour and is therefore least likely to surprise a user.A definition of the
sphinxcontainer
environment is added in a style file input bysphinx.sty
, so inserting container directives will not break thelatexpdf
build. Should the user wish to insert LaTeX formatting based on a container directive, then they can redefinesphinxcontainer
inlatex_elements["preamble"]
. For example, if an.rst
file contains:and
conf.py
contains:then
red text
will be rendered in red, and all the other text will be displayed in black.Relates
Limitations
The documentation for container links straight through to the docutils documentation, so I don't know where I should document this change.
Life is a bit more complicated for the user if they need to customise the end of the environment depending on the container class, or if the container specifies multiple classes. However, both of these are really LaTeX limitations. I suggest that this small change provides all the relevant information through to LaTeX, and that fixing LaTeX's limitations in this matter should not be seen as Sphinx's problem.