Repository: ucbds-infra/otter-grader

Issue:
Make xeCJK opt-in behavior
Make the use of the xeCJK template for Otter Export opt-in only. Currently, if PDF generation fails, Otter Export automatically tries again with xeCJK enabled, and then only shows the error message generated by the xeCJK template. Instead, this retry should not happen automatically, and instead the `xecjk` argument should be used to determine whether to use the xeCJK template or not. This would need to be added as an argument to functions that call Otter Export (e.g. `otter.Notebook.export`). This also means that `no_xecjk`/`--no-xecjk` can be removed.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 600de4fa..d66c12d7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * Removed deprecated Otter Assign format v0
 * Disabled editing for question prompt cells in the notebooks generated by Otter Assign per [#431](https://github.com/ucbds-infra/otter-grader/issues/431)
 * Removed `ucbdsinfra/otter-grader` Docker image and made default image for Otter Grade `ubuntu:22.04` per [#534](https://github.com/ucbds-infra/otter-grader/issues/534)
+* Make `xeCJK` opt-in behavior for exporting notebooks via LaTeX per [#548](https://github.com/ucbds-infra/otter-grader/issues/548)
 
 **v4.2.1:**
 
diff --git a/otter/cli.py b/otter/cli.py
index 08c9b414..779daec6 100644
--- a/otter/cli.py
+++ b/otter/cli.py
@@ -87,7 +87,7 @@ defaults = export.__kwdefaults__
 @click.option("--pagebreaks", is_flag=True, help="Whether the PDF should have pagebreaks between questions")
 @click.option("-s", "--save", is_flag=True, help="Save intermediate file(s) as well")
 @click.option("-e", "--exporter", default=defaults["exporter"], type=click.Choice(["latex", "html"]), help="Type of PDF exporter to use")
-@click.option("--no-xecjk", is_flag=True, help="Force-disable xeCJK in Otter's LaTeX template")
+@click.option("--xecjk", is_flag=True, help="Enable xeCJK in Otter's LaTeX template")
 def export_cli(*args, **kwargs):
     """
     Export a Jupyter Notebook SRC as a PDF at DEST with optional filtering.
diff --git a/otter/export/__init__.py b/otter/export/__init__.py
index 22f959e1..d1d6f68c 100644
--- a/otter/export/__init__.py
+++ b/otter/export/__init__.py
@@ -45,7 +45,7 @@ def export_notebook(nb_path, dest=None, exporter_type=None, **kwargs):
 
 
 def main(src, *, dest=None, exporter=None, filtering=False, pagebreaks=False, save=False, 
-         no_xecjk=False):
+         xecjk=False):
     """
     Runs Otter Export
 
@@ -57,7 +57,7 @@ def main(src, *, dest=None, exporter=None, filtering=False, pagebreaks=False, sa
         pagebreaks (``bool``): whether to pagebreak between filtered regions; ignored if ``filtering``
             is ``False``
         save (``bool``): whether to save any intermediate files (e.g. ``.tex``, ``.html``)
-        no_xecjk (``bool``): whether to disable xeCJK in the LaTeX template
+        xecjk (``bool``): whether to use xeCJK in the LaTeX template
     """
     export_notebook(
         src,
@@ -67,5 +67,5 @@ def main(src, *, dest=None, exporter=None, filtering=False, pagebreaks=False, sa
         pagebreaks = pagebreaks,
         save_tex = save,
         save_html = save,
-        no_xecjk=no_xecjk,
+        xecjk=xecjk,
     )
diff --git a/otter/export/exporters/via_latex.py b/otter/export/exporters/via_latex.py
index 9ff607d0..fdb6fa63 100644
--- a/otter/export/exporters/via_latex.py
+++ b/otter/export/exporters/via_latex.py
@@ -31,12 +31,9 @@ class PDFViaLatexExporter(BaseExporter):
     })
 
     @classmethod
-    def convert_notebook(cls, nb_path, dest, xecjk=False, no_xecjk=False, **kwargs):
+    def convert_notebook(cls, nb_path, dest, xecjk=False, **kwargs):
         warnings.filterwarnings("ignore", r"invalid escape sequence '\\c'", DeprecationWarning)
 
-        if xecjk and no_xecjk:
-            raise ValueError("xeCJK LaTeX template indicated but disallowed")
-
         options = cls.default_options.copy()
         options.update(kwargs)
 
@@ -71,17 +68,13 @@ class PDFViaLatexExporter(BaseExporter):
                 output_file.write(pdf_output[0])
 
         except nbconvert.pdf.LatexFailed as error:
-            if not xecjk and not no_xecjk:
-                cls.convert_notebook(nb_path, dest, xecjk=True, **kwargs)
-
-            else:
-                message = "There was an error generating your LaTeX; showing full error message:\n"
-                message += indent(error.output, "    ")
-                if xecjk:
-                    message += "\n\nIf the error above is related to xeCJK or fandol in LaTeX " \
-                        "and you don't require this functionality, try running again with " \
-                        "no_xecjk set to True or the --no-xecjk flag."
-                raise ExportFailedException(message)
+            message = "There was an error generating your LaTeX; showing full error message:\n"
+            message += indent(error.output, "    ")
+            if xecjk:
+                message += "\n\nIf the error above is related to xeCJK or fandol in LaTeX " \
+                    "and you don't require this functionality, try running again without " \
+                    "xecjk set to True or the --xecjk flag."
+            raise ExportFailedException(message)
 
         finally:
             if NBCONVERT_6:


Test patch:
diff --git a/test/test_cli.py b/test/test_cli.py
index 481f0ebb..116a79cf 100644
--- a/test/test_cli.py
+++ b/test/test_cli.py
@@ -215,7 +215,7 @@ def test_export(mocked_export, run_cli):
         filtering=False,
         pagebreaks=False,
         save=False,
-        no_xecjk=False,
+        xecjk=False,
     )
 
     result = run_cli([*cmd_start])
@@ -242,9 +242,9 @@ def test_export(mocked_export, run_cli):
     assert_cli_result(result, expect_error=False)
     mocked_export.assert_called_with(**{**std_kwargs, "save": True})
 
-    result = run_cli([*cmd_start, "--no-xecjk"])
+    result = run_cli([*cmd_start, "--xecjk"])
     assert_cli_result(result, expect_error=False)
-    mocked_export.assert_called_with(**{**std_kwargs, "no_xecjk": True})
+    mocked_export.assert_called_with(**{**std_kwargs, "xecjk": True})
 
     result = run_cli([*cmd_start, "-e", "latex"])
     assert_cli_result(result, expect_error=False)