Fix mathtext mismatched braces

[devRD]

PR summary

Fixes #26510

PR checklist

• "closes #0000" is in the body of the PR description to link the related issue
• new and changed code is tested
• Plotting related features are demonstrated in an example
• New Features and API Changes are noted with a directive and release note
• Documentation complies with general and docstring guidelines

[ksunden]

We discussed this a bit on GSOC call, the error message is not the most useful, but it is actually pretty hard to get a better one (the group parser just doesn't match without a closing brace, so it kicks it back as wholesale invalid, rather than being specific about invalid because the closing brace is missing)

Current feeling is that erroring is better than not, but if someone has an idea of how to get the parser to give a more informative message, I'd be open to hearing it.

[ksunden]

Actually, this diff may get us a better error message:

diff --git a/lib/matplotlib/_mathtext.py b/lib/matplotlib/_mathtext.py
index 25a825b7b0..de4c25bfe5 100644
--- a/lib/matplotlib/_mathtext.py
+++ b/lib/matplotlib/_mathtext.py
@@ -1894,6 +1894,7 @@ class Parser:
         p.function = csnames("name", self._function_names)
 
         p.group = p.start_group + ZeroOrMore(p.token)("group") + p.end_group
+        p.unclosed_group = p.start_group + ZeroOrMore(p.token)("group") + StringEnd()
 
         p.frac  = cmd(r"\frac", p.required_group("num") + p.required_group("den"))
         p.dfrac = cmd(r"\dfrac", p.required_group("num") + p.required_group("den"))
@@ -1943,6 +1944,7 @@ class Parser:
             p.simple
             | p.auto_delim
             | p.unknown_symbol  # Must be last
+            | p.unclosed_group  # Must be last
         )
 
         p.operatorname = cmd(r"\operatorname", "{" + ZeroOrMore(p.simple)("name") + "}")
@@ -2144,6 +2146,9 @@ class Parser:
     def unknown_symbol(self, s, loc, toks):
         raise ParseFatalException(s, loc, f"Unknown symbol: {toks['name']}")
 
+    def unclosed_group(self, s, loc, toks):
+        raise ParseFatalException(s, len(s), "Expected '}'")
+
     _accent_map = {
         r'hat':            r'\circumflexaccent',
         r'breve':          r'\combiningbreve',
diff --git a/lib/matplotlib/tests/test_mathtext.py b/lib/matplotlib/tests/test_mathtext.py
index d80312495d..5a9e8a8b92 100644
--- a/lib/matplotlib/tests/test_mathtext.py
+++ b/lib/matplotlib/tests/test_mathtext.py
@@ -320,6 +320,7 @@ def test_fontinfo():
         (r'$a^2^2$', r'Double superscript'),
         (r'$a_2_2$', r'Double subscript'),
         (r'$a^2_a^2$', r'Double superscript'),
+        (r'$a = {b$', r"Expected '}'"),
     ],
     ids=[
         'hspace without value',
@@ -347,7 +348,8 @@ def test_fontinfo():
         'unknown symbol',
         'double superscript',
         'double subscript',
-        'super on sub without braces'
+        'super on sub without braces',
+        'unclosed group',
     ]
 )
 def test_mathtext_exceptions(math, msg):

@devRD thoughts?

[devRD]

I think the diff makes sense, we do get a better error message. Added the changes...

[oscargus]

The location of this method is not that logical. I'd suggest moving it down to other group-methods. Otherwise, it looks good!

[oscargus]

Let me rephrase that: I think this should be removed as there is another method below (at a different level, so I wonder if this is used at all?).

[devRD]

I think the method is being utilized because we get the same error message as above.

Re: there is another method below (at a different level - I might be missing something, could you point out which one?

[devRD]

Although not completely sure if this is correct, I think the same behavior is also achieved by redefining the group grammar to:
p.group = p.start_group + ZeroOrMore(p.token)("group") - p.end_group
(The - sign here checks for trailing end_groups("}") and raises an Exception if none is found)

Instead, the error message here would be ParseSyntaxException: Expected end_group, found end of text

[oscargus]

Here is the other, identical, method. Should only require one and I think this is the one.

[devRD]

Yup, I missed that. Thanks!

[oscargus]

Looks good! Not sure if it should go into 3.8 or 3.9 (I'd vote, always, for 3.8...), so I let one of the reviewers closer to the release process tag and merge.