Repository: google-research/arxiv-latex-cleaner

Issue:
ValueError: Unmatched \ifthenelse
Hi, I tried the newest version (1.0.7) and got the following error
```
Traceback (most recent call last):
  File "/opt/miniconda3/bin/arxiv_latex_cleaner", line 5, in <module>
    from arxiv_latex_cleaner.__main__ import __main__
  File "/opt/miniconda3/lib/python3.12/site-packages/arxiv_latex_cleaner/__main__.py", line 201, in <module>
    run_arxiv_cleaner(final_args)
  File "/opt/miniconda3/lib/python3.12/site-packages/arxiv_latex_cleaner/arxiv_latex_cleaner.py", line 721, in run_arxiv_cleaner
    tex_contents[tex_file] = _remove_comments_and_commands_to_delete(
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/miniconda3/lib/python3.12/site-packages/arxiv_latex_cleaner/arxiv_latex_cleaner.py", line 356, in _remove_comments_and_commands_to_delete
    content = _simplify_conditional_blocks(content)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/miniconda3/lib/python3.12/site-packages/arxiv_latex_cleaner/arxiv_latex_cleaner.py", line 235, in _simplify_conditional_blocks
    raise ValueError(f"Unmatched {tree['start'].group()}")
ValueError: Unmatched \ifthenelse
```
However, the version 1.0.6 succeeded. I would expect there is some new bug introduced by the newest version.

Discussion:

Patch:
diff --git a/README.md b/README.md
index 7121204..4d7ee17 100644
--- a/README.md
+++ b/README.md
@@ -126,7 +126,11 @@ usage: arxiv_latex_cleaner@v1.0.7  [-h] [--resize_images] [--im_size IM_SIZE]
                                    [--images_allowlist IMAGES_ALLOWLIST]
                                    [--keep_bib]
                                    [--commands_to_delete COMMANDS_TO_DELETE [COMMANDS_TO_DELETE ...]]
+                                   [--commands_only_to_delete COMMANDS_ONLY_TO_DELETE [COMMANDS_ONLY_TO_DELETE ...]]
+                                   [--environments_to_delete ENVIRONMENTS_TO_DELETE [ENVIRONMENTS_TO_DELETE ...]]
+                                   [--if_exceptions IF_EXCEPTIONS [IF_EXCEPTIONS ...]]
                                    [--use_external_tikz USE_EXTERNAL_TIKZ]
+                                   [--svg_inkscape [SVG_INKSCAPE]]
                                    [--config CONFIG] [--verbose]
                                    input_folder
 
@@ -180,6 +184,15 @@ optional arguments:
                         cannot come immediately after
                         `environments_to_delete`, as the parser does not have
                         any way to know if it's another environment to delete.
+  --if_exceptions IF_EXCEPTIONS [IF_EXCEPTIONS ...]
+                        Constant TeX primitive conditionals (\iffalse, \iftrue,
+                        etc.) are simplified, i.e., true branches are kept, false
+                        branches deleted. To parse the conditional constructs
+                        correctly, all commands starting with `\if` are assumed to
+                        be TeX primitive conditionals (e.g., declared by
+                        \newif\ifvar). Some known exceptions to this rule are
+                        already included (e.g., \iff, \ifthenelse, etc.), but you
+                        can add custom exceptions using `--if_exceptions iffalt`.
   --use_external_tikz USE_EXTERNAL_TIKZ
                         Folder (relative to input folder) containing
                         externalized tikz figures in PDF format.
diff --git a/arxiv_latex_cleaner/__main__.py b/arxiv_latex_cleaner/__main__.py
index 6459eee..90b64e7 100644
--- a/arxiv_latex_cleaner/__main__.py
+++ b/arxiv_latex_cleaner/__main__.py
@@ -135,6 +135,35 @@ PARSER.add_argument(
     ),
 )
 
+def if_prefixed(orig_string):
+  if orig_string.startswith("\\"):
+    string = orig_string[1:]
+  else:
+    string = orig_string
+  if not string.startswith("if"):
+    raise argparse.ArgumentTypeError(
+        f"Expected a string starting with 'if', got '{orig_string}'!"
+    )
+  return string
+
+PARSER.add_argument(
+    "--if_exceptions",
+    nargs="+",
+    default=[],
+    required=False,
+    type=if_prefixed,
+    help=(
+        "Constant TeX primitive conditionals (\\iffalse, \\iftrue, etc.) are "
+        "simplified, i.e., true branches are kept, false branches deleted. "
+        "To parse the conditional constructs correctly, all commands starting "
+        "with `\\if` are assumed to be TeX primitive conditionals (e.g., "
+        "declared by \\newif\\ifvar). Some known exceptions to this rule are "
+        "already included (e.g., \\iff, \\ifthenelse, etc.), but you can add "
+        "custom exceptions using `--if_exceptions iffalt`."
+    ),
+)
+
+
 PARSER.add_argument(
     "--use_external_tikz",
     type=str,
diff --git a/arxiv_latex_cleaner/arxiv_latex_cleaner.py b/arxiv_latex_cleaner/arxiv_latex_cleaner.py
index ca3d2ff..a0ccbc2 100644
--- a/arxiv_latex_cleaner/arxiv_latex_cleaner.py
+++ b/arxiv_latex_cleaner/arxiv_latex_cleaner.py
@@ -177,20 +177,42 @@ def _remove_environment(text, environment):
   )
 
 
-def _simplify_conditional_blocks(text):
+def _simplify_conditional_blocks(text, if_exceptions = []):
   r"""Simplify possibly nested conditional blocks from 'text'.
 
   For example, `\iffalse TEST1\else TEST2\fi` is simplified to `TEST2`,
   and `\iftrue TEST1\else TEST2\fi` is simplified to `TEST1`.
   Unknown conditionals are left untouched.
 
-  Raises a ValueError if an unmatched \if or \else is found.
+  If the conditional tree is malformed, the function will print a warning
+  to stderr and return the original text.
   """
   p = regex.compile(r'(?!(?<=\\newif\s*))\\if\s*(\w+)|\\else(?!\w)|\\fi(?!\w)')
-  toplevel_tree = {'left': [], 'kind': 'toplevel', 'parent': None}
+  toplevel_tree = {'left': [], "right": [], 'kind': 'toplevel', 'parent': None}
 
   tree = toplevel_tree
 
+  exceptions = [
+    # TeX primitives
+    "iff",
+    # package etoolbox
+    "ifpatchable", "ifpatchable*", "ifbool", "iftoggle", "ifdef", "ifcsdef",
+    "ifundef", "ifcsundef", "ifdefmacro", "ifcsmacro", "ifdefparam",
+    "ifcsparam", "ifcsprefix", "ifdefprotected", "ifcsprotected",
+    "ifdefltxprotect", "ifcsltxprotect", "ifdefempty", "ifcsempty",
+    "ifdefvoid", "ifcsvoid", "ifdefequal", "ifcsequal", "ifdefstring",
+    "ifcsstring", "ifdefstrequal", "ifcsstrequal", "ifdefcounter",
+    "ifcscounter", "ifltxcounter", "ifdeflength", "ifcslength", "ifdefdimen",
+    "ifcsdimen", "ifstrequal", "ifstrempty", "ifblank", "ifnumcomp",
+    "ifnumequal", "ifnumodd", "ifdimcomp", "ifdimequal", "ifdimgreater",
+    "ifdimless", "ifboolexpr", "ifboolexpe", "ifinlist", "ifinlistcs",
+    "ifrmnum",
+    # package hyperref
+    "ifpdfstringunicode",
+    # package ifthen
+    "ifthenelse"
+  ] + if_exceptions
+
   def new_subtree(kind):
     return {'kind': kind, 'left': [], 'right': []}
 
@@ -201,6 +223,24 @@ def _simplify_conditional_blocks(text):
       tree['right'].append(subtree)
     subtree['parent'] = tree
 
+  def print_tree(tree, indent, write):
+    if "start" in tree:
+      write(' ' * indent + tree["start"].group() + "\n")
+    for subtree in tree["left"]:
+      print_tree(subtree, indent + 2, write)
+    if "else" in tree:
+      write(' ' * indent + tree["else"].group() + "\n")
+    for subtree in tree["right"]:
+      print_tree(subtree, indent + 2)
+    if "end" in tree:
+      write(' ' * indent + tree["end"].group() + "\n")
+
+  def print_abort(error_finding):
+    os.sys.stderr.write(f"Warning: Found {error_finding}! Not removing any conditional blocks...\n")
+    os.sys.stderr.write(f"         This is the matched tree (as built up to the error):\n")
+    print_tree(toplevel_tree, indent=9, write=os.sys.stderr.write)
+    os.sys.stderr.write(f"         Potentially, you need to supply an exception using --if_exceptions'.\n")
+
   for m in p.finditer(text):
     m_no_space = m.group().replace(' ', '')
     if m_no_space == r'\iffalse' or m_no_space == r'\if0':
@@ -214,25 +254,34 @@ def _simplify_conditional_blocks(text):
       add_subtree(tree, subtree)
       tree = subtree
     elif m_no_space.startswith(r'\if'):
+      if m_no_space[1:] in exceptions:
+        continue
       subtree = new_subtree('unknown')
       subtree['start'] = m
       add_subtree(tree, subtree)
       tree = subtree
     elif m_no_space == r'\else':
       if tree['parent'] is None:
-        os.sys.stderr.write('Warning: Ignoring unmatched \\else!\n')
-        continue
+        print_abort(r'unmatched \else')
+        return text
+      elif "else" in tree:
+        print_abort(r'duplicate \else')
+        return text
+
       tree['else'] = m
     elif m.group() == r'\fi':
       if tree['parent'] is None:
-        os.sys.stderr.write('Warning: Ignoring unmatched \\fi!\n')
-        continue
+        print_abort(r'unmatched \fi')
+        return text
+
       tree['end'] = m
       tree = tree['parent']
     else:
-      raise ValueError('Unreachable!')
+      raise RuntimeError('Unreachable!')
+
   if tree['parent'] is not None:
-    raise ValueError(f"Unmatched {tree['start'].group()}")
+    print_abort('unmatched ' + tree['start'].group())
+    return text
 
   positions_to_delete = []
 
@@ -353,7 +402,7 @@ def _remove_comments_and_commands_to_delete(content, parameters):
   """Erases all LaTeX comments in the content, and writes it."""
   content = [_remove_comments_inline(line) for line in content]
   content = _remove_environment(''.join(content), 'comment')
-  content = _simplify_conditional_blocks(content)
+  content = _simplify_conditional_blocks(content, parameters.get('if_exceptions', []))
   for environment in parameters.get('environments_to_delete', []):
     content = _remove_environment(content, environment)
   for command in parameters.get('commands_only_to_delete', []):
diff --git a/tex/main.tex b/tex/main.tex
index 5d37470..867b92e 100644
--- a/tex/main.tex
+++ b/tex/main.tex
@@ -58,6 +58,9 @@ Text
 
 \iftrue Expect me (iftrue block)!\else I shall be gone (else block of iftrue)!\fi
 
+\iffalse
+\iffalt
+\fi
 
 \newcommand{\red}[1]{{\color{red} #1}}
 hello test \red{hello


Test patch:
diff --git a/arxiv_latex_cleaner/tests/arxiv_latex_cleaner_test.py b/arxiv_latex_cleaner/tests/arxiv_latex_cleaner_test.py
index afcb8d2..4cb7798 100644
--- a/arxiv_latex_cleaner/tests/arxiv_latex_cleaner_test.py
+++ b/arxiv_latex_cleaner/tests/arxiv_latex_cleaner_test.py
@@ -501,6 +501,16 @@ class UnitTests(parameterized.TestCase):
           'text_in': '\\newif  \\ifvar \\ifvar\\iffalse test\\fi\\fi',
           'true_output': '\\newif  \\ifvar \\ifvar\\fi',
       },
+      {
+          'testcase_name': 'known exceptions (iff) ignored in \\iffalse',
+          'text_in': '\\iffalse \\iff\\fi',
+          'true_output': '',
+      },
+      {
+          'testcase_name': 'known exceptions (iff) ignored in \\iftrue',
+          'text_in': '\\iftrue\\iff\\else\\fi',
+          'true_output': '\\iff',
+      },
   )
   def test_simplify_conditional_blocks(self, text_in, true_output):
     self.assertEqual(
@@ -918,6 +928,7 @@ class IntegrationTests(parameterized.TestCase):
         'pdf_im_resolution': 500,
         'commands_to_delete': ['mytodo'],
         'commands_only_to_delete': ['red'],
+        'if_exceptions': ['iffalt'],
         'environments_to_delete': ['mynote'],
         'use_external_tikz': 'ext_tikz',
         'keep_bib': False,