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.

Sign up for GitHub

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

more correct emulation of \box and \copy primitives #2480

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

more correct emulation of \box and \copy primitives #2480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0145e84
more correct emulation of \box and \copy primitives
dginev Jan 8, 2025
a70a295
explicit use of Clone module
dginev Jan 8, 2025
f564f53
use tex quotes in test
dginev Jan 8, 2025
ecbc892
perldoc for removeValue
dginev Jan 8, 2025
d4285a3
ensure \box does not unbind outer values when used twice in a frame
dginev Jan 9, 2025
8655d3f
idempotent value removal by checking key-exists and a 0 undo value
dginev Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile.PL
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ WriteMakefile(NAME => 'LaTeXML',
PREREQ_PM => {
'Archive::Zip' => 0,
'DB_File' => 0,
'Clone' => 0,
'File::Which' => 0,
'Getopt::Long' => 2.37,
'Image::Size' => 0,
Expand Down
23 changes: 23 additions & 0 deletions lib/LaTeXML/Core/State.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,24 @@ sub shiftValue {
assign_internal($self, 'value', $key, [], 'global') unless $$vtable{$key}[0];
return shift(@{ $$vtable{$key}[0] }); }

sub removeValue {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use this code to implement scope => 'samelevel' assignments (with a better name, if you can find it), which would be equivalent to removeValue when assigning \undef. Should you one day discover another assignment 'in place'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also a possibility. My main worry is that assign_internal is already somewhat entangled, so adding more to it just makes life harder... Will wait for feedback from Bruce.

I enjoy the idea of having a dedicated removal method, for what that's worth. But since the removal isn't global, the name is a bit misleading. It's really UnbindLastValueAssignment, but this is not a Java project...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also a possibility. My main worry is that assign_internal is already somewhat entangled, so adding more to it just makes life harder... Will wait for feedback from Bruce.

I was thinking the opposite: this code is very similar to assign_internal and would be better placed there for ease of maintainance... ?

I am also wondering if removing values has other side effects, e.g. will this make \ifcsname false? That's not really possible in TeX, is it? (This has no real consequence of course since removeValue is only used for the box registers.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is very similar to assign_internal and would be better placed there for ease of maintainance... ?

Only if it can be DRY ("do not repeat yourself"). Otherwise just growing the body of the function will make it more impenetrable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going back to the DRY idea, couldn't this code be folded into the global assignment? If I parse correctly, your new code and global assignment differ by only two lines (last; and $$frame{value}{$key} = 0). So a samelevel scope could be implemented by adding the line last if $samelevel in assign_internal (but it wouldn't remove the value as you do here, of course).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am open to the idea.

Would likely seek a slightly better name ... To fit better with global and local, maybe initial is a good keyword? Fishing for inspiration from CSS initial

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that samelevel is a terrible name... let's think about how one could document the new scope. The assignment in scope samelevel means something like:

  • in the last active scope where $name was assigned a value (so last?)
  • in the scope where $name received its current value/the current definition of $name was made (so current_def or current_value?)
  • in the same scope where $name was assigned the current value (so same?)
  • in the scope where $name was initially assigned the current value (so initial?)

I am not totally convinced by initial. It sounds too similar to global.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the primary purpose we'll use this for is still "removing" or "unsetting" the value (CSS has an "unset" property related to initial which also comes to mind). So having an associated scope=>"unset" could also be intuitive when looked at (+ documented) from the right perspective, if the "initial" is too confusing. The thing that bothers me a little is that such calls really don't take a "value" argument, so unifying the interfaces will be inevitably clunky. So I can also see as reasonable to keep the code paths separate.

my ($self, $key) = @_;
if (my $vvec = $$self{value}{$key}) {
my $value;
# undo ONLY the most recent binding for this value.
my $frame;
my @frames = @{ $$self{undo} };
while (@frames) {
$frame = shift(@frames);
if (exists $$frame{value}{$key}) { # Undo the bindings, if $key was bound in this frame
if (my $n = $$frame{value}{$key}) {
($value) = map { shift(@$vvec) } 1 .. $n; }
$$frame{value}{$key} = 0;
last; }
last if $$frame{_FRAME_LOCK_}; }
return $value; }
return; }

# manage a (global) hash of values
sub lookupMapping {
my ($self, $map, $key) = @_;
Expand Down Expand Up @@ -902,6 +920,11 @@ onto the last binding of C<$name>.
Returns whether the value C<$name> is bound. If C<$frame> is given, check
whether it is bound in the C<$frame>-th frame, with 0 being the top frame.

=item C<< $value = $STATE->removeValue($name); >>

If a value for C<$name> is assigned, remove its binding from the closest local frame,
returning the removed value.

=back

=head2 Category Codes
Expand Down
6 changes: 3 additions & 3 deletions lib/LaTeXML/Engine/TeX_Box.pool.ltxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
package LaTeXML::Package::Pool;
use strict;
use warnings;
use Clone qw(clone);
use LaTeXML::Package;
#use Unicode::Normalize;
#use LaTeXML::Util::Pathname;
Expand Down Expand Up @@ -557,14 +558,13 @@ sub adjustBoxColor_internal {

DefPrimitive('\box Number', sub {
my $box = 'box' . $_[1]->valueOf;
my $stuff = LookupValue($box);
my $stuff = RemoveValue($box);
adjustBoxColor($stuff);
AssignValue($box, undef);
($stuff ? $stuff : List()); });

DefPrimitive('\copy Number', sub {
my $box = 'box' . $_[1]->valueOf;
my $stuff = LookupValue($box);
my $stuff = clone(LookupValue($box));
adjustBoxColor($stuff);
($stuff ? $stuff : List()); });

Expand Down
5 changes: 5 additions & 0 deletions lib/LaTeXML/Package.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ our @EXPORT = (qw(&DefAutoload &DefExpandable
&LookupLCcode &AssignLCcode
&LookupUCcode &AssignUCcode
&LookupDelcode &AssignDelcode
&RemoveValue
),

# Random low-level token or string operations.
Expand Down Expand Up @@ -278,6 +279,10 @@ sub ShiftValue {
my ($name) = @_;
return $STATE->shiftValue($name); }

sub RemoveValue {
my ($name) = @_;
return $STATE->removeValue($name); }

sub LookupMapping {
my ($map, $key) = @_;
return $STATE->lookupMapping($map, $key); }
Expand Down
Binary file modified t/digestion/box.pdf
View file
Open in desktop
Binary file not shown.
49 changes: 49 additions & 0 deletions t/digestion/box.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
\documentclass{article}
\usepackage{xcolor}
\def\testbox#1{%
\ifvoid#1 Box #1 is empty
\else\ifhbox#1 Box #1 is horizontal
Expand Down Expand Up @@ -35,4 +36,52 @@
\def\mybox#1{\hbox{#1}}
\[ a - \lower1ex\mybox{a} \]

A used box is emptied upto its assignment scope.

\newbox\myboxA
\newbox\myboxB
\setbox\myboxA\vbox{A must appear once.}
\setbox\myboxB\vbox{B must appear once.}

\box\myboxA
\box\myboxA

\begingroup
\box\myboxB
\endgroup
\box\myboxB


Copied boxes should be deep clones and localize their changes.


\newbox\myboxC
\setbox\myboxC\vbox{Black, then red, then black.}
\copy\myboxC
{\color{red}\copy\myboxC}
\copy\myboxC


Box use should not unbind ``grandparent'' assignments.


\newbox\myboxD
\setbox\myboxD\vbox{D must appear twice.}
\begingroup
\setbox\myboxD\vbox{D must appear twice.}
\box\myboxD
\endgroup
\box\myboxD

\newbox\myboxE
\setbox\myboxE\vbox{E will trail as outer, once.}
\begingroup
\setbox\myboxE\vbox{E will appear inner, once.}
\box\myboxE
\box\myboxE
\endgroup
group ended.
\box\myboxE
\box\myboxE

\end{document}
40 changes: 40 additions & 0 deletions t/digestion/box.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<?latexml class="article"?>
<?latexml package="xcolor"?>
<?latexml RelaxNGSchema="LaTeXML"?>
<document xmlns="http://dlmf.nist.gov/LaTeXML">
<resource src="LaTeXML.css" type="text/css"/>
Expand Down Expand Up @@ -130,4 +131,43 @@
</Math>
</equation>
</para>
<para xml:id="p12">
<p>A used box is emptied upto its assignment scope.</p>
</para>
<para vattach="bottom" xml:id="p13">
<p>A must appear once.</p>
</para>
<para vattach="bottom" xml:id="p14">
<p>B must appear once.</p>
</para>
<para xml:id="p15">
<p>Copied boxes should be deep clones and localize their changes.</p>
</para>
<para vattach="bottom" xml:id="p16">
<p>Black, then red, then black.</p>
</para>
<para vattach="bottom" xml:id="p17">
<p><text color="#FF0000">Black, then red, then black.</text></p>
</para>
<para vattach="bottom" xml:id="p18">
<p>Black, then red, then black.</p>
</para>
<para xml:id="p19">
<p>Box use should not unbind “grandparent” assignments.</p>
</para>
<para vattach="bottom" xml:id="p20">
<p>D must appear twice.</p>
</para>
<para vattach="bottom" xml:id="p21">
<p>D must appear twice.</p>
</para>
<para vattach="bottom" xml:id="p22">
<p>E will appear inner, once.</p>
</para>
<para xml:id="p23">
<p>group ended.
<inline-block vattach="bottom">
<p>E will trail as outer, once.</p>
</inline-block></p>
</para>
</document>
Loading