verybadcat/CSharpMath

Latex LargeOperator inputs crashing application, for example \int_a^b

Closed this issue · 17 comments

Open Try tab, enter the following latex \int_a^b and you'll get
System.ArgumentNullException: 'LaTeX Symbol name must not be null.
Parameter name: symbolName'

NOTE: i'm trying to fix it myself, just need few days to get familiar with codebase first.

This bug first appeared in ff02d0e, which is not yet published on NuGet.

You will need to update Xamarin.Forms to 4.0.0.425677 before being able to build the Example project.

Better to investigate this commit instead.

nope, the problem was before. The commit just brings usage of MathListBuilder.MathListToString.

Simple test:

    [Fact]
    public void TestListToString() {
      var input = @"\int_a^b";
      var list = MathLists.FromString(input);
      var str = MathListBuilder.MathListToString(list);
      Assert.Equal(input, str);
    }

nope, the problem was before. The commit just brings usage of MathListBuilder.MathListToString.

Simple test:

    [Fact]
    public void TestListToString() {
      var input = @"\int_a^b";
      var list = MathLists.FromString(input);
      var str = MathListBuilder.MathListToString(list);
      Assert.Equal(input, str);
    }

Btw mathlisttostring returns "\int ^b_a" as a result, this is also bug right ?

I think that's just ordering of scripts that don't affect the result.

I think that's just ordering of scripts that don't affect the result.

but test is fails

That's just an example, as the result has the same meaning in LaTeX as the input, the test should be modified to accommodate the result.

Related code is here:

https://github.com/verybadcat/CSharpMath/blob/master/CSharpMath/Atoms/Factories/MathListBuilder.cs#L773-L780

So, if replace order of applying it gives \int _a^b indeed.

And it will fail for \int ^a_b.

yep, but at least it doesn't look odd for those who not so confident with LaTeX :-)

Maybe we can adjust/improve output somehow?

Maybe we can adjust/improve output somehow?

I have an idea how to do that, today later i'll update my pr.

And it will fail for \int ^a_b.

Guys I've fixed this, the only thing i've got rid whitespace to make make TestListToString unittest work. If we do need that whitespaces, then maybe better to keep them, and clean whitespaces on unittests ?

The whitespace is required for cases like \sqrt a where it will become \sqrta when the space is not present, which parses as a single command.

This library is not held together by a strong type system and it definitely shows here. The eventual F# version will be a lot more structured.

This bug is an intersection of a number of problems, so there are several potential lines of attack, including fixing the limits/nolimits data on LargeOperator, expanding the AliasMap, overriding equality to make operators with different data equal, avoiding AliasMap by iterating over a list to find data, or storing TeX names in data.

I think the most efficient fix is to make the smallest, and most direct and obvious correction, which is to avoid labelling all Operators as LargeOperators. We have a problem with operators, these are incorrectly given LargeOperator types and irrelevant data about "limits"/"nolimits", and that irrelevant data is causing problems in the very fragile conversion to latex algorithms.

Actually the overuse of LargeOperators turned out to be unrelated.
The fix was to work around/with AliasDictionary:

- var command = MathAtoms.LatexSymbolNameForAtom(op);
+ var command = MathAtoms.LatexSymbolNameForAtom(new LargeOperator(op.Nucleus, null));

But the other changes in my PR I think are worth taking too as cleanup/correctness.

Agreed. Objective C in iosMath did not have a strong type system and CSharpMath inherited from it.