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

Can't change sphinxnote to use sphinxheavybox starting with 5.1.0 #11074

Closed
therealprof opened this issue Jan 2, 2023 · 5 comments · Fixed by #11081
Closed

Can't change sphinxnote to use sphinxheavybox starting with 5.1.0 #11074

therealprof opened this issue Jan 2, 2023 · 5 comments · Fixed by #11081
Labels
builder:latex type:enhancement enhance or introduce a new feature
Milestone

Comments

@therealprof
Copy link

therealprof commented Jan 2, 2023

Describe the bug

I used to redefine the "light" box environments to use heavybox instead:

\renewenvironment{sphinxnote}[1] {\begin{sphinxheavybox}\sphinxstrong{#1} }{\end{sphinxheavybox}}
\renewenvironment{sphinxhint}[1] {\begin{sphinxheavybox}\sphinxstrong{#1} }{\end{sphinxheavybox}}
\renewenvironment{sphinximportant}[1] {\begin{sphinxheavybox}\sphinxstrong{#1} }{\end{sphinxheavybox}}
\renewenvironment{sphinxtip}[1] {\begin{sphinxheavybox}\sphinxstrong{#1} }{\end{sphinxheavybox}}

However, starting with 5.1.0 I get the following error(s) when running LaTeX on the generated code:

Chapter 1.
(/usr/local/texlive/2022/texmf-dist/tex/latex/base/ts1cmss.fd)
! Missing number, treated as zero.
<to be read again>
                   \spx@note@border@top
l.181 \begin{sphinxadmonition}{note}{Note:}

with previous versions this worked fine and as intended. I haven't been able to find a workaround but I suppose this might be related to the introduction of the new configuration attributes in #10648 which doesn't exist for notes?

How to Reproduce

Add this to your conf.py and try to render a document which uses notes:

latex_elements = {
  'preamble':
      r'''\renewenvironment{sphinxnote}[1] {\begin{sphinxheavybox}\sphinxstrong{#1} }{\end{sphinxheavybox}}'''
}

Environment Information

Platform:              darwin; (macOS-10.16-x86_64-i386-64bit)
Python version:        3.9.15 (main, Oct 11 2022, 22:25:13)
[Clang 12.0.0 (clang-1200.0.32.29)])
Python implementation: CPython
Sphinx version:        6.0.0
Docutils version:      0.18.1
Jinja2 version:        3.1.2

Sphinx extensions

No response

Additional context

5.0.2 works just fine, every later version I tried after that is broken.

@jfbu
Copy link
Contributor

jfbu commented Jan 3, 2023

Yes since #10648 the heavybox environment executes some LaTeX set-up which depends on the name of the notice (warning, attention, etc...) and configures the rendering to match either the default or the conf.py setting of the relevant 'sphinxsetup' keys.
If the notice type is not one of warning, caution, attention, danger, error, this step will try to access non-existing LaTeX data and thus crash TeX.

I have been thinking briefly on your problematic and see currently two ways:

  • one may recover from Sphinx 5.0 the former code for sphinxheavybox environment, rename it to sphinxoldheavybox and inject it into your extra preamble, then you use sphinxoldheavybox in your definitions. This has the advantage to a priori work also will older Sphinx releases.
  • or another way is to let your environments use \sphinxsetup to configure the LaTeX: box padding (t,r,b,l), borders (t,r,b,l), shadows (x,y) or radii #10648 parameters affecting the (say) warning type (the effect will only be local), then the environment also must do \def\spx@noticetype{warning} which is a trick and it calls sphinxheavybox after that. In this way you can configure as you wish your environments even though there is no official interface for that. But you have to define all attributes else changes at top level to the warning associated layout attributes will influence your own note type notices.

Not tested, but if you tell me which way looks best, I can give more details.

As per making your usage work out-of-the-box at it worked with Sphinx 5.0 I do not see this at this time as really feasible in a natural way. Not tagging this ticket as Bug because it is not clear why a change to sphinxheavybox should have been accompanied with compatibility layer with a direct use of it. But I may be prejudiced... it is true that people/extensions who used sphinxheavybox directly for own added admonition types got broken by #10648, because the environment now requires extra things to be set-up.

@therealprof
Copy link
Author

The documentation does point out that the admonition boxes can be \renewenvironmented and that the default style is different which to me is a hint that this approach was envisioned:

They may be \renewenvironment ‘d individually, and must then be defined with one argument (it is the heading of the notice, for example Warning: for [warning](https://docutils.sourceforge.io/docs/ref/rst/directives.html#warning) directive, if English is the document language). Their default definitions use either the sphinxheavybox (for the last 5 ones) or the sphinxlightbox environments, configured to use the parameters (colours, border thickness) specific to each type, which can be set via 'sphinxsetup' string.

Also I'm somewhat convinved that there're other users than me who would rather prefer an identical styling of all admonition boxes.

Regarding sphinxoldheavybox, that doesn't sound too bad to me, although I was kind of hoping I could start to get rid of all the TeX hacks at some point and use theming/configuration instead; I do have custom classes at the moment and they're super fragile so I already have to pin versions... I've just added a ton of extra magic to allow use with all versions from 2.4.5 to 5.0.2 yesterday. For me it would be nice to drop support for very old versions and instead support the latest but this "non"-bug is certainly in my way...

@jfbu
Copy link
Contributor

jfbu commented Jan 3, 2023

@therealprof I wrote this 6 years ago and it wasn't meant to imply one could use sphinxheavybox for the light admonitions without extra preparation for all eternity... I did at that time add to the code the extra step which made it possible at that time (i.e. I prepared a dummywhite background color to the light admonitions, so the sphinxheavybox was usable without breaking from the lack of a background color configuration). However this was more like a non-documented internal feature. Now the sphinxheavybox has for each of the associated admonition type 20 style attributes, meaning as many LaTeX macros or other storage are needed for each notice type.

The sphinxheavybox adds some potential restrictions regarding to nesting, and migrating all "light" notices to the "heavy" could break some projects (and I could not guarantee the same vertical spacing, but I don't think people care much). The documentation is currently indeed lacking explanations on how one can use sphinxheavybox directly, but this is very heavy LaTeX stuff, and the few Sphinx users which would be at ease with it may as well read the source code in sphinx.sty and sphinxlatexadmonitions.sty and they will know what to do in order to genuinely extend the codebase. And it is as effective to use the trick I alluded to to recycle the warning type by pretending via a suitably located \def\spx@noticetype{warning} to be a warning and emit then the \sphinxsetup{....} for warning which will be appropriate to configure all 20 keys each time once custom environment is met.

This being said, I am willing to refactor a bit the code so that your use case would work as is. This will be a bit costly but well my initial impression is that it is feasible. Must look into it.

@jfbu jfbu added type:enhancement enhance or introduce a new feature and removed type:question labels Jan 3, 2023
@jfbu jfbu added this to the 6.1.0 milestone Jan 3, 2023
@jfbu
Copy link
Contributor

jfbu commented Jan 3, 2023

@therealprof Done for 6.1.0.

@therealprof
Copy link
Author

Thanks a lot!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
builder:latex type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants