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

Sandbox run 01.2024, fixes for Fatal cases (10) #2309

Merged
merged 10 commits into from
Feb 7, 2024

Conversation

dginev
Copy link
Collaborator

@dginev dginev commented Feb 1, 2024

This PR lifts a first small selection of the unfortunate Fatal regressions in the arXiv sandboxes with the latest master:

  1. Add a guard for \xyoption to avoid loading the same option twice (as inspired by the original sty, which also does so)

  2. allow more recursion in deeply nested argument reads, even if a true infinite loop lurks behind that - the final Fatal printed gets a more informative trace in the genuine loop cases.

  3. add a guard in revertScript to only call ->defined_as on Token objects (and not Tokens).

  4. recognize and guard the advanced use of \multirow where the height param can be a negative or fractional number. Not implemented yet, but won't Fatal anymore.

  5. Fix a surprisingly common regression with xparse and lipsum conflicting (really my hacky expl3 "pool" conflicting with the expl3.sty binding). I believe this was the trickiest issue to detect here. And yes, 1% of sandbox articles included lipsum in a conflicting setting.

    • The minimal example is simply:
    \documentclass{article}
    \usepackage{xparse}
    \usepackage{lipsum}
    \begin{document}
    \end{document}

    I think eventually the expl3 "pool" should disappear entirely, but I will defer that until we start loading latex.ltx raw. I think that would be a better home for that removal.

  6. A connected change outside of this PR is updating the biblatex.sty experimental binding to catch up to the new internals of url and hyperref - by simply not using them and loading hyperref to get a working \href and \url.

  7. guard literal keyword regexes in listings.sty with quotemeta()

  8. guard failed parses from realizeXMNode coming back with a non-node object

  9. ensure import.sty paths work with ZIP inputs, by making requested paths absolute w.r.t the "SOURCEDIRECTORY" parameter (source is unpacked to a temp dir for ZIP).

  10. Recognize math mode in ulem.sty macros.

This concludes round 1 of my debugging of Fatals. I will rerun the sandboxes with the updated commits and circle back with a comment.

@dginev dginev changed the title Sandbox run 01.2024, fixes for Fatal cases (1) Sandbox run 01.2024, fixes for Fatal cases (10) Feb 1, 2024
@dginev
Copy link
Collaborator Author

dginev commented Feb 6, 2024

The PR's impact on our arXiv sandboxes is now documented here:
https://gist.github.com/dginev/abb72c989f4474a3a33300ed23ac6c86

@@ -4453,8 +4454,9 @@ sub revertScript {
$l++; shift(@t); }
while (@t && $l) {
my $t = shift(@t);
if ($t->defined_as(T_BEGIN)) { $l++; }
elsif ($t->defined_as(T_END)) { $l--; } }
if (ref $t eq 'LaTeXML::Core::Token') { # jump over pre-wrapped Tokens, see arXiv:2210.11051
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting that the leading comment ends with "Worry about list of lists". That ->revert would yield a list of both Token and Tokens almost seems like the real bug. Either way, your patch doesn't protect against leading Tokens. It's also unclear that simply skipping over Tokens is the right thing; what if there are unbalanced or leading or trailing braces in there?

I'd think a simpler, though slightly inefficient, approach would be

my @tokens = Tokens($script->revert)->unlist;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Excellent suggestion - and indeed more correct.

Done: Tested with two articles, seems to work well.

Copy link
Owner

@brucemiller brucemiller left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!!

@brucemiller brucemiller merged commit 5fd5c5b into brucemiller:master Feb 7, 2024
13 checks passed
@brucemiller brucemiller deleted the sandboxed-fatal-fixes-2024 branch February 7, 2024 16:34
teepeemm pushed a commit to teepeemm/LaTeXML that referenced this pull request Oct 29, 2024
* process Dimension height param for \parbox

* guard and warn against limitations of multirow binding

* allow more recursion for parameter reads; guard double-loading on \xyoption

* guard revertScript from inner Tokens

* avoid expl3 loading confusion, base lipsum.sty on xparse.sty

* add quotemeta guard for literal regex matches in listings

* guard against realizeXMNode coming back with a non-libxml result

* ensure import.sty works with ZIP inputs, by consulting the sourcedirectory param

* math-aware ulem macros (todo: add XMWrap class to contents)

* flatten Token list in revertScript; thanks @brucemiller
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.

2 participants