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

User level comands of color and graphics should be robust #208

Closed
FrankMittelbach opened this issue Nov 3, 2019 · 10 comments
Closed

User level comands of color and graphics should be robust #208

FrankMittelbach opened this issue Nov 3, 2019 · 10 comments

Comments

@FrankMittelbach
Copy link
Member

Brief outline of the bug

In color package only \color but nothing else is robust. In particular \normalcolor needs robustifying so that it can be used in declarations that need to do \protected@edef (needed, for example in \emfontdeclare.

Basical declarations that can only appear in weldefined places in the preamble or document class can probably be left alone.

Same for the graphics packages.

@davidcarlisle
Copy link
Member

davidcarlisle commented Nov 3, 2019 via email

@FrankMittelbach
Copy link
Member Author

that was the idea :-) Note that there is already a test file available to track what is already robust and what not

@FrankMittelbach FrankMittelbach moved this from Pool (unscheduled issues) to To do in upcoming LaTeX2e releases Nov 4, 2019
@FrankMittelbach FrankMittelbach added this to the release 2020 spring milestone Nov 4, 2019
@davidcarlisle
Copy link
Member

For color there is the complication that apart from \color itself most commands have a trick #1# definition to short circuit the argument processing, and with \color itself, use without an optional argument isn't directly equivalent to any value of the optional argument so can't declare these with \DeclareRobustCommand optional argument handling (or at least not directly)

the #1#{ declaration saved a few bytes that we needed to save in 1993, If that was the only issue I'd suggest changing it but now I think the best way to make these robust but keep edge cases and 3rd-party extensions as compatible as possible is to keep the existing declarations but then \MakeRobust afterwards. so equivalent to the following , except with the \MakeRobust in the package code.

\documentclass{article}

\usepackage{color}

\typeout{\colorbox{yellow}{zzz}}

\MakeRobust\textcolor % #1#
\MakeRobust\normalcolor% could declare with \DeclareRobustCommand
\MakeRobust\colorbox % #1#
\MakeRobust\fcolorbox % #1#

\typeout{\colorbox{yellow}{zzz}}

\begin{document}

\fcolorbox{red}{blue}{zzz}


\end{document}

@FrankMittelbach FrankMittelbach removed their assignment Nov 21, 2019
@davidcarlisle davidcarlisle added the fixed in dev Fixed in development branch, not in stable release label Nov 24, 2019
@davidcarlisle
Copy link
Member

@FrankMittelbach any thoughts on the comments here 54cf1a9#r36149137 {@jfbu)

@FrankMittelbach
Copy link
Member Author

Looking at it from the perspective of trying to standardize and simplify LaTeX coding I'm not keen on adding something like \color@protectedjust for the sake of the theoretical possibility that there is some package that tests eTeX via \protected being defined.

  1. to my knowledge that is "theoretical" and such a package currently doesn't exist
  2. it can't be a LaTeX package because there eTeX is required and thus always there

So such a test would only make sense in a plain TeX context and there I think your approach of handling it in the wrapper but requiring it for LaTeX is the better approach. After all we require eTeX now for 10+ years.

@jfbu
Copy link

jfbu commented Nov 27, 2019

@FrankMittelbach about my comment I had in mind indeed the tex user, as core LaTeX requires e-TeX I agree using \protected as is makes sense in LaTeX context, and David's remark

[in the] color.tex wrapper for this I'll make sure \protected is \relax if not defined, just while color.sty is input

looks to me as fixing this. And grep -r '\\ifx\\protected' returns only 2 matches in .../texmf-dist/tex/latex

amsaddr/amsaddr.sty:  \ifx\protected@edef\@undefined
texosquery/texosquery.tex:  \ifx\protected@edef\undefined

and actually none in .../tex/generic which was my concern. I know I sometimes raise "theoretical" issues!

@jfbu
Copy link

jfbu commented Nov 27, 2019

ah... there are matches for grep -r '\\ifx.*\\protected'

amsaddr/amsaddr.sty:  \ifx\protected@edef\@undefined
ltxnew/ltxnew.sty:      \else\ifx\x\protected\def\z{\protected}%%3
pgfkeyx/pgfkeyx.sty:    \ifx#2p\expandafter\expandafter\expandafter\protected\fi
pgfkeyx/pgfkeyx.sty:    \ifx#2p\expandafter\expandafter\expandafter\protected\fi
preview/preview.sty:  \ifx\@undefined\protected \else \protected\fi
skeyval/skeyval-core.tex:    \ifx#2p\expandafter\expandafter\expandafter\protected\fi
skeyval/skeyval-core.tex:    \ifx#2p\expandafter\expandafter\expandafter\protected\fi
texosquery/texosquery.tex:  \ifx\protected@edef\undefined

and the preview.sty case might be a real hit (not checked).

davidcarlisle added a commit to davidcarlisle/dpctex that referenced this issue Nov 27, 2019
@FrankMittelbach
Copy link
Member Author

ah... there are matches for grep -r '\\ifx.*\\protected'

ltxnew/ltxnew.sty:      \else\ifx\x\protected\def\z{\protected}%%3
preview/preview.sty:  \ifx\@undefined\protected \else \protected\fi

and the preview.sty case might be a real hit (not checked).

they all are either spurious as in \protected@edef or harmless as far as I can see. Preview would find \protected to be available and then use it. But that would be ok even if it is just a harmless \relax. But anyway in LaTeX all this always gives the same result if we don't change and undefined \protected but simply require that it is there.

@FrankMittelbach
Copy link
Member Author

FrankMittelbach commented Nov 27, 2019 via email

@FrankMittelbach FrankMittelbach moved this from To do to Done in dev in upcoming LaTeX2e releases Nov 29, 2019
@FrankMittelbach
Copy link
Member Author

guess that should be documented in ltnews31 - I made a dummy entry

upcoming LaTeX2e releases automation moved this from Done in dev to Done Jan 31, 2020
@FrankMittelbach FrankMittelbach removed the fixed in dev Fixed in development branch, not in stable release label Jan 31, 2020
davidcarlisle added a commit to davidcarlisle/graphics-pln that referenced this issue Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants