LaTeX regression for admonitions

[chrisjsewell]

Presumably due to #12508, nesting a figure directive in a seealso admonitions now fails LaTeX builds:

.. seealso::

   .. figure:: path/to/image.png

! LaTeX Error: Not in outer par mode.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...                                              
                                                  
l.2119 \begin{figure}[htbp]

This can be replicated if you add a construct like this to one of the latex builds in tests/test_builders/test_build_latex.py

Also, putting a sphinxadmonition in tabulary now fails (this happens in the MyST documentation):

! Missing \endgroup inserted.
<inserted text> 
                \endgroup 
l.6208 \end{tabulary}

cc @jfbu @AA-Turner

[jfbu]

Yes this is caused by #12508. Putting a figure in any warning type admonition would have similarly failed earlier. Using image directive works.

I did not anticipate this for simple psychological reason that in LaTeX world the word figure is known to all to be usable only at top-level, so I would never have thought about putting a figure directive in reST sources inside wrapping context, simply due to sharing the name with the LaTeX concept and would automatically have used image there. My bad.

About the tabulary again this is not too much surprising, I guess this comes from mark-up with some admonition inside a table cell? I need to check Docutils reference whether this kind of construct is legit. I guess so... I always thought of admonitions as top level and certainly not as being usable inside table cells. Some non-obvious layer is implemented to allow nesting of admonitions.

And here too warning type admonitions surely never have worked, (at least since they were improved to allow a pagebreak).

These are two distinct problems:

• contents allowed inside light admonitions
• places where light admonitions are usable

For the latter, we can probably let the Sphinx LaTeX writer pass information to the admonition that it should revert to the "lightbox" environment.

For the former, I need to see if other things than figure are involved. Then one can think of a LaTeX hack which redefines figure environment inside admontions to fix this.

[jfbu]

As temporary workaround, here is what to add to conf.py

latex_elements = {
    'preamble' : r"""
\renewenvironment{sphinxnote}[1]
  {\begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
\renewenvironment{sphinxhint}[1]
  {\begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
\renewenvironment{sphinximportant}[1]
  {\begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
\renewenvironment{sphinxtip}[1]
  {\begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
\renewenvironment{sphinxseealso}[1]
  {\sphinxcolorlet{spx@notice@bordercolor}{sphinxseealsoBorderColor}%
   \csname spx@notice@border\endcsname=
   \dimexpr\csname spx@seealso@border@top\endcsname\relax
  \begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
\renewenvironment{sphinxtodo}[1]
  {\sphinxcolorlet{spx@notice@bordercolor}{sphinxtodoBorderColor}%
   \csname spx@notice@border\endcsname=
   \dimexpr\csname spx@todo@border@top\endcsname\relax
  \begin{sphinxlightbox}\sphinxstrong{#1} }
  {\end{sphinxlightbox}}
""",
}

edited: s/sealso/seealso at one place

This will revert to pre-#12508 for note, hint, tip, important, and somewhat for todo (which was handled as note formerly) and seealso (which had quasi no mark-up, but lightbox should be ok).

For info, sphinxlightbox is not a box at all. It is simply one horizontal line before and one after... There is no way that I know of in LaTeX to insert a background color limited by the horizontal lines. One can do that only by using a TeX box, which can not split at pagebreak, or use the very delicate framed package (or mdframed or tcolorbox) which will not accept a figure inside their contents. Manual solution is to use a normal graphic inclusion and \captionof macro to insert a caption like figure environment would do. Probably the fix to first half of this issue will be for all "heavybox" rendered admonitions to renewenvironment figure to do that.

[jfbu]

I edited my previous comment which had an annoying type sealso in the initial version (which is the one GitHub mailed out).

[electric-coder]

I always thought of admonitions as top level and certainly not as being usable inside table cells.

@jfbu per reST spec a table cell can contain any body element, taken together with the spec for admonition:

Admonitions

Admonitions (...) can appear anywhere an ordinary body element can.

Tables

Each cell contains zero or more body elements.

Sphinx's HTML builder does an excellent job of allowing this kind of nesting.

[jfbu]

I always thought of admonitions as top level and certainly not as being usable inside table cells.

@jfbu per reST spec a table cell can contain any body element, taken together with the spec for admonition:

Point taken, thanks!

I will take care of this issue during the afternoon (European time) but I have tasks of a professional nature to handle as well.

[jfbu]

The fix for the figure in seealso is much easier than I feared. I will now try to reproduce the admonition in table cell issue.

diff --git a/sphinx/writers/latex.py b/sphinx/writers/latex.py
index 1b2f193ba..2b915f551 100644
--- a/sphinx/writers/latex.py
+++ b/sphinx/writers/latex.py
@@ -306,6 +306,7 @@ class LaTeXTranslator(SphinxTranslator):
         self.in_term = 0
         self.needs_linetrimming = 0
         self.in_minipage = 0
+        # only used by figure inside an admonition
         self.no_latex_floats = 0
         self.first_document = 1
         self.this_is_the_title = 1
@@ -966,11 +967,13 @@ class LaTeXTranslator(SphinxTranslator):
     def visit_seealso(self, node: Element) -> None:
         self.body.append(BLANKLINE)
         self.body.append(r'\begin{sphinxseealso}{%s:}' % admonitionlabels['seealso'] + CR)
+        self.no_latex_floats +=1
 
     def depart_seealso(self, node: Element) -> None:
         self.body.append(BLANKLINE)
         self.body.append(r'\end{sphinxseealso}')
         self.body.append(BLANKLINE)
+        self.no_latex_floats -=1
 
     def visit_rubric(self, node: nodes.rubric) -> None:
         if len(node) == 1 and node.astext() in ('Footnotes', _('Footnotes')):

[kartben]

I will now try to reproduce the admonition in table cell issue.

https://github.com/zephyrproject-rtos/zephyr/blob/596ef0a519eec584e2c403927543b130a7166a45/doc/develop/tools/clion.rst?plain=1#L137-L171

if that helps :) Thanks!

[jfbu]

I will now try to reproduce the admonition in table cell issue.

https://github.com/zephyrproject-rtos/zephyr/blob/596ef0a519eec584e2c403927543b130a7166a45/doc/develop/tools/clion.rst?plain=1#L137-L171

if that helps :) Thanks!

Thanks a thousand 😃

Turns out the probable fix was also a one-liner, fortunately all architecture was ready.

Here is the complete diff to fix (hopefully) both aspects.

diff --git a/sphinx/ext/todo.py b/sphinx/ext/todo.py
index 94473e7b8..20582d7b0 100644
--- a/sphinx/ext/todo.py
+++ b/sphinx/ext/todo.py
@@ -215,6 +215,7 @@ def latex_visit_todo_node(self: LaTeXTranslator, node: todo_node) -> None:
         title_node = cast(nodes.title, node[0])
         title = texescape.escape(title_node.astext(), self.config.latex_engine)
         self.body.append('%s:}' % title)
+        self.no_latex_floats += 1
         node.pop(0)
     else:
         raise nodes.SkipNode
@@ -222,6 +223,7 @@ def latex_visit_todo_node(self: LaTeXTranslator, node: todo_node) -> None:
 
 def latex_depart_todo_node(self: LaTeXTranslator, node: todo_node) -> None:
     self.body.append('\\end{sphinxtodo}\n')
+    self.no_latex_floats -= 1
 
 
 def setup(app: Sphinx) -> ExtensionMetadata:
diff --git a/sphinx/texinputs/sphinxlatextables.sty b/sphinx/texinputs/sphinxlatextables.sty
index 9f3944a65..54b42cb4d 100644
--- a/sphinx/texinputs/sphinxlatextables.sty
+++ b/sphinx/texinputs/sphinxlatextables.sty
@@ -108,6 +108,7 @@
     \vbox{}% get correct baseline from above
     \LTpre\z@skip\LTpost\z@skip % set to zero longtable's own skips
     \edef\sphinxbaselineskip{\dimexpr\the\dimexpr\baselineskip\relax\relax}%
+    \spx@inframedtrue % message to sphinxheavybox
    }%
 % Compatibility with caption package
 \def\sphinxthelongtablecaptionisattop{%
@@ -121,7 +122,9 @@
 \def\sphinxatlongtableend{\@nobreakfalse % latex3/latex2e#173
     \prevdepth\z@\vskip\sphinxtablepost\relax}%
 % B. Table with tabular or tabulary
-\def\sphinxattablestart{\par\vskip\dimexpr\sphinxtablepre\relax}%
+\def\sphinxattablestart{\par\vskip\dimexpr\sphinxtablepre\relax
+                        \spx@inframedtrue % message to sphinxheavybox
+                        }%
 \let\sphinxattableend\sphinxatlongtableend
 % This is used by tabular and tabulary templates
 \newcommand*\sphinxcapstartof[1]{%
diff --git a/sphinx/writers/latex.py b/sphinx/writers/latex.py
index 1b2f193ba..2b915f551 100644
--- a/sphinx/writers/latex.py
+++ b/sphinx/writers/latex.py
@@ -306,6 +306,7 @@ class LaTeXTranslator(SphinxTranslator):
         self.in_term = 0
         self.needs_linetrimming = 0
         self.in_minipage = 0
+        # only used by figure inside an admonition
         self.no_latex_floats = 0
         self.first_document = 1
         self.this_is_the_title = 1
@@ -966,11 +967,13 @@ class LaTeXTranslator(SphinxTranslator):
     def visit_seealso(self, node: Element) -> None:
         self.body.append(BLANKLINE)
         self.body.append(r'\begin{sphinxseealso}{%s:}' % admonitionlabels['seealso'] + CR)
+        self.no_latex_floats +=1
 
     def depart_seealso(self, node: Element) -> None:
         self.body.append(BLANKLINE)
         self.body.append(r'\end{sphinxseealso}')
         self.body.append(BLANKLINE)
+        self.no_latex_floats -=1
 
     def visit_rubric(self, node: nodes.rubric) -> None:
         if len(node) == 1 and node.astext() in ('Footnotes', _('Footnotes')):

and some sample output:

I will make a PR soon (today) but I have to add tests and I can't right now.

[chrisjsewell]

Easy peasy, lemon squeezy, cheers!

[kartben]

Thanks @jfbu! Given this is a regression from 7.3.x, can we expect a new 7.4.x release incorporating the fix once it's available? Cheers!