diff --git a/doc/generic/pgf/CHANGELOG.md b/doc/generic/pgf/CHANGELOG.md index 12bcebb00..837fd5ff1 100644 --- a/doc/generic/pgf/CHANGELOG.md +++ b/doc/generic/pgf/CHANGELOG.md @@ -16,6 +16,18 @@ lot of contributed changes. Thanks to everyone who volunteered their time! `\pgfrevision{,date}` and `\pgfversion{,date}` are synonyms for now, but the revision should eventually gain back its original meaning. However, as of now this is not supported by l3build. +- Many operations in `pgfkeys` used to use `\csname` directly which lets the + given csname become `\relax` in case it wasn't defined. This resulted in some + leakage of accidentally `\relax`ed keys into the global scope. This has been + cleaned up a little. To preserve compatibility macros that used to expand to a + `\relax`ed csname now expand to a primitive `\relax`. This affects the + user-level commands `\pgfkeysgetvalue` and `\pgfkeysgetvalueof`. For the + former the change should not be visible except for the number of expansions + required. For `\pgfkeysgetvalueof`, however, the behavior is manifestly + different in that it will now expand to an alias for the primitive `\relax` in + case the value is undefined instead of a `\relax`ed csname. It has always been + semantically wrong to assign to the result of `\pgfkeysgetvalueof`, but now it + will have undesired side-effects. Therefore this change is breaking. ### Added @@ -60,6 +72,7 @@ lot of contributed changes. Thanks to everyone who volunteered their time! #1112 - Form-only patterns have no specified color #1122 - Make `graphdrawing` work with `name prefix` and `name suffix` options #1087 +- pgfkeys was a bit too relaxed around `\relax` #1132 ### Changed diff --git a/doc/generic/pgf/pgfmanual-en-pgfkeys.tex b/doc/generic/pgf/pgfmanual-en-pgfkeys.tex index 95994059d..80700f932 100644 --- a/doc/generic/pgf/pgfmanual-en-pgfkeys.tex +++ b/doc/generic/pgf/pgfmanual-en-pgfkeys.tex @@ -254,25 +254,24 @@ \subsection{The Key Tree} \end{command} \begin{command}{\pgfkeysvalueof\marg{full key}} - Inserts the value stored in \meta{full key} at the current position into - the text. + Inserts the value stored in \meta{full key} at the current position into the + text. It expands to an alias of the primitive |\relax| if the key is undefined. % \begin{codeexample}[] \pgfkeyssetvalue{/my family/my key}{Hello, world!} \pgfkeysvalueof{/my family/my key} \end{codeexample} % + \emph{Note:} It is an error to assign to the result of the expansion of + |\pgfkeysvalueof|, not only semantically but in recent versions of \pgfname\ + also logically. To set the value of a key \emph{always} use the appropriate + interfaces, e.g.\ |\pgfkeyssetvalue|. \end{command} \begin{command}{\pgfkeysifdefined\marg{full key}\marg{if}\marg{else}} Checks whether this key was previously set using either |\pgfkeyssetvalue| or |\pgfkeyslet|. If so, the code in \meta{if} is executed, otherwise the code in \meta{else}. - - This command will use e\TeX's |\ifcsname| command, if available, for - efficiency. This means, however, that it may behave differently for \TeX\ - and for e\TeX\ when you set keys to |\relax|. For this reason you should - not do so. % \begin{codeexample}[] \pgfkeyssetvalue{/my family/my key}{Hello, world!} @@ -284,7 +283,7 @@ \subsection{The Key Tree} \subsection{Setting Keys} -Settings keys is done using a powerful command called |\pgfkeys|. This command +Setting keys is done using a powerful command called |\pgfkeys|. This command takes a list of so-called \emph{key--value pairs}. These are pairs of the form \meta{key}|=|\meta{value}. The principal idea is the following: For each pair in the list, some \emph{action} is taken. This action can be one of the diff --git a/testfiles/gh-issue-1131.lvt b/testfiles/gh-issue-1131.lvt new file mode 100644 index 000000000..83d8b2228 --- /dev/null +++ b/testfiles/gh-issue-1131.lvt @@ -0,0 +1,36 @@ +\documentclass{minimal} +\input{pgf-regression-test} + +\RequirePackage{pgfkeys} + +\begin{document} + +\START + +\BEGINTEST{pgfkeys: prevent csname from implicitly defining /foo} +\pgfkeysgetvalue{/foo}{\mycmd} +\pgfkeysifdefined{/foo}{\TYPE{true}}{\TYPE{false}} +\ENDTEST + +\BEGINTEST{pgfkeys: keys must be able to hold \relax} + +% essentially \let\csname pgfk@/bar\endcsname\relax +\pgfkeyslet{/bar}{\relax} +\pgfkeys{/bar} +\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}} + +% essentially \def\csname pgfk@/bar\endcsname{\relax} +\pgfkeys{/bar/.initial=\relax} +\pgfkeys{/bar} +\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}} + +\pgfkeyslet{/bar}{\undefined} +\pgfkeysifdefined{/bar}{\TYPE{true}}{\TYPE{false}} +\ENDTEST + +\BEGINTEST{pgfkeys: nice error handling for accidental \relax in /.@cmd} +\csname pgfk@/baz/.@cmd\endcsname +\pgfkeys{/baz} +\ENDTEST + +\END diff --git a/testfiles/gh-issue-1131.tlg b/testfiles/gh-issue-1131.tlg new file mode 100644 index 000000000..4b691e323 --- /dev/null +++ b/testfiles/gh-issue-1131.tlg @@ -0,0 +1,27 @@ +This is a generated file for the l3build validation system. +Don't change this file in any respect. +============================================================ +TEST 1: pgfkeys: prevent csname from implicitly defining /foo +============================================================ +false +============================================================ +============================================================ +TEST 2: pgfkeys: keys must be able to hold \relax +============================================================ +true +true +false +============================================================ +============================================================ +TEST 3: pgfkeys: nice error handling for accidental \relax in /.@cmd +============================================================ +! Package pgfkeys Error: I do not know the key '/baz' and I am going to ignore it. Perhaps you misspelled it. +See the pgfkeys package documentation for explanation. +Type H for immediate help. + ... +l. ...\pgfkeys{/baz} +This error message was generated by an \errmessage +command, so I can't give any explicit help. +Pretend that you're Hercule Poirot: Examine all clues, +and deduce the truth by order and method. +============================================================ diff --git a/tex/generic/pgf/utilities/pgfkeys.code.tex b/tex/generic/pgf/utilities/pgfkeys.code.tex index c0877e22b..397b0e8be 100644 --- a/tex/generic/pgf/utilities/pgfkeys.code.tex +++ b/tex/generic/pgf/utilities/pgfkeys.code.tex @@ -26,6 +26,13 @@ \def\pgfkeys@empty{} \long\def\pgfkeys@firstoftwo#1#2{#1} \long\def\pgfkeys@secondoftwo#1#2{#2} +\long\def\pgfkeys@ifcsname#1{% + \ifcsname#1\endcsname + \expandafter\pgfkeys@firstoftwo + \else + \expandafter\pgfkeys@secondoftwo + \fi +} % This is useful: @@ -145,7 +152,7 @@ % % \pgfkeyslet{/algo/swap}{\myswap} -\def\pgfkeyslet#1#2{% +\long\def\pgfkeyslet#1#2{% \expandafter\let\csname pgfk@#1\endcsname#2% } @@ -163,7 +170,10 @@ % % \pgfkeysgetvalue{/tikz/swap}{\myswap} -\def\pgfkeysgetvalue#1#2{\expandafter\let\expandafter#2\csname pgfk@#1\endcsname} +\long\def\pgfkeysgetvalue#1#2{% + \pgfkeys@ifcsname{pgfk@#1}% + {\expandafter\let\expandafter#2\csname pgfk@#1\endcsname}% + {\let#2\relax}} @@ -180,7 +190,8 @@ % % The length is \pgfkeysvalue{/tikz/length}. -\def\pgfkeysvalueof#1{\csname pgfk@#1\endcsname} +\let\pgfkeys@relax\relax +\long\def\pgfkeysvalueof#1{\csname\pgfkeys@ifcsname{pgfk@#1}{pgfk@#1}{pgfkeys@relax}\endcsname} @@ -379,7 +390,9 @@ \def\pgfkeys@case@one{% \pgfkeysifdefined{\pgfkeyscurrentkey/.@cmd}% {\pgfkeysgetvalue{\pgfkeyscurrentkey/.@cmd}{\pgfkeys@code}% - \expandafter\pgfkeys@code\pgfkeyscurrentvalue\pgfeov} + \ifx\pgfkeys@code\relax\expandafter\pgfkeys@firstoftwo\else\expandafter\pgfkeys@secondoftwo\fi + {\pgfkeys@unknown}% + {\expandafter\pgfkeys@code\pgfkeyscurrentvalue\pgfeov}} {\pgfkeys@case@two}% } @@ -447,20 +460,20 @@ \def\pgfkeys@ifexecutehandler#1#2{#1}% \let\pgfkeys@ifexecutehandler@handleall=\pgfkeys@ifexecutehandler \def\pgfkeys@ifexecutehandler@handleonlyexisting#1#2{% - \ifcsname pgfk@excpt@\pgfkeyscurrentname\endcsname% + \pgfkeys@ifcsname{pgfk@excpt@\pgfkeyscurrentname}{% #1% ok, this particular key handler is known and should be processed in any case (for example .try) - \else + }{% % implement the 'only existing' feature here: \pgfkeysifdefined{\pgfkeyscurrentpath}{#1}{% \pgfkeysifdefined{\pgfkeyscurrentpath/.@cmd}{#1}{#2}% }{}% - \fi% + }% }% \def\pgfkeys@ifexecutehandler@handlefullorexisting#1#2{% \ifpgfkeysaddeddefaultpath - \ifcsname pgfk@excpt@\pgfkeyscurrentname\endcsname% + \pgfkeys@ifcsname{pgfk@excpt@\pgfkeyscurrentname}{% #1% ok, this particular key handler is known and be processed in any case (for example .try) - \else + }{% % implement the 'only existing' feature here: \pgfkeysifdefined{\pgfkeyscurrentpath}{% #1% @@ -471,7 +484,7 @@ #2% }% }% - \fi% + }% \else #1% ok, always true if the USER explicitly provided the full key path. \fi @@ -859,12 +872,12 @@ }% } \def\pgfkeys@handle@boolean#1#2{% - \ifcsname#1#2\endcsname% + \pgfkeys@ifcsname{#1#2}{% \csname#1#2\endcsname% - \else% + }{% \def\pgf@marshal{\pgfkeysvalueof{/errors/boolean expected/.@cmd}}% \expandafter\pgf@marshal\expandafter{\pgfkeyscurrentkey}{#2}\pgfeov% - \fi + }% } \pgfkeys{/handlers/.is choice/.code=% @@ -996,12 +1009,12 @@ \long\def\pgfkeys@exp@call#1{\pgfkeysalso{\pgfkeyscurrentpath={#1}}} \def\pgfkeys@mathparse{% - \ifcsname pgfmathparse\endcsname - \expandafter\pgfmathparse - \else + \pgfkeys@ifcsname{pgfmathparse}{% + \pgfmathparse + }{% \pgfkeys@error{You have to load `pgfmath' to use \string\pgfmathparse}% - \expandafter\def\expandafter\pgfmathresult - \fi + \def\pgfmathresult + }% } \pgfkeys{/handlers/.evaluated/.code=\pgfkeys@mathparse{#1}\expandafter\pgfkeys@exp@call\expandafter{\pgfmathresult}}