-
Notifications
You must be signed in to change notification settings - Fork 101
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
Tex pool reorg #2358
Tex pool reorg #2358
Conversation
…ML-specific support and a binding for plain.tex
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 looks surprisingly well-cataloged! Kudos to David Bausum for the thematic structure.
I generally quite like it. One note is that this kind of PR is the perfect place for subtle typos to sneak in, since git won't micromanage the large chunks of text moving from one file to another, and we can't really inspect it all manually. So I hope copy-paste was bug free :> The tests passing is a big encouragement.
One curious question: Is there an obvious performance footprint from reorganizing? Or are make test
times comparable to master?
lib/LaTeXML/Util/Pathname.pm
Outdated
@@ -433,6 +433,7 @@ sub build_kpse_cache { | |||
foreach my $path (split(/$KPATHSEP/, $texpaths)) { | |||
$path =~ s/^!!//; $path =~ s|//+$|/|; | |||
push(@filters, $path) if -d $path; } | |||
my $filterre = '(?:' . join('|', map { "\Q$_\E"; } @filters) . ')'; |
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.
Edge case note: I think there is a subtle behavior change here if @filters
is empty. The new regex will then evaluate to (?:)
, which apparently matches against every string. So in that case $skip
would be always false, rather than always true. I think?
Edit:
Particularly !grep{ ... } ()
is always true.
While $d !~ /(?:)/
is always false.
…ools; move and rename all pool-level modules
@dginev here's another take, per our discussions; give it another look. |
✅ I like the new naming scheme. Looks good to merge. |
* A slightly more efficient kpse cache scan * Move all pool files to LaTeXML/Engine; have LoadPool look there for pools; move and rename all pool-level modules * Split TeX.pool into modules related to control-sequence family, LaTeXML-specific support and a binding for plain.tex
Split the (huge)
TeX.pool
into more manageable, modular components:plain.tex
.Future (or continuing) work will
\lx@
plain.tex
andlatex.ltx
directly for the bulk of definitions, after establishing what needs LaTeXML-specific binding, eg for semantics.