latex3 / latex2e

The LaTeX2e kernel
https://www.latex-project.org/
LaTeX Project Public License v1.3c
1.93k stars 267 forks source link

showkeys prints key twice for amsmath equation #118

Open gabor-braun opened 5 years ago

gabor-braun commented 5 years ago

Brief outline of the bug

In the log file, the output of \showlists clearly shows that the label `eq:1' appears twice at exactly the same spot. In the PDF file this exact overprinting is present but not visible, so this is an efficiency problem.

In particlar, the following snipper appears twice in the output of \showlists

..........\OT1/cmtt/m/n/9 e
..........\OT1/cmtt/m/n/9 q
..........\OT1/cmtt/m/n/9 :
..........\OT1/cmtt/m/n/9 1

Minimal example showing the bug

\RequirePackage{latexbug}
\documentclass{article}
\usepackage[leqno]{amsmath}
\usepackage{showkeys}
\begin{document}
\begin{equation}
  \label{eq:1}
  a = b
\end{equation}
\showboxbreadth\maxdimen
\showboxdepth\maxdimen
\showlists
\end{document}

Log file (required) and possibly PDF file

This is pdfTeX, Version 3.14159265-2.6-1.40.19 (TeX Live 2019/dev/Debian) (preloaded format=pdflatex 2018.12.21)  26 JAN 2019 10:53
entering extended mode
 restricted \write18 enabled.
 file:line:error style messages enabled.
 %&-line parsing enabled.
**\input test-showkeys.tex
(./test-showkeys.tex
(/usr/share/texlive/texmf-dist/tex/latex/latexbug/latexbug.sty
Package: latexbug 2017/10/12 v1.0d Bug-classification
)
(/usr/share/texlive/texmf-dist/tex/latex/base/article.cls
Document Class: article 2018/09/03 v1.4i Standard LaTeX document class
(/usr/share/texlive/texmf-dist/tex/latex/base/size10.clo
File: size10.clo 2018/09/03 v1.4i Standard LaTeX file (size option)
)
\c@part=\count80
\c@section=\count81
\c@subsection=\count82
\c@subsubsection=\count83
\c@paragraph=\count84
\c@subparagraph=\count85
\c@figure=\count86
\c@table=\count87
\abovecaptionskip=\skip41
\belowcaptionskip=\skip42
\bibindent=\dimen102
)
(/usr/share/texlive/texmf-dist/tex/latex/amsmath/amsmath.sty
Package: amsmath 2018/12/01 v2.17b AMS math features
\@mathmargin=\skip43

For additional information on amsmath, use the `?' option.
(/usr/share/texlive/texmf-dist/tex/latex/amsmath/amstext.sty
Package: amstext 2000/06/29 v2.01 AMS text

(/usr/share/texlive/texmf-dist/tex/latex/amsmath/amsgen.sty
File: amsgen.sty 1999/11/30 v2.0 generic functions
\@emptytoks=\toks14
\ex@=\dimen103
))
(/usr/share/texlive/texmf-dist/tex/latex/amsmath/amsbsy.sty
Package: amsbsy 1999/11/29 v1.2d Bold Symbols
\pmbraise@=\dimen104
)
(/usr/share/texlive/texmf-dist/tex/latex/amsmath/amsopn.sty
Package: amsopn 2016/03/08 v2.02 operator names
)
\inf@bad=\count88
LaTeX Info: Redefining \frac on input line 223.
\uproot@=\count89
\leftroot@=\count90
LaTeX Info: Redefining \overline on input line 385.
\classnum@=\count91
\DOTSCASE@=\count92
LaTeX Info: Redefining \ldots on input line 482.
LaTeX Info: Redefining \dots on input line 485.
LaTeX Info: Redefining \cdots on input line 606.
\Mathstrutbox@=\box27
\strutbox@=\box28
\big@size=\dimen105
LaTeX Font Info:    Redeclaring font encoding OML on input line 729.
LaTeX Font Info:    Redeclaring font encoding OMS on input line 730.
\macc@depth=\count93
\c@MaxMatrixCols=\count94
\dotsspace@=\muskip10
\c@parentequation=\count95
\dspbrk@lvl=\count96
\tag@help=\toks15
\row@=\count97
\column@=\count98
\maxfields@=\count99
\andhelp@=\toks16
\eqnshift@=\dimen106
\alignsep@=\dimen107
\tagshift@=\dimen108
\tagwidth@=\dimen109
\totwidth@=\dimen110
\lineht@=\dimen111
\@envbody=\toks17
\multlinegap=\skip44
\multlinetaggap=\skip45
\mathdisplay@stack=\toks18
LaTeX Info: Redefining \[ on input line 2844.
LaTeX Info: Redefining \] on input line 2845.
)
(/usr/share/texlive/texmf-dist/tex/latex/tools/showkeys.sty
Package: showkeys 2014/10/28 v3.17 Show cite and label keys (DPC, MH)
)
(./test-showkeys.aux)
\openout1 = `test-showkeys.aux'.

LaTeX Font Info:    Checking defaults for OML/cmm/m/it on input line 5.
LaTeX Font Info:    ... okay on input line 5.
LaTeX Font Info:    Checking defaults for T1/cmr/m/n on input line 5.
LaTeX Font Info:    ... okay on input line 5.
LaTeX Font Info:    Checking defaults for OT1/cmr/m/n on input line 5.
LaTeX Font Info:    ... okay on input line 5.
LaTeX Font Info:    Checking defaults for OMS/cmsy/m/n on input line 5.
LaTeX Font Info:    ... okay on input line 5.
LaTeX Font Info:    Checking defaults for OMX/cmex/m/n on input line 5.
LaTeX Font Info:    ... okay on input line 5.
LaTeX Font Info:    Checking defaults for U/cmr/m/n on input line 5.
LaTeX Font Info:    ... okay on input line 5.

### horizontal mode entered at line 9
spacefactor 1000
### vertical mode entered at line 0
### current page:
\write-{}
\glue(\topskip) 10.0
\hbox(0.0+0.0)x345.0, glue set 330.0fil
.\hbox(0.0+0.0)x15.0
.\penalty 10000
.\glue(\parfillskip) 0.0 plus 1.0fil
.\glue(\rightskip) 0.0
\penalty 10000
\glue(\abovedisplayskip) 10.0 plus 2.0 minus 5.0
\glue(\baselineskip) 4.5
\hbox(7.5+2.5)x183.95538
.\hbox(7.5+2.5)x12.77782, display
..\hbox(0.0+0.0)x0.0, glue set - 36.69977fil
...\glue 0.0 plus 1.0fil minus 1.0fil
...\hbox(0.0+0.0)x25.69977
....\hbox(8.89998+5.4)x25.69977
.....\hbox(14.29997+0.0)x25.69977, shifted 5.4
......\vbox(14.29997+0.0)x25.69977
.......\rule(0.4+0.0)x*
.......\hbox(13.49998+0.0)x25.69977
........\rule(*+*)x0.4
........\vbox(13.49998+0.0)x24.89978
.........\glue 3.0
.........\hbox(5.49998+2.0)x24.89978
..........\kern 3.0
..........\OT1/cmtt/m/n/9 e
..........\OT1/cmtt/m/n/9 q
..........\OT1/cmtt/m/n/9 :
..........\OT1/cmtt/m/n/9 1
..........\kern 3.0
.........\glue 3.0
........\rule(*+*)x0.4
.......\rule(0.4+0.0)x*
...\kern 11.0
..\hbox(0.0+0.0)x0.0, glue set - 36.69977fil
...\glue 0.0 plus 1.0fil minus 1.0fil
...\hbox(0.0+0.0)x25.69977
....\hbox(8.89998+5.4)x25.69977
.....\hbox(14.29997+0.0)x25.69977, shifted 5.4
......\vbox(14.29997+0.0)x25.69977
.......\rule(0.4+0.0)x*
.......\hbox(13.49998+0.0)x25.69977
........\rule(*+*)x0.4
........\vbox(13.49998+0.0)x24.89978
.........\glue 3.0
.........\hbox(5.49998+2.0)x24.89978
..........\kern 3.0
..........\OT1/cmtt/m/n/9 e
..........\OT1/cmtt/m/n/9 q
..........\OT1/cmtt/m/n/9 :
..........\OT1/cmtt/m/n/9 1
..........\kern 3.0
.........\glue 3.0
........\rule(*+*)x0.4
.......\rule(0.4+0.0)x*
...\kern 11.0
..\hbox(7.5+2.5)x12.77782
...\OT1/cmr/m/n/10 (
...\OT1/cmr/m/n/10 1
...\kern 0.0
...\OT1/cmr/m/n/10 )
..\write1{\newlabel{eq:1}{{1}{\thepage }}}
.\kern148.2668
.\hbox(6.94444+0.0)x22.91077, display
..\OML/cmm/m/it/10 a
..\glue(\thickmuskip) 2.77771 plus 2.77771
..\OT1/cmr/m/n/10 =
..\glue(\thickmuskip) 2.77771 plus 2.77771
..\OML/cmm/m/it/10 b
\penalty 0
\glue(\belowdisplayskip) 10.0 plus 2.0 minus 5.0
total height 44.5 plus 4.0 minus 10.0
 goal height 550.0
prevdepth 2.5, prevgraf 4 lines

./test-showkeys.tex:12: OK.
l.12 \showlists

[1

{/var/lib/texmf/fonts/map/pdftex/updmap/pdftex.map}] (./test-showkeys.aux) ) 
Here is how much of TeX's memory you used:
 1392 strings out of 493817
 20178 string characters out of 6156336
 74306 words of memory out of 5000000
 5176 multiletter control sequences out of 15000+600000
 4560 words of font info for 18 fonts, out of 8000000 for 9000
 417 hyphenation exceptions out of 8191
 27i,8n,21p,237b,486s stack positions out of 5000i,500n,10000p,200000b,80000s
</u
sr/share/texlive/texmf-dist/fonts/type1/public/amsfonts/cm/cmmi10.pfb></usr/sha
re/texlive/texmf-dist/fonts/type1/public/amsfonts/cm/cmr10.pfb></usr/share/texl
ive/texmf-dist/fonts/type1/public/amsfonts/cm/cmtt9.pfb>
Output written on test-showkeys.pdf (1 page, 23847 bytes).
PDF statistics:
 20 PDF objects out of 1000 (max. 8388607)
 13 compressed objects within 1 object stream
 0 named destinations out of 1000 (max. 500000)
 1 words of extra memory for PDF output out of 10000 (max. 10000000)

Discussion

Some macro definitions valid in the document:

\tagform@:
macro:#1->\ifx \df@label \@empty \SK@lab@relax \else \expandafter \SK@@label \meaning \df@label \SK@ \fi \llap {\SK@lab \kern \marginparsep }\SK@lab@relax \SK@tagform@ {#1}

\SK@tagform@:
macro:#1->\maketag@@@ {(\ignorespaces #1\unskip \@@italiccorr )}

\maketag@@@:
macro:#1->\ifx \df@label \@empty \SK@lab@relax \else \expandafter \SK@@label \meaning \df@label \SK@ \fi \llap {\SK@lab \kern \marginparsep }\SK@lab@relax \SK@maketag@@@ {#1}

\SK@maketag@@@:
macro:#1->\hbox {\m@th \normalfont #1}

It seems that \tagform@ prints the key twice: once directly via \llap{...} and once indirectly via the call chain \SK@tagform@ -> \maketag@@@ -> \llap.

Maybe showkeys shouldn't modify \tagform@ at all but only \maketag@@@.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity.

blefloch commented 3 years ago

(Thoughts welcome.)

Origin of bug.

When amsmath is loaded, the showkeys package patches it in two places to insert the equation label: \maketag@@@ and \tagform@, which are called by amsmath's \tag* and \tag respectively (and \thetag and automatically to label equations). Since \tagform@ itself calls \maketag@@@, the showkeys patch is run twice. Without the leqno option, the code is schematically

\def\tagform@#1{%
  Define \SK@lab
  \SK@tagform@{#1}% (amsmath's \tagform@ which calls \maketag@@@)
  Typeset \SK@lab and empty it}

\def\maketag@@@#1{%
  Define \SK@lab
  \SK@maketag@@@{#1}% (amsmath's \maketag@@@)
  Typeset \SK@lab and empty it}

This means that \SK@lab is defined twice, then typeset (after amsmath's \maketag@@@) and emptied, then an empty box is typeset the second time. With the leqno option, the order is a bit different so \SK@lab is defined and typeset and defined and typeset a second time.

\def\tagform@#1{%
  Define \SK@lab
  Typeset \SK@lab and empty it
  \SK@tagform@{#1}} % calls \maketag@@@

\def\maketag@@@#1{%
  Define \SK@lab
  Typeset \SK@lab and empty it
  \SK@maketag@@@{#1}}

Proposed solution

Remove the patch on \tagform@ entirely.

I've checked redefinitions in TeXLive 2021 packages, and saw only one non-trivial place (most redefinitions of \tagform@ call \maketag@@@, some set \tagform@ to \@gobble). In amsmath.dtx itself, when \tag* is used inside an equation (or equation*) environment, see \tag@in@display. Then two definitions occur: \alt@tag is defined to change \tagform@ and \df@tag is defined to call \maketag@@@. Elsewhere, the code runs \alt@tag \df@tag in this order: the first redefines \tagform@ to not call \maketag@@@, but then the \df@tag simply calls \maketag@@@. In fact, I cannot see how \alt@tag has any useful effect in this scenario.

Minor unrelated bug?

When amsmath is loaded, the showkeys patch to \@eqnnum (to support eqnarray) does not end up showing any key because it looks for \df@label, which seems like it is not set up by amsmath for this environment.

gabor-braun commented 3 years ago

(Thoughts welcome.)

Origin of bug.

When amsmath is loaded, the showkeys package patches it in two places to insert the equation label: \maketag@@@ and \tagform@, which are called by amsmath's \tag* and \tag respectively (and \thetag and automatically to label equations).

A more accurate and concise description is that these are called when typesetting equation tags, with \maketag@@@ typesetting the raw tag, and \tagform@ typesetting the tag in document convention (adding parentheses by default), internally using \maketag@@@.

In short the informative part of equation tags are always typeset by \maketag@@@, making it a suitable as the only place to add showing the label (except for references like \ref and friends of course):

Proposed solution

Remove the patch on \tagform@ entirely.

For the proposed solution, probably the following snippet in amsmath needs adjusting too, as then redefining \SK@tagform@ would be a no-op (inside \tag@in@aling@a):

\gdef\alt@tag{\def\SK@tagform@{#2\@gobble}%
  \ifx\SK@@label\relax \let\tagform@\SK@tagform@ \fi
}%

It should probably become what it would be without showkeys support in the first place (ignoring backwards compatibility):

\gdef\alt@tag{\def\tagform@{#2\@gobble}%
}%

(most redefinitions of \tagform@ call \maketag@@@, some set \tagform@ to \@gobble).

These fit into the high-level picture above: equation tags are always typeset by \maketag@@@. (The \@gobble variants don't typeset anything.)

In amsmath.dtx itself, when \tag* is used inside an equation (or equation*) environment, see \tag@in@display. Then two definitions occur: \alt@tag is defined to change \tagform@ and \df@tag is defined to call \maketag@@@. Elsewhere, the code runs \alt@tag \df@tag in this order: the first redefines \tagform@ to not call \maketag@@@, but then the \df@tag simply calls \maketag@@@. In fact, I cannot see how \alt@tag has any useful effect in this scenario.

Even here \maketag@@@ is called to typeset the equation tag, so fits in the scheme above.

By the way, I am wondering why \tag@in@display cannot be simplified to

\let\tag@in@display\tag@in@align

as the code of \tag@in@align seems to be independent of the equation environment? Then of course \tag@in@display@a and \alt@tag would no longer be needed (the first one becomes unused, the latter one always empty). (This would also make the above suggested change in \tag@in@aling@a irrelevant.)

Minor unrelated bug?

When amsmath is loaded, the showkeys patch to \@eqnnum (to support eqnarray) does not end up showing any key because it looks for \df@label, which seems like it is not set up by amsmath for this environment.

From showkeys code documentation:

When the AMS packages are loaded showkeys assumes environments work ‘The AMS way’ However eqnarray (unlike equation) is not redefined, so here we need to remove some of the AMS hacks.

 \toks@\expandafter{\eqnarray}
 \edef\eqnarray{\let\noexpand\tagform@\noexpand\SK@tagform@\the\toks@}

I.e. showkeys is trying to restore the old definition of \tagform@, whch fails but forgetting to restore \maketag@@@, too. I.e,, it should do:

 \toks@\expandafter{\eqnarray}
 \edef\eqnarray{%
    \let\noexpand\maketag@@@\noexpand\SK@maketag@@@
   \let\noexpand\tagform@\noexpand\SK@tagform@\the\toks@}

(Obviously, if the above proposal is accepted, \tagform@ shouldn't be restored.)

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity.

nxg commented 2 years ago

For what it's worth, showlabels had the same issue (I wonder if this is because I stole the idea from @davidcarlisle (hello, David, fancy meeting you here!), or simply jumped to the same conclusion myself). I made the suggested change to showlabels (ie, redefining \maketag@@@ and leaving \tagform@ alone) and it does appear as if the duplication has disappeared.

I haven't torture-tested the fix, because looking through amsmath.sty, I end up agreeing with @gabor-braun (to whom many thanks!) that monkeypatching \maketag@@@ should be sufficient here.

Incidentally, the bug is reproducible both with and without the [leqno] option.