Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incompatibility with hyperref, unknown key in pdfinfo value #34

Closed
niluxv opened this issue Feb 18, 2023 · 7 comments
Closed

Incompatibility with hyperref, unknown key in pdfinfo value #34

niluxv opened this issue Feb 18, 2023 · 7 comments
Labels
bug Something isn't working compatibility

Comments

@niluxv
Copy link

niluxv commented Feb 18, 2023

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 muzimuzhi added bug Something isn't working compatibility labels Feb 18, 2023
@muzimuzhi muzimuzhi changed the title Incompatibility with hyperxmp Incompatibility with hyperref, unknown key in pdfinfo value Feb 18, 2023
@muzimuzhi
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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

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

@niluxv
Copy link
Author

niluxv commented Feb 19, 2023

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
Copy link
Owner

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility
Projects
None yet
Development

No branches or pull requests

3 participants