fix: add shortcuts to uninstallcommands

[hmenke]

Motivation for this change

Currently shortcuts leak into boxes (most prominently nodes). For normal TikZ commands (\draw, etc.) there is special uninstallation code but not for user-defined shortcuts, so in the following example the \meaning of \specialsauce inside the node text is macro:->\node {specialsauce} even though it should be undefined.

\documentclass{article}
\usepackage{tikz}
\begin{document}
\def\myspecialsauce{\node{specialsauce}}
\tikzaddtikzonlycommandshortcutlet{\specialsauce}{\myspecialsauce}
\begin{tikzpicture}
  \specialsauce;
  \node at (0,1) {\texttt{\meaning\specialsauce}};
\end{tikzpicture}
\end{document}

With this patch in place it is a bit better. Commands that have been defined before will be correctly restored inside the node text, but things that are undefined outside become \relax (by virtue of using \csname). Suggestions to properly propagate undefined are appreciated.

Checklist

Please signoff your commits to explicitly state your agreement to the Developer Certificate of Origin. If that is not possible you may check the boxes below instead:

• Code changes are licensed under GPLv2 + LPPLv1.3c
• Documentation changes are licensed under FDLv1.2

[muzimuzhi]

but things that are undefined outside become \relax (by virtue of using \csname). Suggestions to properly propagate undefined are appreciated.

I pushed an attempt. Now the logic becomes

\def\tikzaddtikzonlycommandshortcutlet#1#2
  if #1 defined
    add to installcommands: store #1 in backup \cs{tikz@orig#1 }, \let#1=#2
    add to uninstallcommands: restore #1 from backup
  else
    add to installcommands: no backup, \let#1=#2
    add to uninstallcommands: ensure #1 is undefined
  fi

[muzimuzhi]

Another attempt. This time \ifdefined is moved inside. Less readable¹ but a bit more efficient when there are many tikzpictures or nodes (because \csname tikz@orig\detokenize{#1} is now pre-expanded to a cs before appended to installcommands/uninstallcommands).

Also added a test file and a changelog entry. Feel free to revert or change any of them.

Footnotes

1. Hope the readability will be improve when latex3 functions are usable in pgf in the future. ↩

[hmenke]

Thank you very much for your input. Some improvements still need to be made but overall this looks very clean.

[hmenke]

This won't work because now \ifdefined is executed at the point where \tikzaddtikzonlycommandshortcutlet is used. However at this point #2 might not be defined yet, so code like this will restore the wrong thing

\tikzaddtikzonlycommandshortcutlet{\oops}{\specialsauce} % <-- \specialsauce not yet defined
\def\specialsauce{tada}

[muzimuzhi]

\ifdefined always acts on #1 so when #2 is defined is irrelevant here and I suppose you want to say

However at this point #1 (not #2) might not be defined yet, so code like this will restore the wrong thing

\tikzaddtikzonlycommandshortcutlet{\oops}{\myspecialsauce} % <-- \oops not yet defined
\def\oops{tada}

Or maybe you noticed in the patch to uninstallcommands, we should check if \tikz@orig#1 is defined.

Anyway I find it suffices to just construct cs from \csname tikz@orig ...\endcsname before adding it to installcommands/uninstallcommands.

[muzimuzhi]

Decided to rename \AssertEquals in CamelCase (for readability when the names of future assertion commands get longer) and move it to pgf-regression-test.tex.

[hmenke]

Another thing that came to my mind is that we should make sure that this nests properly, especially when install and uninstall are not paired (which might happen). Nesting tikzpictures is not really supported but we shouldn't make it more painful than it has to be.

[muzimuzhi]

Another thing that came to my mind is that we should make sure that this nests properly, especially when install and uninstall are not paired (which might happen).

For unpaired use case, do you mean sth like \tikz { \draw ...; \tikz \node...;} instead of \tikz { \draw ...; \node {\tikz \node ...;}; }? That is, a picture directly inside another one, instead of inside node text.

A direct workaround may be

diff --git a/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex b/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
index 6cb246e6..dc0889ed 100644
--- a/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
+++ b/tex/generic/pgf/frontendlayer/tikz/tikz.code.tex
@@ -1727,7 +1727,11 @@
   \let\tikz@atend@picture=\pgfutil@empty%
   \let\tikz@transform=\relax%
   \def\tikz@time{.5}%
-  \tikz@installcommands%
+  \iftikz@inside@picture
+  \else
+    \tikz@installcommands%
+    \tikz@inside@picturetrue
+  \fi
   \scope[every picture,#1]%
   \iftikz@handle@active@code%
     \tikz@switchoff@shorthands%

Are there other cases?