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

LaTeX: compatibility with caption package #5630

Merged
merged 5 commits into from
Nov 18, 2018

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Nov 13, 2018

This compatibility is mainly re-instored for convenience of user to
style the fonts used for the caption, and also possibly influence the
horizontal position via "width" or "margin" option of caption package
(attention that caption package obeys the document class tacit "twoside"
option, so if left and right margins are not set-up to be the same,
positioning of caption will depend on parity of the page number).

Regarding vertical skips, for captions placed on top (which is the table
templates default and also the literalblockcappos default), this commit
ensures that the vertical spacing separating the caption last baseline
to the top of framing is governed by \sphinxbelowcaptionspace in all
these three cases: tabular(y), longtable, literal blocks; the "skip"
option of caption package is ignored for them.

The "skip" is obeyed for the spacing between an image and its caption,
which currently in Sphinx is always below the image (no customization of
the figure caption vertical placement is currently available).

If literalblockcappos is "b" (captions follow code-blocks), this commit
removes the caption-package added \abovecaptionskip, so that "skip" is
also ignored for code-blocks in this case. This looks better due to the
extra space already added by the framing of the code-block and achieves
same spacing as without caption package (presumably loaded mainly to
style the fonts used for the caption). However in future maybe caption's
package "skip" should be obeyed for "literalblock" caption type.

For testing with caption package I mainly used this:

   'preamble': r'''
\usepackage{caption}
\captionsetup{justification=RaggedRight,
              margin={20pt,20pt},
              labelfont=bf, textfont=it,
              format=hang,
              singlelinecheck=false}
'''

I checked in particular behaviour of captions of code-blocks inside table cells whether

    'sphinxsetup': 'literalblockcappos=b',

is made use of or if the default of top positioning of captions for code-blokcs is obeyed.

I made very quick check that hyperlinks to captions still work with caption package loaded as above.

Feature or Bugfix

  • mainly Bugfix

Fixes #5520

This compatibility is mainly re-instored for convenience of user to
style the fonts used for the caption, and also possibly influence the
horizontal position via "width" or "margin" option of caption package
(attention that caption package obeys the document class tacit "twoside"
option, so if left and right margins are not set-up to be the same,
positioning of caption will depend on parity of the page number).

Regarding vertical skips, for captions placed on top (which is the table
templates default and also the literalblockcappos default), this commit
ensures that the vertical spacing separating the caption last baseline
to the top of framing is governed by \sphinxbelowcaptionspace in all
these three cases: tabular(y), longtable, literal blocks; the "skip"
option of caption package is ignored for them.

The "skip" is obeyed for the spacing between an image and its caption,
which currently in Sphinx is always below the image (no customization of
the figure caption vertical placement is currently available).

If literalblockcappos is "b" (captions follow code-blocks), this commit
removes the caption-package added \abovecaptionskip, so that "skip" is
also ignored for code-blocks in this case. This looks better due to the
extra space already added by the framing of the code-block and achieves
same spacing as without caption package (presumably loaded mainly to
style the fonts used for the caption). However in future maybe caption's
package "skip" should be obeyed for "literalblock" caption type.

Fixes: 5520
@jfbu jfbu added this to the 1.8.3 milestone Nov 13, 2018
@codecov
Copy link

codecov bot commented Nov 13, 2018

Codecov Report

Merging #5630 into 1.8 will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              1.8   #5630      +/-   ##
=========================================
+ Coverage   82.09%   82.1%   +<.01%     
=========================================
  Files         300     300              
  Lines       40102   40102              
  Branches     6192    6192              
=========================================
+ Hits        32923   32924       +1     
  Misses       5803    5803              
+ Partials     1376    1375       -1
Impacted Files Coverage Δ
sphinx/builders/html.py 82.95% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5e4bc0...cdb70a1. Read the comment docs.

@@ -13,7 +13,7 @@
<% if table.caption -%>
\sphinxcapstartof{table}
\sphinxcaption{<%= ''.join(table.caption) %>}<%= labels %>
\sphinxaftercaption
\sphinxaftertopcaption
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This macro was never publicly documented; I changed its name for clarification that it should not be used if caption is moved to after the table body.

% so we do the same for tabular(y) tables (see their templates)
\def\sphinxtablecapwidth{\LTcapwidth}% (default is 4in: max width of caption)
% TODO: add alignment options? but user can employ caption package instead.
\newcommand\sphinxcaption{\@dblarg\spx@caption}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

\sphinxcaption is used in the tabular and tabulary templates, its role is to imitate the longtable caption which has a maximal width (4in per default). The \sphinxcaption allowed possibility of optional argument to use something else than \LTcapwidth, but this was bad because usually optional argument of \caption serves to specify a "short" title for making an entry in the corresponding "\listof<tables, figures>". Notice that \sphinxcaption so far is not known to LaTeX writers (figures) but only used via the table templates or via the sphinx.sty code (sphinxVerbatim environment, \sphinxSetupCaptionForVerbatim) for literal blocks. So it is still non-public internal tool and this change will not modify any existing project.

\noindent\hb@xt@\linewidth{\hss
\vtop{\@tempdima\dimexpr#1\relax
\vtop{\@tempdima\dimexpr\sphinxtablecapwidth\relax
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new undocumented macro only serves to implement functioning of \sphinxcaption for maximal width of captions of tabular/tabulary to be same as longtable, because \sphinxtablecapwidth by default will simply be the longtable variable \LTcapwidth.

{\strut\ignorespaces#2\ifhmode\unskip\@finalstrut\strutbox\fi}%
}\hss}%
\par\prevdepth\dp\strutbox
}%
\def\spx@abovecaptionskip{\abovecaptionskip}
\def\spx@abovecaptionskip{\abovecaptionskip}% for literal blocks captions
Copy link
Contributor Author

@jfbu jfbu Nov 14, 2018

Choose a reason for hiding this comment

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

The title of a literal block caption is set in a minipage (see \sphinxVerbatim@TitleBox) and the insertion of \abovecaptionskip from standard expansion of LaTeX \caption is without effect, (wrong comment) so the sphinxVerbatim environment does the skip itself on top of a caption which is positioned above a code-block (following standard but somewhat faulty LaTeX behaviour which always uses \abovecaptionskip even if caption is on top, although of course it was intended primarily to be used when caption is at bottom; see the caption package documentation for more on this incoherency of LaTeX2e). This \spx@abovecaptionskip is given meaning \sphinxverbatimsmallskipamount inside sphinxVerbatimintable to reduce this spacing without modifying \abovecaptionskip itself (I don't remember why I preferred sphinxVerbatimintable not to give locally new value to \abovecaptionskip).

see #5630 (comment) for corrections to the above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah sorry

and the insertion of \abovecaptionskip from standard expansion of LaTeX \caption is without effect,

this is plain wrong comment. The \abovecaptionskip would work even at start of minipage, but I had forgotten when writing up that comment that by default Sphinx uses not \caption but its own \sphinxcaption inside the sphinxVerbatim@TitleBox, and this sets locally \abovecaptionskip to \sphinxabovecaptionskip which by default is 0pt. So the real reason for \spx@abovecaptionskip is because \abovecaptionskip is set locally to 0pt inside the TitleBox, when \sphinxcaption is made use of. As this is also the case for sphinxVerbatimintable it would have remained without effect to set \abovecaptionskip to another value there locally, because this is overruled by \sphinxcaption (in absence of caption package of course, now).


\newcommand\sphinxaftercaption
%
\newcommand\sphinxaftertopcaption
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained above I have modified name of this non-documented LaTeX variable for clarity of future maintenance.

\sphinxbelowcaptionspace\relax}% this is the vspace we want
}%
{\let\spx@ifcaptionpackage\@secondoftwo}%
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LaTeX code comments explain what this does. This is compatibility layer with caption package: if detected, do not use \sphinxcaption own code at all and slightly adjust corrective measure for longtable because caption package patches longtable code and makes it use an other vertical spacing.

Some other modifications of behaviour in presence of caption package are implemented in the sphinxVerbatim environement and use the \spx@ifcaptionpackage conditional defined here.

\kern\dimexpr-\dp\strutbox+\sphinxbelowcaptionspace
% if no frame (code-blocks inside table cells), remove
% the "verbatimsep" whitespace from the top (better visually)
\ifspx@opt@verbatimwithframe\else-\sphinxverbatimsep\fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This removal of some vertical space in case the literal-block is in a table cell (so Sphinx uses no framing but still uses \sphinxverbatimsep whitespace on the 4 sides) is unrelated to handling caption package, but it improves the PDF, and it was occasion to do so.

% the "verbatimsep" whitespace from the top (better visually)
\ifspx@opt@verbatimwithframe\else-\sphinxverbatimsep\fi
% caption package adds \abovecaptionskip vspace, remove it
\spx@ifcaptionpackage{-\abovecaptionskip}{}\relax}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If caption package is active and caption is on top, it adds \abovecaptionskip below the caption, contrarily to standard LaTeX. We remove it for consistency and then only \sphinxbelowcaptionspace governs for all four: tabular, tabulary, longtable, and literalblock the spacing to frame of object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When one uses caption package it succeeds in achieving same spacing for positioning of a top caption for both table environment (with a tabular) or longtable. But Sphinx does not use table environment for a tabular, I have observed that in such non-table environment, the caption package does not succeed in positioning vertically the caption exactly like it does for longtable. Thus, the necessity remains to keep the \sphinxaftertopcaption macro (especially because parskip package is loaded) and its way of achieving \sphinxbelowcaptionspace vertical spacing. So we also need to achieve this for literal blocks, which is done here.

The situation is thus completely different between the status of \abovecaptionskip (used with caption package between caption and object) and the one of \belowcaptionskip (used on external side if caption package used). In the former case Sphinx needs special corrective measures for achieving identical result with tabular(y), longtable and literal blocks, in the latter case, it does not have to worry.

\fi
\def\@captype{literalblock}%
\capstart
% \sphinxVerbatimTitle must reset color
\setbox\sphinxVerbatim@TitleBox
\hbox{\begin{minipage}{\linewidth}%
% caption package may detect wrongly if top or bottom, so we help it
\spx@ifcaptionpackage
{\caption@setposition{\spx@opt@literalblockcappos}}{}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

inside the minipage, the caption package fails to detect automatically if caption is before or after. As we know exactly which is the case, we inform caption package here. Its version of \caption will thus put \abovecaptionskip (default 10pt) between the caption and the object, and \belowcaptionskip (default 0pt) on the other side.

@@ -4,7 +4,7 @@
\centering
\sphinxcapstartof{table}
\sphinxcaption{caption for table}\label{\detokenize{tabular:id1}}
\sphinxaftercaption
\sphinxaftertopcaption
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I don't think this change of name is breaking. There was no reason to use custom template with \sphinxaftercaption elsewhere: if user has custom table template with caption at bottom, there was no reason to use \sphinxaftercaption at all.

@@ -1049,17 +1072,28 @@
\vskip\spx@abovecaptionskip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If caption package is loaded it will add a \vskip\belowcaptionskip above a top caption, as is the case here. By default \belowcaptionskip is 0pt so this changes nothing. If non-zero, there will be extra vertical space above caption. And for a bottom caption, this extra space will be after it. The same will apply to table captions (because Sphinx then does not use its own \sphinxcaption but the standard \caption as patched by caption package). So the behaviour of captions for tables, figures, and literal-blocks remains coherent even then regarding spacing before a top caption or after a bottom caption, so I decided not to add any "corrective" measure here.

% the "verbatimsep" whitespace from the top (better visually)
\ifspx@opt@verbatimwithframe\else-\sphinxverbatimsep\fi
% caption package adds \abovecaptionskip vspace, remove it
\spx@ifcaptionpackage{-\abovecaptionskip}{}\relax}%
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When one uses caption package it succeeds in achieving same spacing for positioning of a top caption for both table environment (with a tabular) or longtable. But Sphinx does not use table environment for a tabular, I have observed that in such non-table environment, the caption package does not succeed in positioning vertically the caption exactly like it does for longtable. Thus, the necessity remains to keep the \sphinxaftertopcaption macro (especially because parskip package is loaded) and its way of achieving \sphinxbelowcaptionspace vertical spacing. So we also need to achieve this for literal blocks, which is done here.

The situation is thus completely different between the status of \abovecaptionskip (used with caption package between caption and object) and the one of \belowcaptionskip (used on external side if caption package used). In the former case Sphinx needs special corrective measures for achieving identical result with tabular(y), longtable and literal blocks, in the latter case, it does not have to worry.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

I don't know caption package so much. But I trust you :-)

@jfbu
Copy link
Contributor Author

jfbu commented Nov 14, 2018

Thanks @tk0miya! I got verbose in my comments/review as usual, let's say that this could prove useful in some years if everybody at Sphinx has forgotten why the LaTeX code got like that. I too wasn't too familiar with caption package, and in the end I was relieved to confirm little was needed for making it Sphinx compatible (again; but this PR does the additional steps to unify literal-blocks and table looks, which caption package by itself of course does not as it does not know about literal blocks and the special way Sphinx uses to place their captions) (the latter is complicated due to usage of framed.sty: the caption has to be inside, because framed always encourages pagebreaks before and after its framing).

I'll wait a bit in case @jessetan has any comment before merging this. It will still be possible to add in future some \sphinxstylecaptiontext macros for some amount of customizability with no need of extra package. But in my testing caption package works reasonably well, so we can always tell people to use it.

The principle point to improve in Sphinx LaTeX currently regarding captions is placement of figure captions which is not under user control. Tables and literal-blocks are under user control (for tables via the templates).

@jessetan
Copy link
Contributor

This solves my reported incompatibility with the captions package, thanks!
Only difference I see is that for a regular table (not longtable) the distance between the caption and the paragraph preceding it is decreased if the caption package is enabled. See screenshots below (put side by side to see more clearly).

Sphinx 1.8.3:
sphinx

Sphinx 1.8.3 with 'preamble': '\\usepackage{caption}':
caption

@jfbu
Copy link
Contributor Author

jfbu commented Nov 17, 2018

@jessetan thanks, yes you are right. This is due to the fact that caption package typesets the caption below an invisible rule, which disrupts TeX baseline placement algorithm. longtable did that already, and Sphinx took measure to ensure same spacing for normal table and longtable. But as caption modifies the captions of normal table to act more like captions of longtable, we now have a mismatch.

Investigating this, it reminded me that I had not unified spacing to captions between tables and literal-blocks. They never coincided I think, and I sort of remember working hard during various literal-block evolutions to maintain exact legacy spacing of literal blocks. Anyway, this may be occasion to unify literal blocks with tables. Here is minimal example to play with.

Section
=======


Text before |bluearrow|\ |redarrow|

.. list-table:: |showbaseline|

    * - foo
      - foo
    * - foo
      - normal table

Text before |bluearrow|

.. rst-class:: longtable

.. list-table:: |showbaseline|

    * - foo
      - foo
    * - foo
      - longtable

Text before |bluearrow|\ |greenarrow|

.. code-block:: python
   :caption: |showbaseline|

   def foo(x):
       return x

.. |bluearrow| raw:: latex
             
               \textcolor{blue}{\rule{\dimexpr.5\linewidth-1cm-10pt\relax}{1pt}\kern-1pt
               \smash{\rule[-26.4pt]{1pt}{26.4pt}}}

.. |redarrow| raw:: latex
             
               \textcolor{red}{\rule{10pt}{1pt}\kern-1pt
               \smash{\rule[-18.4pt]{1pt}{18.4pt}}}

.. |greenarrow| raw:: latex
             
               \textcolor{green}{\rule{16pt}{1pt}\kern-1pt
               \smash{\rule[-24.4pt]{1pt}{24.4pt}}}

.. |showbaseline| raw:: latex
             
                  \rule{8pt}{1pt}\kern2pt\relax

This image was obtained with caption package:

capture d ecran 2018-11-17 a 12 14 55

The same source without caption would give the same result with the sole difference that the first table caption would match the blue line, not the red line. (as you reported)

The green line indicates exact distance to caption of code-block. It is the same whether or not caption package is loaded. It differs slightly from blue line, but is governed by completely different parameters. I hardcoded in the source above the distances, but I know how they are build up.

I think it could be worthwile to unify Listings with tables (assuming captions are on top), at the same time that the caption package issue is fixed.

By the way caption package mistakendly think that the caption is at bottom of table; if it had thought it was on top, the distance would have been different (shorter). Anyway, it is possible to inform caption package of the caption position and this will be part of fix. The fix will be something like, if caption package is detected, "do for normal tables what we already did for longtables".

Ensure same vertical spaces above caption for tabular(y) and longtable
rendered tables.

Fixes: sphinx-doc#5520
@jfbu
Copy link
Contributor Author

jfbu commented Nov 17, 2018

@jessetan Updated. I decided to only fix #5520 and not unify with captions of literal blocks, this will be done on another occasion.

edit: unfortunately it is still not perfect, there is a slight discrepancy when the caption occupies multiple lines, between table and longtable, with caption package loaded. Looks as if I must dig more into caption package code... If I get time I will do, else I will merge this for 1.8.3. No problem I could see with captions on only one line.

Make sure vertiale spacing is the same for tabular(y)/longtable
also for multi-line captions
@jfbu
Copy link
Contributor Author

jfbu commented Nov 18, 2018

I understood the remaining problem with multi-line captions not behaving exactly same for tabular or longtable if caption package is loaded by user.

I will merge this after testing completes; the vertical spacing above literal blocks as discussed above is not modified, this PR is for #5520 only.

@jfbu jfbu merged commit 9c8c210 into sphinx-doc:1.8 Nov 18, 2018
@jfbu jfbu deleted the 5520_latex_caption_package branch November 18, 2018 09:33
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants