👌 IMPROVE: logger warnings (and tests) (#8)

Author: chrisjsewell

README.md

@@ -10,8 +10,6 @@ In normal Sphinx documentation, the documentation structure is defined *via* a b
 
 This extension facilitates a **top-down** approach to defining the Table of Contents (ToC) structure, within a single file that is external to the documentation.
 
-The path to the toc file can be defined with `external_toc_path` (default: `_toc.yml`).
-
 ## User Guide
 
 ### Sphinx Configuration
@@ -20,7 +18,7 @@ Add to your `conf.py`:
 
 ```python
 extensions = ["sphinx_external_toc"]
-external_toc_path = "_toc.yml"  # optional
+external_toc_path = "_toc.yml"  # optional, default: _toc.yml
 ```
 
 ### Basic Structure
@@ -34,9 +32,9 @@ main:
 
 The value of the `file` key will be a path to a file (relative to the `conf.py`) with or without the file extension.
 
-```{important}
+:::{important}
 Each document file can only occur once in the ToC!
-```
+:::
 
 Document files can then have a `parts` key - denoting a list of individual toctrees for that document - and in-turn each part should have a `sections` key - denoting a list of children links, that are one of: `file`, `url` or `glob`:
 
@@ -59,6 +57,9 @@ main:
         - glob: other*
 ```
 
+This is equivalent to having a single `toctree` directive in `intro`, containing `doc1`,
+and a single `toctree` directive in `doc1`, with the `:glob:` flag and containing `doc2`, `https://example.com` and `other*`.
+
 As a shorthand, the `sections` key can be at the same level as the `file`, which denotes a document with a single `part`.
 For example, this file is exactly equivalent to the one above:
 
@@ -142,15 +143,19 @@ To build a template site from only a ToC file:
 $ sphinx-etoc create-site -p path/to/site -e rst path/to/_toc.yml
 ```
 
-Note, when using `glob` you can also add additional files in `meta`/`create_additional`, e.g.
+Note, you can also add additional files in `meta`/`create_files` amd append text to the end of files with `meta`/`create_append`, e.g.
 
 ```yaml
 main:
   file: intro
   sections:
   - glob: doc*
 meta:
-  create_additional:
+  create_append:
+    intro: |
+      This is some
+      extra text
+  create_files:
   - doc1
   - doc2
   - doc3
@@ -206,11 +211,10 @@ Process:
 
 Questions / TODOs:
 
-- What if toctree directive found in file? (raise incompatibility error/warnings)
 - Should `titlesonly` default to `True` (as in jupyter-book)?
 - nested numbered toctree not allowed (logs warning), so should be handled if `numbered: true` is in defaults
 - Add additional top-level keys, e.g. `appendices` and `bibliography`
-- testing against Windows
+- testing against Windows (including toc with subfolders)
 - option to add files not in toc to `ignore_paths` (including glob)
 - Add tests for "bad" toc files
 

codecov.yml

@@ -2,7 +2,7 @@ coverage:
   status:
     project:
       default:
-        target: 82%
+        target: 85%
         threshold: 0.2%
     patch:
       default:

sphinx_external_toc/events.py

@@ -1,8 +1,8 @@
 """Sphinx event functions."""
 from pathlib import Path
-from typing import Optional, Set
+from typing import List, Optional, Set
 
-from docutils.nodes import document, compound as compound_node
+from docutils import nodes
 from sphinx.addnodes import toctree as toctree_node
 from sphinx.application import Sphinx
 from sphinx.config import Config
@@ -17,6 +17,32 @@
 logger = logging.getLogger(__name__)
 
 
+def create_warning(
+    app: Sphinx,
+    doctree: nodes.document,
+    category: str,
+    message: str,
+    *,
+    line: Optional[int] = None,
+    append_to: Optional[nodes.Element] = None,
+    wtype: str = "etoc",
+) -> Optional[nodes.system_message]:
+    """Generate a warning, logging it if necessary.
+
+    If the warning type is listed in the ``suppress_warnings`` configuration,
+    then ``None`` will be returned and no warning logged.
+    """
+    message = f"{message} [{wtype}.{category}]"
+    kwargs = {"line": line} if line is not None else {}
+
+    if not logging.is_suppressed_warning(wtype, category, app.config.suppress_warnings):
+        msg_node = doctree.reporter.warning(message, **kwargs)
+        if append_to is not None:
+            append_to.append(msg_node)
+        return msg_node
+    return None
+
+
 def parse_toc_to_env(app: Sphinx, config: Config) -> None:
     """Parse the external toc file and store it in the Sphinx environment."""
     try:
@@ -53,11 +79,21 @@ def add_changed_toctrees(
     return changed_docs
 
 
-def append_toctrees(app: Sphinx, doctree: document) -> None:
+def append_toctrees(app: Sphinx, doctree: nodes.document) -> None:
     """Create the toctree nodes and add it to the document.
 
     Adapted from `sphinx/directives/other.py::TocTree`
     """
+    # check for existing toctrees and raise warning
+    for node in doctree.traverse(toctree_node):
+        create_warning(
+            app,
+            doctree,
+            "toctree",
+            "toctree directive not expected with external-toc",
+            line=node.line,
+        )
+
     site_map: SiteMap = app.env.external_site_map
     doc_item: Optional[DocItem] = site_map.get(app.env.docname)
 
@@ -87,10 +123,10 @@ def append_toctrees(app: Sphinx, doctree: document) -> None:
         subnode["includehidden"] = False
         subnode["numbered"] = toctree.numbered
         subnode["titlesonly"] = toctree.titlesonly
-        wrappernode = compound_node(classes=["toctree-wrapper"])
+        wrappernode = nodes.compound(classes=["toctree-wrapper"])
         wrappernode.append(subnode)
 
-        node_list = []
+        node_list: List[nodes.Element] = []
 
         for entry in toctree.sections:
 
@@ -116,7 +152,7 @@ def append_toctrees(app: Sphinx, doctree: document) -> None:
                     else:
                         message = f"toctree contains reference to nonexisting document {docname!r}"
 
-                    node_list.append(doctree.reporter.warning(message))
+                    create_warning(app, doctree, "ref", message, append_to=node_list)
                     app.env.note_reread()
                 else:
                     subnode["entries"].append((title, docname))
@@ -130,11 +166,10 @@ def append_toctrees(app: Sphinx, doctree: document) -> None:
                     subnode["entries"].append((None, docname))
                     subnode["includefiles"].append(docname)
                 if not docnames:
-                    node_list.append(
-                        doctree.reporter.warning(
-                            f"toctree glob pattern '{entry}' didn't match any documents"
-                        )
+                    message = (
+                        f"toctree glob pattern '{entry}' didn't match any documents"
                     )
+                    create_warning(app, doctree, "glob", message, append_to=node_list)
 
         # reversing entries can be useful when globbing
         if toctree.reversed:

sphinx_external_toc/tools.py

@@ -1,9 +1,10 @@
 from itertools import chain
 from pathlib import Path, PurePosixPath
 from os import linesep
-from typing import Union
+import shutil
+from typing import Mapping, Optional, Sequence, Union
 
-from .api import parse_toc_file
+from .api import parse_toc_file, SiteMap
 
 
 def create_site_from_toc(
@@ -13,38 +14,65 @@ def create_site_from_toc(
     default_ext: str = ".rst",
     encoding: str = "utf8",
     overwrite: bool = False,
-) -> Path:
+    toc_name: Optional[str] = "_toc.yml",
+) -> SiteMap:
     """Create the files defined in the external toc file.
 
     Additional files can also be created by defining them in
-    `meta`/`create_additional` of the toc
+    `meta`/`create_files` of the toc.
+    Text can also be appended to files, by defining them in `meta`/`create_append`
+    (ass a mapping of files -> text)
 
     :param toc_path: Path to ToC.
     :param root_path: The root directory , or use ToC file directory.
     :param default_ext: The default file extension to use.
     :param encoding: Encoding for writing files
     :param overwrite: Overwrite existing files (otherwise raise ``IOError``).
+    :param toc_name: Copy toc file to root with this name
 
-    :returns: Root path.
     """
     assert default_ext in {".rst", ".md"}
     site_map = parse_toc_file(toc_path)
 
     root_path = Path(toc_path).parent if root_path is None else Path(root_path)
+    root_path.mkdir(parents=True, exist_ok=True)
 
-    for docname in chain(site_map, site_map.meta.get("create_additional", [])):
+    # retrieve and validate meta variables
+    additional_files = site_map.meta.get("create_files", [])
+    assert isinstance(additional_files, Sequence), "'create_files' should be a list"
+    append_text = site_map.meta.get("create_append", {})
+    assert isinstance(append_text, Mapping), "'create_append' should be a mapping"
+
+    # copy toc file to root
+    if toc_name and not root_path.joinpath(toc_name).exists():
+        shutil.copyfile(toc_path, root_path.joinpath(toc_name))
+
+    # create files
+    for docname in chain(site_map, additional_files):
+
+        # create document
+        filename = docname
         if not any(docname.endswith(ext) for ext in {".rst", ".md"}):
-            docname += default_ext
-        docpath = root_path.joinpath(PurePosixPath(docname))
+            filename += default_ext
+        docpath = root_path.joinpath(PurePosixPath(filename))
         if docpath.exists() and not overwrite:
             raise IOError(f"Path already exists: {docpath}")
         docpath.parent.mkdir(parents=True, exist_ok=True)
-        heading = f"Heading: {docname}"
+
         content = []
-        if docname.endswith(".rst"):
+
+        # add heading based on file type
+        heading = f"Heading: {filename}"
+        if filename.endswith(".rst"):
             content = [heading, "=" * len(heading), ""]
-        elif docname.endswith(".md"):
+        elif filename.endswith(".md"):
             content = ["# " + heading, ""]
+
+        # append extra text
+        extra_lines = append_text.get(docname, "").splitlines()
+        if extra_lines:
+            content.extend(extra_lines + [""])
+
         docpath.write_text(linesep.join(content), encoding=encoding)
 
-    return root_path
+    return site_map

tests/_bad_toc_files/contains_toctree.yml

@@ -0,0 +1,11 @@
+main:
+  file: intro
+meta:
+  create_files:
+  - doc1
+  create_append:
+    intro: |
+      .. toctree::
+
+         doc1
+  expected_warning: toctree directive not expected

tests/_bad_toc_files/no_glob_match.yml

@@ -0,0 +1,6 @@
+main:
+  file: intro
+  sections:
+  - glob: doc*
+meta:
+  expected_warning: toctree glob pattern

tests/_toc_files/glob.yml

@@ -4,7 +4,7 @@ main:
   sections:
   - glob: doc*
 meta:
-  create_additional:
+  create_files:
   - doc1
   - doc2
   - doc3

tests/test_api/test_file_to_sitemap_glob_.yml

@@ -1,5 +1,5 @@
 _meta:
-  create_additional:
+  create_files:
   - doc1
   - doc2
   - doc3

tests/test_sphinx.py

@@ -1,6 +1,5 @@
 import os
 from pathlib import Path
-import shutil
 
 import pytest
 from sphinx.testing.util import SphinxTestApp
@@ -24,7 +23,8 @@ def __init__(self, app: SphinxTestApp, src: Path):
 
     def build(self, assert_pass=True):
         self.app.build()
-        assert self.warnings == "", self.status
+        if assert_pass:
+            assert self.warnings == "", self.status
         return self
 
     @property
@@ -52,15 +52,41 @@ def _func(src_path: Path, **kwargs) -> SphinxBuild:
 @pytest.mark.parametrize(
     "path", TOC_FILES, ids=[path.name.rsplit(".", 1)[0] for path in TOC_FILES]
 )
-def test_sphinx_build(path: Path, tmp_path: Path, sphinx_build_factory):
+def test_success(path: Path, tmp_path: Path, sphinx_build_factory):
+    """Test successful builds."""
     src_dir = tmp_path / "srcdir"
-    src_dir.mkdir()
-    # copy toc to temp
-    shutil.copyfile(path, src_dir / "_toc.yml")
-    # write conf.py
-    src_dir.joinpath("conf.py").write_text(CONF_CONTENT, encoding="utf8")
     # write document files
     create_site_from_toc(path, root_path=src_dir)
+    # write conf.py
+    src_dir.joinpath("conf.py").write_text(CONF_CONTENT, encoding="utf8")
     # run sphinx
     builder = sphinx_build_factory(src_dir)
     builder.build()
+
+
+def test_contains_toctree(tmp_path: Path, sphinx_build_factory):
+    """Test for warning if ``toctree`` directive in file."""
+    path = Path(__file__).parent.joinpath("_bad_toc_files", "contains_toctree.yml")
+    src_dir = tmp_path / "srcdir"
+    # write document files
+    sitemap = create_site_from_toc(path, root_path=src_dir)
+    # write conf.py
+    src_dir.joinpath("conf.py").write_text(CONF_CONTENT, encoding="utf8")
+    # run sphinx
+    builder = sphinx_build_factory(src_dir)
+    builder.build(assert_pass=False)
+    assert sitemap.meta["expected_warning"] in builder.warnings
+
+
+def test_no_glob_match(tmp_path: Path, sphinx_build_factory):
+    """Test for warning if glob pattern does not match any files."""
+    path = Path(__file__).parent.joinpath("_bad_toc_files", "no_glob_match.yml")
+    src_dir = tmp_path / "srcdir"
+    # write document files
+    sitemap = create_site_from_toc(path, root_path=src_dir)
+    # write conf.py
+    src_dir.joinpath("conf.py").write_text(CONF_CONTENT, encoding="utf8")
+    # run sphinx
+    builder = sphinx_build_factory(src_dir)
+    builder.build(assert_pass=False)
+    assert sitemap.meta["expected_warning"] in builder.warnings

tests/test_tools/test_file_to_sitemap_basic_.yml

@@ -1,3 +1,4 @@
+- _toc.yml
 - doc1.rst
 - doc2.rst
 - doc3.rst