latex3 / mathtools

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

crampedsubarray increases index height #17

Closed RuixiZhang42 closed 3 years ago

RuixiZhang42 commented 3 years ago

The code for crampedsubarray (which also affects \crampedsubstack) is too simple: It is just \radical\z@{##}. As a result, the height of its content is increased by 5/4 of \fontdimen8\scriptfont3. Here is a MWE illustrating the problem:

\documentclass{article}
\usepackage{mathtools}
\usepackage{graphicx} % for \scalebox
\newcommand\drawbaseline{%
  \leavevmode
  \hbox to 0pt {\hss\vrule width 50pt height 0.1pt depth 0pt }%
}
\begin{document}
\centering
\scalebox{6}%
  {$\displaystyle\sum_{k^2=n}\quad
    \sum_{\crampedsubstack{k^2=n\drawbaseline}}$}
\end{document}

crampedsubstack

Proposed fix

Replace https://github.com/latex3/mathtools/blob/49e7124494eaa39bf2e65fe73fa6b475e5937984/mathtools.dtx#L5998 with

  \MT_cramped_internal:Nn \scriptstyle {##}% <-- changed line

Also replace https://github.com/latex3/mathtools/blob/49e7124494eaa39bf2e65fe73fa6b475e5937984/mathtools.dtx#L4164 with

    \MT_cramped_internal:Nn \scriptstyle {####}% <- a comment sign is missing here as well
RuixiZhang42 commented 3 years ago

Yikes! \MT_cramped_internal:Nn \scriptstyle {##} leads to low-level error Misplaced \cr. Alignment is tricky, so timing expansion is important. The following works:

% for https://github.com/latex3/mathtools/blob/49e7124494eaa39bf2e65fe73fa6b475e5937984/mathtools.dtx#L5998
  \span \MT_cramped_internal:Nn \scriptstyle {##}% <-- changed line

i.e., we expand (once) \MT_cramped_internal:Nn in the alignment preamble.

Similarly, this also works:

% for https://github.com/latex3/mathtools/blob/49e7124494eaa39bf2e65fe73fa6b475e5937984/mathtools.dtx#L4164
    \span \MT_cramped_internal:Nn \scriptstyle {####}% <- comment sign
RuixiZhang42 commented 3 years ago

@daleif I realized I had been using the wrong test code… Here is my exploration for a working solution. First some explanations and then the fix that actually works.

The idea is that in an alignment preamble, the # part should not be passed as an argument to some other commands, because otherwise it can mess up the scanning of a particular alignment entry. My previous test code had

% My test code:
\def\MT_cramped_internal:Nn#1#2{%
  \setbox\z@\hbox{...#1...#2...}% <--- I used \setbox, not \sbox!!!
  <do something with #1>%
}

So when we want to use \MT_cramped_internal:Nn in the preamble of \ialign, we just need one expansion of \MT_cramped_internal:Nn and things work out fine.

But mathtools actually uses \sbox (my oversight, sigh).

% mathtools' implementation:
\def\MT_cramped_internal:Nn#1#2{%
  \sbox\z@{...#1...#2...}% <--- uses \sbox, not \setbox!!!
  <do something with #1>%
}

So we need three (3) expansions to get to \setbox!

  1. Expand \MT_cramped_internal:Nn to get \sbox \z@ {...};
  2. Expand \sbox \z@ {...} to get \protect \sbox<space> \z@ {...} (because \sbox is a robust command, double sighs);
  3. Somehow expand \protect \sbox<space> \z@ {...} to get \protect \setbox \z@ \hbox {...}.

Fix that actually works

We want 3 expansions in the alignment preamble, so we need 3 \span’s. We also want to expand the control sequences that actually need to be expanded, so we need many carefully positioned \expandafter’s to get the timing right.

Here is the code that actually works:

\newenvironment{crampedsubarray}[1]{%
  ...
  \ialign\bgroup\ifx c#1\hfil\fi
%  $\m@th\scriptstyle\kern-\nulldelimiterspace\radical\z@{##}$% <-- commented out
% Expansion madness follows:
  \span\expandafter\span\expandafter\expandafter\expandafter
    \span\expandafter\expandafter\expandafter\expandafter
      \MT_cramped_internal:Nn \scriptstyle {##}% <--- 1+3+4 \expandafter's
  \hfil\crcr%
}{%
  ...
}
  1. The first \span causes the first expansion, which results in \span \expandafter \span \expandafter \expandafter \sbox \z@ {...} ...
  2. Then we have the second expansion, which results in \span \expandafter \protect \sbox<space> \z@ {...} ...
  3. Finally we have the third expansion, pass two arguments to \sbox<space>, and get the desired preamble template \protect \setbox \z@ \hbox {\color@setgroup ...\color@endgroup } ...
davidcarlisle commented 3 years ago

@RuixiZhang42 not had time to follow the code yet sorry (may look over the holiday period) but do you need all the expansion, can't you simply use \setbox\z@\hbox{{...}} ? As long as you have the double group \setbox is safe enough. Actually you don't even need the inner group as there is a $ group already, so \setbox\z@\hbox{$...$} unless I am missing something?

RuixiZhang42 commented 3 years ago

@davidcarlisle

can't you simply use \setbox\z@\hbox{{...}}? […] so \setbox\z@\hbox{$...$} […]

Yes, I can. And I did use \setbox\z@\hbox{$...$} in my original test code. To summarize:

  1. Either change \MT_cramped_internal:Nn to switch to \setbox\z@\hbox{$...$}, then we just need \span \MT_cramped_internal:Nn \scriptstyle {##}% in the alignment preamble;
  2. Or \MT_cramped_internal:Nn can keep \sbox\z@{$...$}, but then we need 3 expansions implemented as \span \@xp \span \@xp \@xp \@xp \span \@xp \@xp \@xp \@xp \MT_cramped_internal:Nn \scriptstyle {##}%, where \@xp is \expandafter (amsgen).

The former approach is clearly the winner IMO. But the latter seems more consistent with mathtools since it uses \sbox everywhere.

davidcarlisle commented 3 years ago

I think I prefer setbox:-) the exact \expandafter and \span combination required otherwise depends on the sbox implementation and would for example have changed when we made sbox robust so this seems rather too delicate.

daleif commented 3 years ago

Thanks for the code suggestions. It will be added shortly (the solution with \span + \MT_cramped_internal:Nn + setbox)

Since I did not write most of the mathtools code. Would it generally be better to use \setbox over \sbox in many of the situations in mathtools?

RuixiZhang42 commented 3 years ago

@daleif According to \sbox’s implementation, its essential purpose is to introduce an extra level of grouping to support color (extra group for a “color stack”).

AFAIU, David’s suggestion is that (in this case) the math mode $’s already create an extra level of grouping, so there is no need for \sbox. As a guideline, I think all code of the form \sbox<number>{$<argument>$} can be replaced with \setbox<number>\hbox{$<argument>$}, which should all be safe.

daleif commented 3 years ago

Super, I'll note that somewhere for later.

This issue should now be fixed