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

Make sbox robust and scantokens of its content #2

Closed
wants to merge 1 commit into from
Closed

Make sbox robust and scantokens of its content #2

wants to merge 1 commit into from

Conversation

hmenke
Copy link

@hmenke hmenke commented Nov 16, 2017

The \sbox macro is flawed in two ways.

  1. It performs an assignment but is not protected.

  2. It scans the box content as an argument which fixes catcodes and makes it impossible to box, e.g. tabular material. Compare with this question on TeX.SX: How can a matrix of nodes be saved into a box?

This PR attempts to fix this behaviour while maintaining backwards compatibility (i.e. every code which relied on the old, faulty implementation was probably not behaving correctly anyway).

@davidcarlisle
Copy link
Member

speaking personally but I think this is by design.

I don't think it is like \let or \def where you need protection to stop the tokens being assigned expanding, \sbox\foo{x} may expand to \setbox\foo\hbox but that makes the same assignment.
Although the color groups might possibly be an issue..

The choice to make the {...} a normal macro argument was by design and done consistently in latex2e the same is true of \mbox compared to \hbox and latex \footnote which (unlike plain tex \footnote) takes a macro argument. One can argue about the design choice but if that is changed there would be more extensive changes throughout the format rather than just this, it would be odd if the tikzmatrix worked in \sbox but not \savebox or \fbox or \scalebox...

@blefloch
Copy link
Member

Thanks. My experience of \scantokens is that it is almost never the right tool. Here you get a stray space in a test file because \scantokens ends with an end of the line marker, which becomes a space due to \endlinechar. This can be avoided.

A much bigger problem is that if a package (say, listings) grabs some argument from the user, then changes catcodes to verbatim catcodes, then calls \sbox\foo{user argument}, the user's argument will now be retokenized using the verbatim catcodes.

@eg9
Copy link
Contributor

eg9 commented Nov 16, 2017

  1. LaTeX has fragile macros and \sbox is one of them.

  2. It's a problem of TikZ that in matrices & is made active. Since the issue can happen also in other cases, TikZ provides ampersand replacement. “Fixing“ this in LaTeX would open several other issues.

The lrbox environment is available for that purpose.

@FrankMittelbach
Copy link
Member

Concerning LaTeX's fagile commands, we may wish at some point to have another go over the sources to clear out some of the remaining one when there is no harm in that. As to \scantokens as Bruno said, this is not making things better to let this loose on general user input if you don't control the surroundings. Furthermore LaTeX2e has certain design principles and that is that catcodes get fixed in arguments and code that temper with them will not work in short commands with arguments but only in environments (which is why there is lrbox).

Also, as you can see even your simple change has caused to fail the test suite and that was not because it was wrong input in that test (and who knows how many others would have failed). The code in the kernel is subtle and a huge amount of different documents with different requirements depend on it. We haven't explicitly said (so far) that we will not accept pull requests, but as Joseph hinted in the CONRIBUTING.md draft (this repository isn't acutally live yet) the likelyhood that we can and will accept code is low. So don't be disappointed if we turn this down.

@hmenke hmenke closed this Nov 17, 2017
@hmenke
Copy link
Author

hmenke commented Nov 17, 2017

Even though I do not agree with the fact that \sbox should be fragile, I can totally relate to Bruno's comment about \scantokens. I'm just closing this for now.

@josephwright
Copy link
Member

As always with LaTeX, any change has a risk but making remaining fragile commands robust is probably one that might work out.

The long-term desire for a fixed set of actives remains: it would be best if & was always active, but making the change now in the format would almost certainly cause significant issues.

@josephwright
Copy link
Member

@hmenke Thanks for the PR: setting up the GitHub repo, we did discuss how to handle these, as @FrankMittelbach suggests.

davidcarlisle added a commit that referenced this pull request Apr 8, 2018
davidcarlisle added a commit that referenced this pull request Aug 15, 2020
davidcarlisle added a commit that referenced this pull request Aug 18, 2020
* first draft of default unit code in picture mode

* latexrelease guards

* fix includeinrelease guards

* update for default units

* too much macrocode

* correction to dashbox change

* update for default units rollback

* undefine internal space command from robust versions in rollback code

* update for default units rollback

* picture mode commands in ltboxes

* different length registers for vertical and horizontal

* picture mode test

* update for default units rollback

* update for default units rollback

* updated filehook-006 test mostly from lthooks2 branch

* updated filehook-006 test mostly from lthooks2 branch

* #2 not #1  in picture box update

* spurious < in rollback code

* documentation

* [ci skip] ltnews entry for picture extensions

* [ci skip] change log entry for picture extensions

* document using advance with defaultunitsset

* wording clarification as suggested in review of PR

* Move comment back to matching place in the code (accidentally moved while adding latexrelease guards)

* old syntax version of test 01

* correct 7mm value
@josephwright josephwright mentioned this pull request Sep 11, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants