-
Notifications
You must be signed in to change notification settings - Fork 100
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
Rearrangements #2387
Rearrangements #2387
Conversation
… stored as-if 'at'; make mathDefault font use OT1 encoding; make relativeTo also compare encoding; new Font->asFontinfo to return simulated fontinfo for eventually better integration
…otherwise use the cmd itself; similar for \mathchardef
…ialized \mathcodes; removed helpers and ligatures to core Engine pool 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.
Nice progress!
I left comments on the pieces I understand already, and you have my "fingers crossed" for the TeX-native transitional changes I only understand in spirit.
lib/LaTeXML/Common/Font.pm
Outdated
$size *= $scaled if defined $scaled; | ||
$size = 1 unless $size; # Yes, also if 0, "" (from regexp) | ||
$size = $at if defined $at; | ||
$size = $at * $scaled if defined $scaled; |
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.
Maybe the 3 size assignments here can be compacted to avoid edge cases:
$size = (defined $at) ? (defined $scaled ? $at * $scaled : $at) : 1;
Sample test:
sub trysize {
my ($at, $scaled) = @_;
my $size = (defined $at) ? (defined $scaled ? $at * $scaled : $at) : 1;
return $size
}
print trysize(undef, undef),"\n";
print trysize(undef, 0.5),"\n";
print trysize(0.5, undef),"\n";
print trysize(0.5, 0.5),"\n";
# output:
# 1
# 1
# 0.5
# 0.25
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.
That said, the new code will not scale a previously available $size
, it will only scale with respect to the $at
value. Is that change intended?
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.
Essentially a typo, from backing off a change that wasn't ready. The 2nd line should indeed have been ```$size=$size * $scaled if $scaled;``. Probably those lines will disappear, or be handled differently in the next PR, so not worth optimizing. commit on the way...
@@ -266,6 +276,7 @@ sub stringify { | |||
no warnings 'recursion'; | |||
my ($self) = @_; | |||
my ($fam, $ser, $shp, $siz, $col, $bkg, $opa, $enc, $lang, $mstyle, $flags) = @$self; | |||
# !!!!! |
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.
What could be some words to describe that exclamation?
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.
that got added? Hmm...
my ($fam, $ser, $shp, $siz, $col, $bkg, $opa, $enc, $lang, $mstyle, $flags) = @$self; | ||
return { family => $fam, series => $ser, shape => $shp, size => $siz, | ||
color => $col, background => $bkg, opacity => $opa, | ||
encoding => $enc || 'OT1', language => $lang, mathstyle => $mstyle }; } |
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.
$flags
are not returned?
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.
I think they'll only show up in the CSS style of specifying fonts, so shouldn't be needed for "fontinfo", but the whole synthesization idea is a bit up-in-the-air.
@@ -328,6 +349,7 @@ sub relativeTo { | |||
my ($self, $other) = @_; | |||
my ($fam, $ser, $shp, $siz, $col, $bkg, $opa, $enc, $lang, $mstyle, $flags) = @$self; | |||
my ($ofam, $oser, $oshp, $osiz, $ocol, $obkg, $oopa, $oenc, $olang, $omstyle, $oflags) = @$other; | |||
# !!!! |
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.
[2] What could be some words to describe that exclamation?
lib/LaTeXML/Common/Font/Metric.pm
Outdated
@@ -152,12 +153,15 @@ sub process_lig_kern { | |||
$prognum = 256 * $op + $remainder; $firstinstr = 0; | |||
next; } | |||
$next = $$fontmap[$next]; | |||
$next = $$next[0] if ref $next; |
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.
The changes handling dual type between arrayref vs scalar happen a bit often in this subroutine. This will be somewhat painful to port to Rust, but also makes me wonder why both are allowed in the first place.
Something that will be on my plate a little later, jsut noting it here.
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.
Indeed, there's a few such changes that leaked into this PR, but are really about the upcoming one. The idea is for the FontMap to store not only the equivalent Unicode, but some set of properties/attributes (esp. math related ones). Ie. all still up for discussion.
@@ -47,13 +47,17 @@ sub invoke { | |||
my $mathglyph = $$self{mathglyph}; | |||
# A dilemma: If the \chardef were in a style file, you're prefer to revert to the $cs | |||
# but if defined in the document source, better to use \char ###\relax, so it still "works" | |||
if (defined $mathglyph) { # Must be a math char | |||
my $src = $$self{locator} && $$self{locator}->toString; | |||
my $local = $src && $src !~ /\.(?:sty|ltxml|ltxmlc)/; # Dumps currently have undefined src! |
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.
Is .ltxmlc
a new extension? We don't have them in the repository at the moment. If it's dump related, why c
and not d
? Or even better .ltxml.dump
or .dump.ltxml
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.
Exactly for dumps and your naming suggestion sounds good
lib/LaTeXML/Engine/LaTeX.pool.ltxml
Outdated
@@ -4889,6 +4889,7 @@ DefConstructor('\@framebox[Dimension][]{}', | |||
$document->setAttribute($c[0], $k => $v); } } } } | |||
); | |||
|
|||
AssignValue(allocated_boxes => 0); |
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.
Maybe the AssignValue should be 'global'
scoped? Likely identical, until it isn't :>
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.
will do.
# DefLigature(qr{ffi}, "\x{FB03}", fontTest => \&nonTypewriterT1); | ||
# DefLigature(qr{ffl}, "\x{FB04}", fontTest => \&nonTypewriterT1); | ||
|
||
DefLigature(qr{\.\.\.}, "\x{2026}", fontTest => sub { $_[0]->getFamily ne 'typewriter'; }); # ldots |
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 test can also be \&nonTypewriter
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.
Ah the benefits of good modularity! will do
$document->insertElement('ltx:text', $line, class => 'ltx_align_' . $alignment); | ||
$document->insertElement('ltx:break'); } | ||
else { | ||
$document->absorb($line); } |
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.
should this case emit an Info or Warning message? Silently absorbing while dropping the alignment may be worth logging.
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.
not a bad idea...
\outer\def\newwrite{\alloc@7\write\chardef\sixt@@n} | ||
\outer\def\newfam{\alloc@8\fam\chardef\sixt@@n} | ||
\outer\def\newlanguage{\alloc@9\language\chardef\@cclvi} | ||
EoTeX |
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.
Dropping Perl definitions in favor of RawTeX? Scary. Hopefully more reliable, but something to keep an eye on while testing...
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.
Yeah, scary, but really the underlying theme: most of plain.pool.ltxml
could disappear if (some incarnation of) plain.tex
can be read in.
which really was changes to Fontmap that weren't yet ready for inclusion This reverts commit 8144738.
Last commit reverts all the FontMap tweaks that were in |
* Recognize more TeX font names; note that fonts loaded as 'scaled' are stored as-if 'at'; make mathDefault font use OT1 encoding; make relativeTo also compare encoding; new Font->asFontinfo to return simulated fontinfo for eventually better integration * Have \chardef revert to \char only for definitions from main source, otherwise use the cmd itself; similar for \mathchardef * Slightly more robust coding * Corrections to INITEX initializations of mathcode * Corrected \sfcode default * Moved dash & quote ligatures and helpers from plain.pool * Moved cdots,ldots ligatures from plain.pool * Moved alignLine helper from plain.pool * More TeX-like allocation,\newcount, etc; initialized more fonts; initialized \mathcodes; removed helpers and ligatures to core Engine pool files * accommodate change to allocators * Retract two changes that are too early * Fix typo for scaled font * Assign allocated_boxes globally * use the utilities that we've just defined * Give clue when alignment of a line is dropped * Revert "Slightly more robust coding" which really was changes to Fontmap that weren't yet ready for inclusion This reverts commit 8144738.
This PR continues the previous TeX-pool reorg PR in preparation for more extensive refactoring.
\mathcode
,\sfcode