👌 IMPROVE: Add comments, bit refactoring, docs, test for latex (#34)

* adding comments and creating common functions

* added a simple test for latex

* removing unnecessary test latex files

Author: AakashGfude

docs/source/index.md

@@ -16,11 +16,7 @@ testing
 **An exercise extension for Sphinx**.
 
 This package contains a [Sphinx](http://www.sphinx-doc.org/en/master/) extension
-for producing exercise and solution directives.
-
-```{warning}
-sphinx-exercise `0.1.1` is in a development stage and may change rapidly.
-```
+for producing exercise and solution directives, for html and pdf outputs.
 
 **Features**:
 

setup.py

@@ -26,6 +26,7 @@
         "pytest-regressions",
         "beautifulsoup4",
         "myst-nb",
+        "texsoup",
     ],
     "rtd": [
         "sphinx>=3.0",

sphinx_exercise/__init__.py

@@ -18,6 +18,9 @@
 from docutils import nodes as docutil_nodes
 from sphinx.util import logging
 from sphinx.util.fileutil import copy_asset
+from sphinx.builders.latex import LaTeXBuilder
+from sphinx.transforms.post_transforms import SphinxPostTransform
+
 from .directive import ExerciseDirective, SolutionDirective
 from .nodes import (
     exercise_node,
@@ -31,14 +34,15 @@
     depart_solution_node,
     is_solution_node,
     is_exercise_node,
-    is_unenumerable_node,
+    is_exercise_unenumerable_node,
     is_extension_node,
     NODE_TYPES,
 )
-from sphinx.transforms.post_transforms import SphinxPostTransform
-from .utils import get_node_number, get_refuri, has_math_child
+from .utils import get_node_number, get_refuri, has_math_child, find_parent
 
 logger = logging.getLogger(__name__)
+
+# Variables
 SOLUTION_PLACEHOLDER = "Solution to "
 MATH_PLACEHOLDER = ":math:"
 
@@ -118,6 +122,9 @@ def doctree_read(app: Sphinx, document: Node) -> None:
 
 
 def update_title(title):
+    """
+    Does necessary formatting to the title node, and wraps it with an inline node.
+    """
     inline = docutil_nodes.inline()
 
     if len(title) == 1 and isinstance(title[0], docutil_nodes.Text):
@@ -142,15 +149,21 @@ def update_title(title):
 
 
 def process_math_placeholder(node, update_title, source_node):
+    """Convert the placeholder math text to a math node."""
     if MATH_PLACEHOLDER in node.astext():
         title = update_title(source_node[0])
         return node.replace(node[0], title)
 
 
 def process_reference(self, node, default_title=""):
+    """
+    Processing reference nodes in the document to facilitate the design and the
+    functionality requirements.
+    """
     label = get_refuri(node)
     if label in self.env.exercise_list:
         source_node = self.env.exercise_list[label].get("node")
+        # if reference source is a solution node
         if is_solution_node(source_node):
             target_label = source_node.attributes.get("target_label", "")
             if node.astext().strip() == "Solution to":
@@ -159,15 +172,16 @@ def process_reference(self, node, default_title=""):
             target_label = source_node.attributes.get("label", "")
         target_attr = self.env.exercise_list[target_label]
         target_node = target_attr.get("node", Node)
+        # if reference target is exercise node
         if is_exercise_node(target_node):
             if default_title:
                 number = get_node_number(self.app, target_node, "exercise")
                 node.insert(len(node[0]), docutil_nodes.Text(" Exercise " + number))
                 return
             else:
                 node = process_math_placeholder(node, update_title, source_node)
-
-        if is_unenumerable_node(target_node):
+        # if reference target is an exercise unenumerable node
+        if is_exercise_unenumerable_node(target_node):
             if default_title:
                 if target_attr.get("title"):
                     if has_math_child(target_node[0]):
@@ -186,7 +200,7 @@ def process_reference(self, node, default_title=""):
 
 
 class ReferenceTransform(SphinxPostTransform):
-    default_priority = 998
+    default_priority = 998  # should be processed before processing solution nodes
 
     def run(self):
 
@@ -195,7 +209,7 @@ def run(self):
 
 
 class SolutionTransorm(SphinxPostTransform):
-    default_priority = 999
+    default_priority = 999  # should be after processing reference nodes
 
     def run(self):
 
@@ -205,7 +219,11 @@ def run(self):
                 target_attr = self.env.exercise_list[target_labelid]
             except Exception:
                 # target_labelid not found
-                docpath = self.env.doc2path(self.app.builder.current_docname)
+                if isinstance(self.app.builder, LaTeXBuilder):
+                    docname = find_parent(self.app.builder.env, node, "section")
+                else:
+                    docname = self.app.builder.current_docname
+                docpath = self.env.doc2path(docname)
                 path = docpath[: docpath.rfind(".")]
                 msg = f"undefined label: {target_labelid}"
                 logger.warning(msg, location=path, color="red")
@@ -250,6 +268,7 @@ def run(self):
                 source_node = source_attr.get("node", Node)
                 node_title = node.get("title", "")
 
+                # processing for nodes which have
                 if "{name}" in node_title and has_math_child(source_node[0]):
                     newtitle = docutil_nodes.inline()
                     for item in node_title.split():
@@ -278,11 +297,11 @@ def setup(app: Sphinx) -> Dict[str, Any]:
     app.add_config_value("hide_solutions", False, "env")
 
     app.add_css_file("exercise.css")
-    app.connect("config-inited", init_numfig)  # 1
-    app.connect("env-purge-doc", purge_exercises)  # 5 per file
-    app.connect("doctree-read", doctree_read)  # 8
-    app.connect("env-merge-info", merge_exercises)  # 9
-    app.connect("build-finished", copy_asset_files)  # 16
+    app.connect("config-inited", init_numfig)  # event order - 1
+    app.connect("env-purge-doc", purge_exercises)  # event order - 5 per file
+    app.connect("doctree-read", doctree_read)  # event order - 8
+    app.connect("env-merge-info", merge_exercises)  # event order - 9
+    app.connect("build-finished", copy_asset_files)  # event order - 16
 
     app.add_enumerable_node(
         exercise_node,

sphinx_exercise/directive.py

@@ -39,7 +39,8 @@ def run(self) -> List[Node]:
         if class_name:
             classes.extend(class_name)
 
-        title_text = ""
+        # Have a dummy title text if no title specified, as 'std' domain needs
+        # a title to process it as enumerable node.
         if typ == "exercise":
             title_text = f"{self.name.title()} "
 
@@ -49,11 +50,7 @@ def run(self) -> List[Node]:
             title_text = f"{self.name.title()} to "
             target_label = self.arguments[0]
 
-        textnodes, messages = self.state.inline_text(title_text, self.lineno)
-
-        section = nodes.section(ids=[f"{typ}-content"])
-        self.state.nested_parse(self.content, self.content_offset, section)
-
+        # selecting the type of node
         if typ == "exercise":
             if "nonumber" in self.options:
                 node = exercise_unenumerable_node()
@@ -62,6 +59,11 @@ def run(self) -> List[Node]:
         else:
             node = solution_node()
 
+        # state parsing
+        section = nodes.section(ids=[f"{typ}-content"])
+        textnodes, messages = self.state.inline_text(title_text, self.lineno)
+        self.state.nested_parse(self.content, self.content_offset, section)
+
         node += nodes.title(title_text, "", *textnodes)
         node += section
 

sphinx_exercise/nodes.py

@@ -11,7 +11,7 @@
 from sphinx.util import logging
 from docutils import nodes as docutil_nodes
 from sphinx.writers.latex import LaTeXTranslator
-from .utils import get_node_number, find_parent
+from .utils import get_node_number, find_parent, list_rindex
 
 logger = logging.getLogger(__name__)
 
@@ -32,51 +32,67 @@ class exercise_unenumerable_node(docutil_nodes.Admonition, docutil_nodes.Element
     pass
 
 
+def _visit_nodes_latex(self, node, find_parent):
+    """ Function to handle visit_node for latex. """
+    docname = find_parent(self.builder.env, node, "section")
+    self.body.append(
+        "\\phantomsection \\label{" + f"{docname}:{node.attributes['label']}" + "}"
+    )
+    self.body.append(latex_admonition_start)
+
+
+def _depart_nodes_latex(self, node, title, pop_index=False):
+    """ Function to handle depart_node for latex. """
+    idx = list_rindex(self.body, latex_admonition_start) + 2
+    if pop_index:
+        self.body.pop(idx)
+    self.body.insert(idx, title)
+    self.body.append(latex_admonition_end)
+
+
+def _remove_placeholder_title_exercise(typ, node):
+    """ Removing the exercise placeholder we put in title earlier."""
+    for title in node.traverse(docutil_nodes.title):
+        if typ.title() in title.astext():
+            title[0] = docutil_nodes.Text("")
+
+
 def visit_enumerable_node(self, node: Node) -> None:
+    typ = node.attributes.get("type", "")
     if isinstance(self, LaTeXTranslator):
-        docname = find_parent(self.builder.env, node, "section")
-        self.body.append("\\label{" + f"{docname}:{node.attributes['label']}" + "}")
-        self.body.append(latex_admonition_start)
+        _remove_placeholder_title_exercise(typ, node)
+        _visit_nodes_latex(self, node, find_parent)
     else:
-        for title in node.traverse(docutil_nodes.title):
-            if "Exercise" in title.astext():
-                title[0] = docutil_nodes.Text("")
+        _remove_placeholder_title_exercise(typ, node)
         self.body.append(self.starttag(node, "div", CLASS="admonition"))
 
 
 def depart_enumerable_node(self, node: Node) -> None:
     typ = node.attributes.get("type", "")
     if isinstance(self, LaTeXTranslator):
         number = get_node_number(self, node, typ)
-        idx = list_rindex(self.body, latex_admonition_start) + 2
-        self.body.insert(idx, f"{typ.title()} {number} ")
-        self.body.append(latex_admonition_end)
+        _depart_nodes_latex(self, node, f"{typ.title()} {number} ")
     else:
         number = get_node_number(self, node, typ)
-        if number:
-            idx = self.body.index(f"{typ.title()} {number} ")
-            self.body[idx] = f"{typ.title()} {number} "
+        idx = list_rindex(self.body, f"{typ.title()} {number} ")
+        self.body[idx] = f"{typ.title()} {number} "
         self.body.append("</div>")
 
 
 def visit_exercise_unenumerable_node(self, node: Node) -> None:
+    typ = node.attributes.get("type", "")
     if isinstance(self, LaTeXTranslator):
-        docname = find_parent(self.builder.env, node, "section")
-        self.body.append("\\label{" + f"{docname}:{node.attributes['label']}" + "}")
-        self.body.append(latex_admonition_start)
+        _remove_placeholder_title_exercise(typ, node)
+        _visit_nodes_latex(self, node, find_parent)
     else:
-        for title in node.traverse(docutil_nodes.title):
-            if "Exercise" in title.astext():
-                title[0] = docutil_nodes.Text("")
+        _remove_placeholder_title_exercise(typ, node)
         self.body.append(self.starttag(node, "div", CLASS="admonition"))
 
 
 def depart_exercise_unenumerable_node(self, node: Node) -> None:
     typ = node.attributes.get("type", "")
     if isinstance(self, LaTeXTranslator):
-        idx = list_rindex(self.body, latex_admonition_start) + 2
-        self.body.insert(idx, f"{typ.title()} ")
-        self.body.append(latex_admonition_end)
+        _depart_nodes_latex(self, node, f"{typ.title()} ")
     else:
         idx = list_rindex(self.body, '<p class="admonition-title">') + 1
         element = f"<span>{typ.title()} </span>"
@@ -86,23 +102,18 @@ def depart_exercise_unenumerable_node(self, node: Node) -> None:
 
 def visit_solution_node(self, node: Node) -> None:
     if isinstance(self, LaTeXTranslator):
-        docname = find_parent(self.builder.env, node, "section")
-        self.body.append("\\label{" + f"{docname}:{node.attributes['label']}" + "}")
-        self.body.append(latex_admonition_start)
+        _visit_nodes_latex(self, node, find_parent)
     else:
         self.body.append(self.starttag(node, "div", CLASS="admonition"))
 
 
 def depart_solution_node(self, node: Node) -> None:
     typ = node.attributes.get("type", "")
     if isinstance(self, LaTeXTranslator):
-        idx = list_rindex(self.body, latex_admonition_start) + 2
-        self.body.pop(idx)
-        self.body.insert(idx, f"{typ.title()} ")
-        self.body.append(latex_admonition_end)
+        _depart_nodes_latex(self, node, f"{typ.title()} to ", True)
     else:
         number = get_node_number(self, node, typ)
-        idx = self.body.index(f"{typ.title()} {number} ")
+        idx = list_rindex(self.body, f"{typ.title()} {number} ")
         self.body.pop(idx)
         self.body.append("</div>")
 
@@ -111,7 +122,7 @@ def is_exercise_node(node):
     return isinstance(node, exercise_node)
 
 
-def is_unenumerable_node(node):
+def is_exercise_unenumerable_node(node):
     return isinstance(node, exercise_unenumerable_node)
 
 
@@ -121,7 +132,9 @@ def is_solution_node(node):
 
 def is_extension_node(node):
     return (
-        is_exercise_node(node) or is_unenumerable_node(node) or is_solution_node(node)
+        is_exercise_node(node)
+        or is_exercise_unenumerable_node(node)
+        or is_solution_node(node)
     )
 
 
@@ -131,14 +144,6 @@ def rreplace(s, old, new, occurrence):
     return new.join(li)
 
 
-def list_rindex(li, x) -> int:
-    """Getting the last occurence of an item in a list."""
-    for i in reversed(range(len(li))):
-        if li[i] == x:
-            return i
-    raise ValueError("{} is not in list".format(x))
-
-
 NODE_TYPES = {
     "exercise": {"node": exercise_node, "type": "exercise"},
     "solution": {"node": solution_node, "type": "solution"},

sphinx_exercise/utils.py

@@ -1,3 +1,4 @@
+# Utility functions
 from sphinx.writers.latex import LaTeXTranslator
 from docutils import nodes as docutil_nodes
 
@@ -53,3 +54,11 @@ def get_refuri(node):
         id_ = node.get("refid", "")
 
     return id_.split("#")[-1]
+
+
+def list_rindex(li, x) -> int:
+    """Getting the last occurence of an item in a list."""
+    for i in reversed(range(len(li))):
+        if li[i] == x:
+            return i
+    raise ValueError("{} is not in list".format(x))

tests/books/test-simplebook/Makefile

@@ -0,0 +1,20 @@
+# Minimal makefile for Sphinx documentation
+#
+
+# You can set these variables from the command line, and also
+# from the environment for the first two.
+SPHINXOPTS    ?=
+SPHINXBUILD   ?= sphinx-build
+SOURCEDIR     = .
+BUILDDIR      = build
+
+# Put it first so that "make" without argument is like "make help".
+help:
+	@$(SPHINXBUILD) -M help "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
+
+.PHONY: help Makefile
+
+# Catch-all target: route all unknown targets to Sphinx using the new
+# "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
+%: Makefile
+	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)