josephwright / etoolbox

Tool-box for LaTeX programmers using e-TeX
LaTeX Project Public License v1.3c
41 stars 7 forks source link

\robustify "\," relaxes "\, " #45

Closed lstonys closed 10 months ago

lstonys commented 10 months ago

Could \robustify make unexpandable macro (I need them in aux file or xml (tex4ht)) if it was declared with \DeclareRobustCommand? Now \robustify\, some how relaxes \, which was defined with \DeclareRobustCommand{\,}{...}

Here is NWE:

\documentclass{article}
\usepackage{etoolbox}
\DeclareRobustCommand\,{\tmspace+\thinmuskip{.1667em}} % taken from amsmath

% \expandafter\show \csname , \endcsname             % here \tmspace +\thinmuskip {.1667em}.
\robustify\,
% \expandafter\show \csname , \endcsname              % here \, =\relax.  because of csname but in real it was undefined

\begin{document}
$a\* b$    % HERE you'll get undefined "\, "
\makeatletter
\immediate\write\@mainaux{\string\@namedef{a}{a\,b}} % I want to write it without expansion like it would be defined \protected\def\,...
\makeatother
\end{document}

In older amsmath (from TL2019) \, was defined like \renewcommand{\,}{\tmspace+\thinmuskip{.1667em}}. so \robustify\, was warking ok.

josephwright commented 10 months ago

I don't see an issue here: I get no error and \, is as expected a \protected macro after robustification:

\robustify\,\show\,
...
> \,=\protected macro:
->\tmspace +\thinmuskip {.1667em}.
josephwright commented 10 months ago

Ah, you mean that \thinspace is \let to \,, and that causes an issue? That's not really the fault of etoolbox: you just need to redo the \let.

josephwright commented 10 months ago

(I would use \DeclareCommandCopy)

josephwright commented 10 months ago

I suspect we could do with a kernel adjustment to do

\DeclareCommandCopy\thinspace\,

so that \thinspace is then independent of \, entirely - I will raise with the team.

lstonys commented 10 months ago

Please compile my MWE and you will get error (I edited it) It's ok if I write

\csletcs{my, }{, }
\robustify\,
\csletcs{, }{my, }

Problem is that \robustify undefines that macro with the space \,. If in amsmath whoud be

 \protected\def\,{\tmspace+\thinmuskip{.1667em}}

everything would be fine. But now thay made it like

\DeclareRobustCommand\,{\tmspace+\thinmuskip{.1667em}}
lstonys commented 10 months ago

I suggest to change \DeclareCommandCopy with \pretected defs. It will be better for output files. For example you have to write math content to tagged PDF (tex as a source of math formula) then you'll need to deal with expansion. Sometimes will help \detokenize for everything and sometimes you'll need to expand something but command like \, I don't think will ever be useful expanded.

davidcarlisle commented 10 months ago

we could/should use DeclareCommandCopy but another alternative rather than every other package making sure it never access the internal ...space token of a robust command definition would be for etoolbox not to do

          \csundef{\expandafter\@gobble\string#1 }%

which is presumably there just as an optimisation to reclaim some token space that isn't necessarily safe to reclaim.

Obviously we could also change amsmath to use \protected but that just fixes one case, the etoolbox \robustify is really there for adding \protected to legacy code that doesn't already use it

davidcarlisle commented 10 months ago

Note there is no need to use \robustify\, at all as it is already (2e) robust so would not expand if a \protected@write was used, the example at the start uses a raw \write which is never supported for arbitrary latex input.

u-fischer commented 10 months ago

@davidcarlisle move the @ ;-) \protected@write\@mainaux{}{\string\@namedef{a}{a\,b}}

josephwright commented 10 months ago

we could/should use DeclareCommandCopy but another alternative rather than every other package making sure it never access the internal ...space token of a robust command definition would be for etoolbox not to do

          \csundef{\expandafter\@gobble\string#1 }%

which is presumably there just as an optimisation to reclaim some token space that isn't necessarily safe to reclaim.

I've adjusted the code to undefine \foo[space] (see #9) but the basic idea is there in the version I inherited from Philipp L. There are no code comments, but based on his general approach I think this was not about 'resources' but 'correctness'. Essentially, doing \robustify\foo is meant to result in the same definitions as you'd get from \newrobustcmd\foo, which doesn't require \foo[space] at all. So leaving \foo[space] around is 'wrong' as it could be misinterpreted (if one checks for \foo[space] meaning 'a robust macro').

I note that in the time I've been looking after etoolbox, this is the first time that undefining \foo[space] has come up as an issue ...

josephwright commented 10 months ago

I suggest to change \DeclareCommandCopy with \pretected defs. It will be better for output files. For example you have to write math content to tagged PDF (tex as a source of math formula) then you'll need to deal with expansion. Sometimes will help \detokenize for everything and sometimes you'll need to expand something but command like , I don't think will ever be useful expanded.

I'm not sure what you mean here: \DeclareCommandCopy (from the kernel, not etoolbox) is meant to make 'safe' copies, which means that it should respect the expansion status of the parent command.

lstonys commented 10 months ago

I'm not sure what you mean here: \DeclareCommandCopy

Sorry I meant \DeclareRobustCommand.

mrpiggi commented 10 months ago

Did you try to use \protected@write\@mainaux{\string\@namedef{a}{a\,b}} as suggested by @davidcarlisle instead of \immediate\write\@mainaux{\string\@namedef{a}{a\,b}}?

lstonys commented 10 months ago

Did you try to use \protected@write\@mainaux{\string\@namedef{a}{a\,b}}

no. That was just an example. When I use \robustify I expect that macro will be re-defed with \protected and in \write, \edef and other expandable macros it will stay unexpanded.

josephwright commented 10 months ago

Did you try to use \protected@write@mainaux{\string@namedef{a}{a,b}}

no. That was just an example. When I use \robustify I expect that macro will be re-defed with \protected and in \write, \edef and other expandable macros it will stay unexpanded.

And they are - this is an edge case situation due to a copy being made and a lot of history.

josephwright commented 10 months ago

I'm not sure what you mean here: \DeclareCommandCopy

Sorry I meant \DeclareRobustCommand.

Not a viable change, even if the team wanted to do this - there is a lot of code out there that relies on the nature of LaTeX robust commands, so any change would need to be handled by explicitly making things \protected. (At present, it seems unlikely that will happen for the entirety of the kernel ever.)

davidcarlisle commented 10 months ago

So while we could change the kernel, a reasonable position would be that this is user-error and after redefining \, you need to re-do any \let that used the old definition.

ie this works

\documentclass{article}
\usepackage{etoolbox}
\DeclareRobustCommand\,{\tmspace+\thinmuskip{.1667em}} % taken from amsmath

% \expandafter\show \csname , \endcsname             % here \tmspace +\thinmuskip {.1667em}.
\robustify\,
\let\thinspace\,
% \expandafter\show \csname , \endcsname              % here \, =\relax.  because of csname but in real it was undefined

\begin{document}
$a\* b$    % HERE you'll get undefined "\, "
\makeatletter
\immediate\write\@mainaux{\string\@namedef{a}{a\,b}} % I want to write it without expansion like it would be defined \protected\def\,...
\makeatother
\end{document}

(or with \RenewCommandCopy instead of \let)

lstonys commented 10 months ago

what if \robustify instead of undefining "command with a space"

\csundef{\expandafter\@gobble\string#1 }%

would assign it to protected variant something like

\csletcs{foo }{foo}%

Then both would be equal Or maybe it doesn't need to undefine this at all.

davidcarlisle commented 10 months ago

Or maybe it doesn't need to undefine this at all.

As I commented above, it certainly doesn't need to although it seems the current undefinition is by design rather than a bug as if the definition had been "originally" \protected then the -space token would never have been defined and leaving it around "by accident" has other risks. So I think the current intended behaviour is that you need to re-do the \let. Especially if we change the format to use NewCommandCopy when defining \thinspace so the problem does not arise here anyway for this example, just documenting that the redefinition may be needed is probably enough?

josephwright commented 10 months ago

@davidcarlisle Basically my conclusion - I will look to put something into the docs to explain this edge case.