latex3 / mathtools

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

Bug in mathic option, use of inproperly initialised counter #62

Closed daleif closed 2 days ago

daleif commented 2 days ago

A user sent me this MWE

\documentclass{article}
\usepackage{mathtools}
\mathtoolsset{mathic}
\usepackage[left=6.6cm, right=6.6cm]{geometry}
\begin{document}
\textit{A connected graph~\(G\) on \(n\) vertices has no less than \(n-1\) edges.}\par
\textit{A connected graph \(G\) on \(n\) vertices has no less than \(n-1\) edges.}
\end{document}

Screenshot from 2024-10-03 11-49-28

I'll explain (credit to David in chat) in a response

daleif commented 2 days ago

The problem is the following code behind the mathic option

\def\MT_mathic_true: {
  \MH_if_boolean:nF {math_italic_corr}{
    \MH_set_boolean_T:n {math_italic_corr}
    \MH_if_boolean:nTF {robustify}{
      \MH_let:NwN \MT_mathic_redeffer: \DeclareRobustCommand
    }{
      \MH_let:NwN \MT_mathic_redeffer: \renewcommand
    }
    \MH_let:NwN \MT_begin_inlinemath: \(
    %\renewcommand*\({
    \MT_mathic_redeffer:*\({
      \relax\ifmmode\@badmath\else
      \ifhmode
        \MH_if_dim:w \fontdimen\@ne\font>\z@
          \MH_if_dim:w \lastskip>\z@
            \skip@\lastskip\unskip
            \MH_if_num:w \lastpenalty>\z@
              \count@\lastpenalty\unpenalty
            \MH_fi:
            \@@italiccorr
            \MH_if_num:w \count@>\z@ %      <----- bug
              \penalty\count@
            \MH_fi:
            \hskip\skip@
          \MH_else:
            \@@italiccorr
          \MH_fi:
        \MH_fi:
      \fi
      $\fi
    }
  }
}

The penalty in the marked position is inserted just as long as \count@ is non-zero, but the code that we are reacting to never sets the scratch counter if that test fails.

For now my suggested fix is to simply use

   \MH_if_num:w \lastpenalty>\z@
              \count@\lastpenalty\unpenalty
   \MH_else:
              \count@\z@
   \MH_fi:

to ensure that the counter is properly initialised to work with the test after the italic correction.

David notes in chat, that given we assume etex, it will be better to do the test using \ifnum\lastnodetype=13 instead of the \ifnum\lastpenalty>\z@ test. I'll mark that as a comment in the DTX and leave the cleanup to when mathtools is converted into Expl3.