latex3 / mathtools

Mathematical tools to use with amsmath
LaTeX Project Public License v1.3c
57 stars 7 forks source link

\clap and \MT_cramped_internal:Nn should leave vertical mode before typesetting #19

Closed RuixiZhang42 closed 3 years ago

RuixiZhang42 commented 3 years ago
% minimal broken example
\documentclass{article}
\usepackage{mathtools}
\begin{document}
\cramped[\textstyle]{k^2} % <- \cramped CAN be used outside of math mode!
This example exposes the problem:
\verb|\MT_cramped_internal:Nn| doesn't leave vertical mode!
So the text here doesn't follow the $k^2$---%
it actually starts in the second line.
\end{document}
  1. When \MT_cramped_internal:Nn typesets \box\z@, it should leave vertical mode first, i.e., \leavevmode\box\z@, or even \ifvmode\leavevmode\fi\box\z@. Similarly, \clap should be \providecommand*\clap[1]{\leavevmode\hb@xt@\z@{\hss#1\hss}} to match LaTeX kernel’s \llap and \rlap.
  2. Also, the commands \cramped and friends all start with a \ifx conditional. If any of them is used directly in an entry of an alignment, then things could fall apart because of premature expansion. So, before the \ifx\@empty#1\@empty, there should be a \relax. This actually applies to all 7 commands: \mathllap, \mathrlap, \mathclap, \cramped, \crampedllap, \crampedclap, \crampedrlap.
RuixiZhang42 commented 3 years ago

Another problem occurs at https://tex.stackexchange.com/q/574997

mathtools already guards against reboxing by adding an extra {} in front of the box, i.e., \mathXlap and \crampedXlap. So this technique should also apply to \MT_cramped_internal:Nn:

\def\MT_cramped_internal:Nn #1#2{
    ...
    \advance\dimen@-\ht\z@ \ht\z@=-\dimen@
    \ifvmode\leavevmode\fi % <-- be sure to leave vertical mode
    {}\box\z@ % <-- add {} at the front to prevent reboxing issues
}
daleif commented 3 years ago

Is it actually possible to trigger an error with the seven commands in item 2? I haven't been able to find an example that crashes.

daleif commented 3 years ago
1. When `\MT_cramped_internal:Nn` typesets `\box\z@`, it should leave vertical mode first, i.e., `\leavevmode\box\z@`, or even `\ifvmode\leavevmode\fi\box\z@`. Similarly, `\clap` should be `\providecommand*\clap[1]{\leavevmode\hb@xt@\z@{\hss#1\hss}}` to match LaTeX kernel’s `\llap` and `\rlap`.

The \leavevmode and the {} from the other comment, seems to do the trick. I'll add those.

You mention \leavevmode in the kernel \llap and \rlap as far as I can see neither uses \leavevmode

2. Also, the commands `\cramped` and friends all start with a `\ifx` conditional. If any of them is used directly in an entry of an alignment, then things could fall apart because of premature expansion. So, before the `\ifx\@empty#1\@empty`, there should be a `\relax`. This actually applies to all 7 commands: `\mathllap`, `\mathrlap`, `\mathclap`, `\cramped`, `\crampedllap`, `\crampedclap`, `\crampedrlap`.

that seems reasonable as I saw that mentioned in an non-mathtools issue on tex.SE, but it would be interesting to see if one could actually trigger this.

daleif commented 3 years ago

Suggested fix for (1) have been added

daleif commented 3 years ago

and part (2) (which I forgot in the first commit)

daleif commented 3 years ago

Should be done now, though I'd still like to see and example that trigger the premature expansion

RuixiZhang42 commented 3 years ago

@daleif Thanks!

About what I said in (1)—\llap and \rlap using \leavevmode—that was a mistake. Sadly I couldn’t remember where I got that false impression…

For (2), premature expansion may be unobservable after all. My theory is: Since \cramped accepts an optional argument, its implementation probably already guards against premature expansion (done by the LaTeX kernel \providecommand). Still, I think the best practice is to add \relax before a conditional.