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

Add error when readBalanced fails to balance at end of Mouth #872

Merged
merged 1 commit into from Oct 2, 2017

Conversation

Projects
None yet
2 participants
@dginev
Collaborator

dginev commented Sep 28, 2017

This was a bit of a shocker in Authorea editing. There were obvious pieces of wrong LaTeX content that never returned an error from latexml, such as:

\textbf{some text here

Importantly, this is being converted in fragment mode, and has no \end{document} in the same mouth to throw a grouping error. This revealed a particular case of unreported errors in LaTeXML:

When a Gullet->readBalanced call does not have a closing brace, the read silently succeeds to slurp all content until the end of the Mouth

This can cause some serious issues if the error is committed early in a large chunk of latex, naturally. So I suggest throwing an informative error, and then proceeding as usual. It is good enough for users to correct their mistake in online editing. Example interface from Authorea:
selection_999 032

Feedback welcome on how exactly to structure the error.

@dginev dginev requested a review from brucemiller Sep 28, 2017

@dginev dginev added bug and removed bug labels Sep 28, 2017

@dginev

This comment has been minimized.

Collaborator

dginev commented Sep 28, 2017

To reproduce the error in the master latexml from the command-line, here is a one-liner:

latexmlc --whatsin=fragment --preload=article.cls 'literal:\textbf{example...'

I would clearly expect this to throw an error, such as the one from this PR

@dginev dginev added the Authorea label Sep 28, 2017

@brucemiller brucemiller merged commit 36bdbad into brucemiller:master Oct 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brucemiller

This comment has been minimized.

Owner

brucemiller commented Oct 2, 2017

Thanks; Actually, I have a nagging doubt that there was a reason to leave w/o error, but can't remember why; perhaps the usually-expected error that follows is a clearer one? Anyway, merge and see what happens :>

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 2, 2017

I was investigating the potential reasons, and the most pertinent bit of logic was the fact some Mouths autoclose and some do not. There is some subtle intention there that I half-understood, but I tested around several conditions and none seemed to print a decent error for this specific case. Maybe it is even subtler!

We'll be informed upon the next arXiv rerun :>

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

I may have found out why it was there. I am now getting a rather unpleasant error when loading elsarticle.cls, on a vanilla empty-ish input:

gullet->readbalanced ran out of input in an unbalanced state.
 /usr/local/share/perl/5.22.1/LaTeXML/Package/elsart_support.sty.ltxml line 236 Input is empty In 
Core::Gullet[@0x30a84f18] /usr/local/share/perl/5.22.1/LaTeXML/Package/elsart_support.sty.ltxml line 
236 <= Core::Definition::Primitive[\newenvir... <= Core::Stomach[@0x30ace348] <= 
Core::Gullet[@0x30a84f18] <= ..

Hm, need a way to keep the error for the good cases, but kick it out of here.

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

Basically it's the technique used internally by RawTeX, I'm amazed we don't have any tests that use that and would fail?

$stomach->getGullet->readingFromMouth(LaTeXML::Core::Mouth->new($text), sub {
@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

Well, while I have connected the error to the piece of code using a new Mouth, I haven't yet connected to the exact reason readBalanced would fail on a perfectly balanced input.

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

Exact RawTeX line with this error is:

RawTeX('\newenvironment{pf}{\begin{@proof}[\proofname]}{\end{@proof}');
@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

One-liner to reproduce:

latexmlc --preamble='literal:\documentclass{elsarticle}' literal:test
@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

Oh my.

After staring here for an unreasonable amount of time... Is there a typo in the RawTeX string? I printed a whole lot of Dumper messages and it now looks as if the ending:

{\end{@proof}

should have an additional ending brace:

{\end{@proof}}

Does this make sense? I can commit that right away.

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

I have verified this is the issue, will push directly to master (since we're talking 1 character)

@dginev

This comment has been minimized.

Collaborator

dginev commented Oct 3, 2017

Done: a090611

Hope you don't mind my first commit to master @brucemiller ! I think this PR may catch some interesting RawTeX oversights now, more convinced it was correct to add this error message.

@brucemiller

This comment has been minimized.

Owner

brucemiller commented Oct 22, 2017

bravo & welcome ! :>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment