muzimuzhi / thmtools

New home for LaTeX package bundle thmtools
LaTeX Project Public License v1.3c
16 stars 3 forks source link

Incompatibility with `hyperref`, unknown key in `pdfinfo` value #34

Closed niluxv closed 6 months ago

niluxv commented 1 year ago

thmtools seems to clash with hyperxmp. MWE:

\documentclass{article}

\usepackage{hyperref}
\usepackage{hyperxmp}

\usepackage{amsthm}
\usepackage{thmtools}

\hypersetup{
    pdfinfo={
        CopyrightNotice={Copyright 2023}
    },
}

\begin{document}
    Hello.
\end{document}

errors with

! TeX capacity exceeded, sorry [parameter stack size=20000].
\PE@edefbabel #1#2#3->
                      \begingroup \csname @save@activestrue\endcsname \edef ...

Commenting out \usepackage{thmtools} or removing the CopyrightNotice from \hypersetup solves the issue.

(\PE@edefbabel seems to be a command from the pdfescape package, which is transitively loaded by either hyperref or hyperxmp.)

muzimuzhi commented 1 year ago

This seems to work:

diff --git a/source/thm-kv.dtx b/source/thm-kv.dtx
index 9fb16c4..36e1356 100644
--- a/source/thm-kv.dtx
+++ b/source/thm-kv.dtx
@@ -101,7 +101,7 @@
         \@xa\let\csname ifincsname\@xa\endcsname\csname iftrue\endcsname
         \edef\KVS@temp{\endgroup
 % 2019/12/22 removed dependency on etexcmds package
-          \noexpand\KVS@ProcessorDefault{#1}{\unexpanded{#2}}%
+          \noexpand\KVS@ProcessorDefault{#1}{\unexpanded\expandafter{#2}}%
         }%
         \KVS@temp
     }%

A slightly simplified example.

\documentclass{article}
\usepackage{thmtools}
\usepackage{hyperref}

\makeatletter
    % thm-kv.sty/dtx
    \def\kv@processor@default#1#2{%
      \begingroup
        \csname @safe@activestrue\endcsname
        \@xa\let\csname ifincsname\@xa\endcsname\csname iftrue\endcsname
        \edef\KVS@temp{\endgroup
% 2019/12/22 removed dependency on etexcmds package
          \noexpand\KVS@ProcessorDefault{#1}{\unexpanded\expandafter{#2}}%
        }%
        \KVS@temp
    }%
\makeatother

\hypersetup{
    pdfinfo={
        % any key unknown to pdfinfo
        xxx={Copyright 2023}
    },
}

\begin{document}
    Hello.
\end{document}

Need more time to check if that's the right fix.

muzimuzhi commented 1 year ago

This seems to work:

diff --git a/source/thm-kv.dtx b/source/thm-kv.dtx
index 9fb16c4..36e1356 100644
--- a/source/thm-kv.dtx
+++ b/source/thm-kv.dtx
@@ -101,7 +101,7 @@
         \@xa\let\csname ifincsname\@xa\endcsname\csname iftrue\endcsname
         \edef\KVS@temp{\endgroup
 % 2019/12/22 removed dependency on etexcmds package
-          \noexpand\KVS@ProcessorDefault{#1}{\unexpanded{#2}}%
+          \noexpand\KVS@ProcessorDefault{#1}{\unexpanded\expandafter{#2}}%
         }%
         \KVS@temp
     }%

No it may break things.

muzimuzhi commented 1 year ago

With vanilla kvsetkeys loaded, the key is fully expanded by \edef in \kv@processor@default, and the expanded result is used in \csname ...\endcsname in \KVS@ProcessorDefault.

But thmtools patches both commands to prevent expansion of key. With these patches, given \def\x{mykey}, \kvsetkeys{<family>}{\x=myval} behaves differently from \kvsetkeys{<family>}{mykey=myval}.

\documentclass{article}
\usepackage{keyval, kvsetkeys}
\usepackage{thmtools}

\makeatletter
\define@key{fam}{mykey}{value is \texttt{>>\detokenize{#1}<<}}
\makeatother

\begin{document}
\kvsetkeys{fam}{mykey=myval} % both:    "value is >>myval<<"

\def\x{mykey}
\kvsetkeys{fam}{\x=myval}    % with thmtools:
                             %   Package kvsetkeys Error: Undefined key `\x '.
                             % without: "value is >>myval<<"

\def\y{myval}
\kvsetkeys{fam}{mykey=\y}    % both:    "value is >>\y <<"
\end{document}

Unfortunately, in handling unknown keys passed to pdfinfo, hyperref uses (source lines)

        \kv@parse@normalized{%
          \HyInfo@Key={#2}%
        }{%
          \kv@processor@default{pdfinfo}%
        }%

Here \kv@parse@normalized{<key-val list>}{<processor>} is an inner macro called by \kvsetkeys (after some normalization), \HyInfo@Key holds the unknown key passed to pdfinfo (for example CopyrightNotice) and acts as the key, {#2} acts as the value.

The patch on thmtools side has been added since 2012 (see texlive r26251), and the usage of \kv@parse@normalized{\HyInfo@Key={#2}}{...} on hyperref side has been there since 2010 (see texlive r16720).

Since the author of thmtools, Dr. Ulrich M. Schwarz is inactive, I can't get to know why the patches are needed and how they serves thmtools. IMHO the safest fix is to ask for hyperref to expand \HyInfo@Key before passing it to \kv@parse@normalized.

muzimuzhi commented 1 year ago

IMHO the safest fix is to ask for hyperref to expand \HyInfo@Key before passing it to \kv@parse@normalized.

see https://github.com/latex3/hyperref/issues/275. Meanwhile I'll try to provide a workaround, either short- or long-term from thmtools' side.

niluxv commented 1 year ago

Thank you for looking into it! I found that as a workaround I can enable pdfmanagement-testphase (and disable hyperxmp).

But thmtools patches both commands to prevent expansion of key.

Hm, changing the semantics of commands of a different package like this sounds like asking for compatibility issues.

muzimuzhi commented 1 year ago

Not finally decided yet, but I plan to disable the patches by default, and provide a new package option to turn them on, if any backward compatibility issues are encountered.

mbertucci47 commented 6 months ago

@muzimuzhi Can this be closed? The OP's example no longer produces an error