-
Notifications
You must be signed in to change notification settings - Fork 130
Support calc.sty's sneaking redefinitions #2652
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
base: master
Are you sure you want to change the base?
Changes from all commits
913e94a
3ca840f
140e2dc
39e776d
9619839
22154a6
fc680c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -209,23 +209,33 @@ sub parseParameters { | |
| $p =~ s/^\s+//; $p =~ s/\s+$//; | ||
| while ($p) { | ||
| # Handle possibly nested cases, such as {Number} | ||
| # That can be explicitly defined as {Number}, | ||
| # OR will be handled as readArg, then the content reparsed as Number | ||
| if ($p =~ s/^(\{([^\}]*)\})\s*//) { | ||
| my ($spec, $inner_spec) = ($1, $2); | ||
| my $inner = ($inner_spec ? parseParameters($inner_spec, $for) : undef); | ||
| # If single inner spec is optional, make whole thing optional | ||
| my $opt = $inner && (scalar(@$inner) == 1) && $$inner[0]{optional}; | ||
| push(@params, LaTeXML::Core::Parameter->new('Plain', $spec, extra => [$inner], | ||
| optional => $opt)); } | ||
| if(LookupMapping('PARAMETER_TYPES', $spec)) { # If specially defined with braces? | ||
| push(@params, LaTeXML::Core::Parameter->new($spec, $spec)); } | ||
| else { | ||
| my $inner = ($inner_spec ? parseParameters($inner_spec, $for) : undef); | ||
| # If single inner spec is optional, make whole thing optional | ||
| my $opt = $inner && (scalar(@$inner) == 1) && $$inner[0]{optional}; | ||
| push(@params, LaTeXML::Core::Parameter->new('Plain', $spec, extra => [$inner], | ||
| optional => $opt)); } } | ||
| elsif ($p =~ s/^(\[([^\]]*)\])\s*//) { # Ditto for Optional | ||
| # Optional also can be defined explicitly as [Type] | ||
| # OR read using readOptional, then content (if any) reparsed as Type. | ||
| my ($spec, $inner_spec) = ($1, $2); | ||
| if ($inner_spec =~ /^Default:(.*)$/) { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec, | ||
| extra => [TokenizeInternal($1), undef])); } | ||
| elsif ($inner_spec) { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec, | ||
| extra => [undef, parseParameters($inner_spec, $for)])); } | ||
| if(LookupMapping('PARAMETER_TYPES', $spec)) { # If specially defined with []? | ||
| push(@params, LaTeXML::Core::Parameter->new($spec, $spec, optional => 1)); } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am now starting to get confused about having two ways of specifying these. I recall we also had a prefix keyword Shouldn't we be normalizing towards one or the other? Is there a risk that a lookup for
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, but there's are Lots of the latter |
||
| else { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec)); } } | ||
| if ($inner_spec =~ /^Default:(.*)$/) { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec, | ||
| extra => [TokenizeInternal($1), undef])); } | ||
| elsif ($inner_spec) { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec, | ||
| extra => [undef, parseParameters($inner_spec, $for)])); } | ||
| else { | ||
| push(@params, LaTeXML::Core::Parameter->new('Optional', $spec)); } } } | ||
| elsif ($p =~ s/^((\w*)(:([^\s\{\[]*))?)\s*//) { | ||
| my ($spec, $type, $extra) = ($1, $2, $4); | ||
| my @extra = map { TokenizeInternal($_) } split('\|', $extra || ''); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This use of
readCalcExpressionwill be a namespace headache once it reaches Rust.The
readCalcExpressionsub is only available to execute (in the perl sesne) after the calc package has loaded, but it is already used in the Engine, before any bindings load. Ouch. Luckily there is late binding, but it's not easy to port.Instead, one can have a method defined in Gullet (so always available early) with some generic name describing its behavior, maybe
readArithExpressionor such? And again use it only when calc is loaded. You may want to do this here, or I may just use my comment as a porting guide a bit later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A different perl trick would have been to refactor the Engine to use a named sub for each reader:
and then have
calc.sty.ltxmlre-bind the reader sub:The main benefit of this approach is performance - there is never a need to do a
LookupValueto check if calc is loaded. And since that will happen during every parameter read, I assume it may even be noticeably faster over large numeric files.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, it's pretty sucky all round, and all alternatives suck too.