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

Don't define \tcbcounter in \newtcolorbox[auto counter]... #203

Closed
muzimuzhi opened this issue Jan 11, 2023 · 5 comments
Closed

Don't define \tcbcounter in \newtcolorbox[auto counter]... #203

muzimuzhi opened this issue Jan 11, 2023 · 5 comments
Assignees

Comments

@muzimuzhi
Copy link
Contributor

muzimuzhi commented Jan 11, 2023

Ideally, box counter wrappers \tcbcounter and \thetcbcounter should only be (re)defined locally in a tcolorbox.

But when auto counter or use counter=<counter> is used, \tcbcounter is defined and used as a temp macro in \tcb@proc@counter@autoanduse. Hence after a \newtcolorbox[auto counter]..., \tcbcounter is always defined while \thetcbcounter is untouched.

\documentclass{article}
\usepackage{tcolorbox}

\newtcolorbox[auto counter]{mybox}{}
\show\tcbcounter
% > \tcbcounter=macro:
% ->tcb@cnt@mybox.
\show\thetcbcounter
% > \thetcbcounter=undefined.

\begin{document}
\end{document}

This inconsistency misled me, see my answer to this TeX-SX question, revision 4. I first found \thetcbcounter undefined, then tried \tcbcounter. The fact that it's defined by \newtcolorbox and holds the last counter name happen to work for minimal example which contains only one \newtcolorbox.

It would be better to not use \tcbcounter as a temp macro, but either use another (standard) temp macro or just compose the counter name and pass it to \tcb@proc@counter@autoanduse. This will not only help others not to make the same mistakes I did, but also allow different global definitions of \tcbcounter and \thetcbcounter (though not recommended).

Proposal
I find in all the \tcb@proc@counter@xxx (actual macros \tcb@proc@counter will be let to), its #1 is used either as tcb@cnt@#1 or thetcb@cnt@#1 as a whole. Therefore in the following proposal,

  • in \tcb@proc@options@init, \tcb@proc@counter{#2} is changed to \tcb@proc@counter{tcb@cnt@#2},
  • in \tcb@proc@counter@newanduse, \letcs\tcbcounter{tcb@cnt@#1} is removed and \tcbcounter is replaced by #1 where its value is used in place, and
  • in all \tcb@proc@counter@xxx variants, tcb@cnt@#1 and thetcb@cnt@#1 are changed to #1 and the#1, respectively.

GitHub's comparing page provides a cleaner git diff result (sth like --word-diff), see master...muzimuzhi:tcolorbox:dont-define-tcbcounter.

I have a feeling that the maintainer may want to rewrite these in LaTeX3. 😄

Full git diff
diff --git a/tex/latex/tcolorbox/tcolorbox.sty b/tex/latex/tcolorbox/tcolorbox.sty
index 1623eed..6ca100d 100644
--- a/tex/latex/tcolorbox/tcolorbox.sty
+++ b/tex/latex/tcolorbox/tcolorbox.sty
@@ -2077,36 +2077,35 @@
 }
 
 \def\tcb@proc@counter@autoanduse#1{%
-  \letcs\tcbcounter{tcb@cnt@#1}%
   \ifx\kvtcb@new@numberwithin\@empty%
-    \csxdef{the\tcbcounter}{\expandafter\noexpand\kvtcb@new@format{\tcbcounter}}%
+    \csxdef{the#1}{\expandafter\noexpand\kvtcb@new@format{#1}}%
   \else%
-    \@addtoreset{\tcbcounter}{\kvtcb@new@numberwithin}%
-    \csxdef{the\tcbcounter}{\expandafter\noexpand\csname the\kvtcb@new@numberwithin\endcsname .\expandafter\noexpand\kvtcb@new@format{\tcbcounter}}%
+    \@addtoreset{#1}{\kvtcb@new@numberwithin}%
+    \csxdef{the#1}{\expandafter\noexpand\csname the\kvtcb@new@numberwithin\endcsname .\expandafter\noexpand\kvtcb@new@format{#1}}%
   \fi%
   \ifx\kvtcb@new@freestyle\@empty%
   \else%
-    \csxdef{the\tcbcounter}{\kvtcb@new@freestyle}%
+    \csxdef{the#1}{\kvtcb@new@freestyle}%
   \fi%
-  \global\csletcs{thetcb@cnt@#1}{the\tcbcounter}%
-  \appto\tcb@new@colopt{,code={\letcs\tcbcounter{tcb@cnt@#1}\letcs\thetcbcounter{thetcb@cnt@#1}\stepcounter{\tcbcounter}\preto\kvtcb@phantom{\addtocounter{\tcbcounter}{-1}\refstepcounter{\tcbcounter}}}}%
+  \global\csletcs{the#1}{the#1}%
+  \appto\tcb@new@colopt{,code={\letcs\tcbcounter{#1}\letcs\thetcbcounter{the#1}\stepcounter{\tcbcounter}\preto\kvtcb@phantom{\addtocounter{\tcbcounter}{-1}\refstepcounter{\tcbcounter}}}}%
 }
 
 \def\tcb@proc@counter@auto#1{%
-  \newcounter{tcb@cnt@#1}%
-  \csxdef{tcb@cnt@#1}{tcb@cnt@#1}%
+  \newcounter{#1}%
+  \csxdef{#1}{#1}%
   \tcb@proc@counter@autoanduse{#1}%
 }
 
 \def\tcb@proc@counter@use#1{%
-  \csxdef{tcb@cnt@#1}{\kvtcb@new@counter}%
+  \csxdef{#1}{\kvtcb@new@counter}%
   \tcb@proc@counter@autoanduse{#1}%
 }
 
 \def\tcb@proc@counter@from#1{%
-  \csxdef{tcb@cnt@#1}{\kvtcb@new@counter}%
-  \global\csletcs{thetcb@cnt@#1}{the\kvtcb@new@counter}%
-  \appto\tcb@new@colopt{,code={\letcs\tcbcounter{tcb@cnt@#1}\letcs\thetcbcounter{thetcb@cnt@#1}\stepcounter{\tcbcounter}\preto\kvtcb@phantom{\addtocounter{\tcbcounter}{-1}\refstepcounter{\tcbcounter}}}}%
+  \csxdef{#1}{\kvtcb@new@counter}%
+  \global\csletcs{the#1}{the\kvtcb@new@counter}%
+  \appto\tcb@new@colopt{,code={\letcs\tcbcounter{#1}\letcs\thetcbcounter{the#1}\stepcounter{\tcbcounter}\preto\kvtcb@phantom{\addtocounter{\tcbcounter}{-1}\refstepcounter{\tcbcounter}}}}%
 }
 
 \def\tcb@proc@counter@no#1{%
@@ -2115,7 +2114,7 @@
 
 \long\def\tcb@proc@options@init#1#2{%
   \tcbset{new/.cd,reset@new,#1}%
-  \tcb@proc@counter{#2}%
+  \tcb@proc@counter{tcb@cnt@#2}%
   \iftcb@resetcounteronoverlays%
     \ifcsname resetcounteronoverlays\endcsname%
       \ifcsname tcb@cnt@#2\endcsname%
muzimuzhi added a commit to muzimuzhi/tcolorbox that referenced this issue Jan 11, 2023
@T-F-S
Copy link
Owner

T-F-S commented Jan 11, 2023

I just took a quick glance and - as always - your proposal sounds reasonable to me. The next days I'm very busy, but I will look into it with more time next week.

@T-F-S
Copy link
Owner

T-F-S commented Jan 27, 2023

I examined the matter and came to the conclusion that this proposal cannot be taken over as I thought at first glance.

  • \tcbcounter has to be used as macro name, because number freestyle needs \tcbcounter at this point (see number freestyle example from the documentation).
  • tcb@cnt@#1 cannot be shortened to #1 (and called with tcb@cnt@#1 from \tcb@proc@options@init), because with use counter you can drop in an arbitrary counter name.

Nevertheless, I tried to do some replacements of \tcbcounter by tcb@cnt@#1 inside \tcb@proc@counter@autoanduse, but always run into errors. Possibly I overlooked some necessity for \tcbcounter at some further spot or made a mistake. I'm too tired to continue this now, but since your original idea cannot be taken over and the small possible changes are more or less cosmetic with a good chance to insert bugs, I probably will not change the implementation at all. But, at least, rewritting this stuff with LaTeX3, I will keep in mind for a later time.

@muzimuzhi
Copy link
Contributor Author

muzimuzhi commented Jan 27, 2023

  • \tcbcounter has to be used as macro name, because number freestyle needs \tcbcounter at this point (see number freestyle example from the documentation).

Oh I didn't know it's a documented/in-use feature until just now.

Then as a last resort appending \undef\tcbcounter to \tcb@proc@counter@autoanduse to make \tcbcounter only temporarily defined should work. A more careful treatment could be to backup and (re)define at beginning and then restore at the end of \tcb@proc@counter@autoanduse.

\documentclass{article}
\usepackage{tcolorbox}
\usepackage{shortvrb}  \MakeShortVerb\|

\makeatletter
\pretocmd\tcb@proc@counter@autoanduse
  {\let\tcb@tcbcounter@temp=\tcbcounter}
  {}{\PatchFailed}
\apptocmd\tcb@proc@counter@autoanduse
  {\let\tcbcounter=\tcb@tcbcounter@temp}
  {}{\PatchFailed}
\makeatother

\def\tcbcounter{init}
\def\thetcbcounter{the init}

\newtcolorbox[auto counter, number within=section,
  number freestyle={(Q/\noexpand\thesection/\noexpand\Alph{\tcbcounter})}
]{phbox}[2][]{
  colback=yellow!15!white,
  colframe=blue!75!black,
  fonttitle=\bfseries,
  title=Question~\thetcbcounter: #2,#1
}

\begin{document}
\ttfamily
\meaning\tcbcounter   \par
\meaning\thetcbcounter
\normalfont

\section{title}

\begin{phbox}[label={myfreestyle}]{Title with freestyle number}
  This box is automatically numbered with \ref{myfreestyle} on page\pageref{myfreestyle}. Inside the box, the \thetcbcounter\ canalso be referenced by |\thetcbcounter|.The real counter name is \texttt{\tcbcounter}.
\end{phbox}
\end{document}

image

@T-F-S
Copy link
Owner

T-F-S commented Jan 31, 2023

Then as a last resort appending \undef\tcbcounter to \tcb@proc@counter@autoanduse to make \tcbcounter only temporarily defined should work. A more careful treatment could be to backup and (re)define at beginning and then restore at the end of \tcb@proc@counter@autoanduse.

I will do that for the next version.

@T-F-S T-F-S self-assigned this Jan 31, 2023
@T-F-S
Copy link
Owner

T-F-S commented Feb 10, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants