Better handling of song parsing errors of chordpro files

Closes #191

patacrep/songs/chordpro/syntax.py

@@ -38,6 +38,8 @@ class ChordproParser(Parser):
         lexer = ChordProLexer(filename=self.filename)
         ast.AST.lexer = lexer.lexer
         parsed = self.parser.parse(content, lexer=lexer.lexer)
+        if parsed is None:
+            raise ContentError(message='Fatal error during song parsing.')
         parsed.error_builders.extend(lexer.error_builders)
         return parsed
 
@@ -325,8 +327,4 @@ class ChordproParser(Parser):
 
 def parse_song(content, filename=None):
     """Parse song and return its metadata."""
-    parser = ChordproParser(filename)
+    return ChordproParser(filename).parse(content)
-    parsed_content = parser.parse(content)
-    if parsed_content is None:
-        raise ContentError(message='Fatal error during song parsing.')
-    return parsed_content

patacrep/tools/convert/__main__.py

@@ -5,6 +5,7 @@ import logging
 import sys
 
 from patacrep import files
+from patacrep.content import ContentError
 from patacrep.songs import DEFAULT_CONFIG
 from patacrep.utils import yesno
 
@@ -57,8 +58,8 @@ def main(args=None):
         sys.exit(1)
 
     for file in song_files:
-        song = renderers[dest][source](file, DEFAULT_CONFIG)
         try:
+            song = renderers[dest][source](file, DEFAULT_CONFIG)
             destname = "{}.{}".format(".".join(file.split(".")[:-1]), dest)
             if os.path.exists(destname):
                 if not confirm(destname):
@@ -66,6 +67,9 @@ def main(args=None):
             with open(destname, "w") as destfile:
                 destfile.write(song.render())
 
+        except ContentError as error:
+            LOGGER.error("Cannot parse file '%s'.", file)
+            sys.exit(1)
         except NotImplementedError:
             LOGGER.error("Cannot convert to format '%s'.", dest)
             sys.exit(1)

test/test_patatools/test_convert.py

@@ -43,7 +43,10 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest):
                         with logging_reduced():
                             if os.path.exists(destname):
                                 os.remove(destname)
-                            self._system(main, args + [in_format, out_format, sourcename])
+                            self.assertEqual(
+                                self._system(main, args + [in_format, out_format, sourcename]),
+                                0,
+                                )
                             expected = controlfile.read().strip().replace(
                                 "@TEST_FOLDER@",
                                 files.path2posix(resource_filename(__name__, "")),
@@ -54,6 +57,26 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest):
                                     expected,
                                     )
 
+    def assertFailConvert(self, basename, in_format, out_format):
+        """Test of the "patatools convert" subcommand"""
+        sourcename = "{}.{}".format(basename, in_format)
+        destname = "{}.{}".format(basename, out_format)
+        controlname = "{}.{}.control".format(sourcename, out_format)
+        for main, args in [
+                (tools_main, ["patatools", "convert"]),
+                (convert_main, ["patatools-convert"]),
+            ]:
+            with self.subTest(main=main, args=args):
+                with self.chdir("test_convert_failure"):
+                    with open_read(controlname) as controlfile:
+                        with logging_reduced():
+                            if os.path.exists(destname):
+                                os.remove(destname)
+                            self.assertEqual(
+                                self._system(main, args + [in_format, out_format, sourcename]),
+                                1,
+                                )
+
     @staticmethod
     @contextlib.contextmanager
     def chdir(*path):
@@ -65,17 +88,21 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest):
     @classmethod
     def _iter_testmethods(cls):
         """Iterate over song files to test."""
-        with cls.chdir("test_convert_success"):
+        for directory, create_test in [
-            for control in sorted(glob.glob('*.*.*.control')):
+                ("test_convert_success", cls._create_test_success),
-                [*base, in_format, out_format, _] = control.split('.')
+                ("test_convert_failure", cls._create_test_failure),
-                base = '.'.join(base)
+            ]:
-                yield (
+            with cls.chdir(directory):
-                    "test_{}_{}_{}".format(base, in_format, out_format),
+                for control in sorted(glob.glob('*.*.*.control')):
-                    cls._create_test(base, in_format, out_format),
+                    [*base, in_format, out_format, _] = control.split('.')
-                    )
+                    base = '.'.join(base)
+                    yield (
+                        "test_{}_{}_{}".format(base, in_format, out_format),
+                        create_test(base, in_format, out_format),
+                        )
 
     @classmethod
-    def _create_test(cls, base, in_format, out_format):
+    def _create_test_success(cls, base, in_format, out_format):
         """Return a function testing that `base` compilation from `in_format` to `out_format` format.
         """
         test_parse_render = lambda self: self.assertConvert(base, in_format, out_format)
@@ -83,3 +110,13 @@ class TestConvert(unittest.TestCase, metaclass=dynamic.DynamicTest):
             "Test that '{base}.{in_format}' is correctly converted to '{out_format}'."
             ).format(base=os.path.basename(base), in_format=in_format, out_format=out_format)
         return test_parse_render
+
+    @classmethod
+    def _create_test_failure(cls, base, in_format, out_format):
+        """Return a function testing that `base` compilation from `in_format` to `out_format` format.
+        """
+        test_parse_render = lambda self: self.assertFailConvert(base, in_format, out_format)
+        test_parse_render.__doc__ = (
+            "Test that '{base}.{in_format}' raises an error when trying to convert it to '{out_format}'."
+            ).format(base=os.path.basename(base), in_format=in_format, out_format=out_format)
+        return test_parse_render

test/test_patatools/test_convert_errors/song.tsg

@@ -1,12 +0,0 @@
-\beginsong{Wonderful song}
-  [by={Anonymous}]
-
-  \begin{verse}
-    \[A]La la la\\
-  \end{verse}
-
-  \begin{chorus}
-    \[C]La la la\\
-  \end{chorus}
-
-\endsong

test/test_patatools/test_convert_failure/song.csg

@@ -0,0 +1,122 @@
+\selectlanguage{english}
+
+\beginsong{}[
+  by={
+  },
+]
+
+
+\begin{verse}
+    \selectlanguage
+    \songcolumns
+    \beginsong
+    \[&y={Traditionnel},cover={traditionnel},al&um={France}]
+\end{verse}
+
+
+\begin{verse}
+    \cover
+    \gtab
+    \gtab
+    \gtab
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Cheva\\[C]liers de la Table Ronde
+    Goûtons \\[G7]voir si le vin est \\[C]bon
+    \rep
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Goûtons \\[F]voir, \echo
+    Goûtons \\[C]voir, \echo
+    Goûtons \\[G7]voir si le vin est bon
+    \rep
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    S'il est bon, s'il est agréable
+    J'en boirai jusqu'à mon plaisir
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    J'en boirai cinq à six bouteilles
+    Et encore, ce n'est pas beaucoup
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Si je meurs, je veux qu'on m'enterre
+    Dans une cave où il y a du bon vin
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Les deux pieds contre la muraille
+    Et la tête sous le robinet
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Et les quatre plus grands ivrognes
+    Porteront les quatre coins du drap
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Pour donner le discours d'usage
+    On prendra le bistrot du coin
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Et si le tonneau se débouche
+    J'en boirai jusqu'à mon plaisir
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Et s'il en reste quelques gouttes
+    Ce sera pour nous rafraîchir
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \begin
+    Sur ma tombe, je veux qu'on inscrive
+    \emph
+    \end
+\end{verse}
+
+
+\begin{verse}
+    \endsong
+\end{verse}
+
+
+
+\endsong