[chordpro] Better error handling

patacrep/data/examples/songs/errors.sgc

@@ -0,0 +1,13 @@
+{language : english}
+{columns :  2}
+{ title : Error}
+{subtitle: A chordpro file with many errors}
+{artist: Traditionnel}
+
+{define: H4 base-fret 7 frets 2}
+{define:}
+
+Bla []bla
+Bla [H]bla
+
+

patacrep/songs/chordpro/ast.py

@@ -86,6 +86,12 @@ class AST:
         """Return the chordpro string corresponding to this object."""
         raise NotImplementedError()
 
+class Error(AST):
+    """Parsing error. To be ignored."""
+
+    def chordpro(self):
+        return ""
+
 class Line(AST):
     """A line is a sequence of (possibly truncated) words, spaces and chords."""
 
@@ -266,7 +272,9 @@ class Song(AST):
             if data.keyword in self.PROCESS_DIRECTIVE:
                 data = getattr(self, self.PROCESS_DIRECTIVE[data.keyword])(data)
 
-        if data is None:
+        if isinstance(data, Error):
+            return self
+        elif data is None:
             # New line
             if not (self.content and isinstance(self.content[0], Newline)):
                 self.content.insert(0, Newline())

patacrep/songs/chordpro/lexer.py

@@ -133,28 +133,32 @@ class ChordProLexer:
         return token
 
     @staticmethod
-    def t_error(token):
-        """Manage errors"""
-        LOGGER.error("Illegal character '{}'".format(token.value[0]))
+    def error(token, more=""):
+        """Display error message, and skip illegal token."""
+        LOGGER.error(
+            "Line {line}: Illegal character '{char}'{more}.".format(
+                line=token.lexer.lineno,
+                char=token.value[0],
+                more=more,
+                )
+            )
         token.lexer.skip(1)
 
-    @staticmethod
-    def t_chord_error(token):
+    def t_error(self, token):
         """Manage errors"""
-        LOGGER.error("Illegal character '{}' in chord..".format(token.value[0]))
-        token.lexer.skip(1)
+        self.error(token)
 
-    @staticmethod
-    def t_tablature_error(token):
+    def t_chord_error(self, token):
         """Manage errors"""
-        LOGGER.error("Illegal character '{}' in tablature..".format(token.value[0]))
-        token.lexer.skip(1)
+        self.error(token, more=" in chord")
 
-    @staticmethod
-    def t_directive_error(token):
+    def t_tablature_error(self, token):
         """Manage errors"""
-        LOGGER.error("Illegal character '{}' in directive..".format(token.value[0]))
-        token.lexer.skip(1)
+        self.error(token, more=" in tablature")
+
+    def t_directive_error(self, token):
+        """Manage errors"""
+        self.error(token, more=" in directive")
 
     def t_directiveargument_error(self, token):
         """Manage errors"""

patacrep/songs/chordpro/syntax.py

@@ -1,6 +1,5 @@
 """ChordPro parser"""
 
-import logging
 import ply.yacc as yacc
 import re
 
@@ -8,10 +7,9 @@ from patacrep.songs.syntax import Parser
 from patacrep.songs.chordpro import ast
 from patacrep.songs.chordpro.lexer import tokens, ChordProLexer
 
-LOGGER = logging.getLogger()
-
 CHORD_RE = re.compile(
     r"""
+        ^
         (?P<key>[A-G])
         (?P<alteration>[b#])?
         (?P<modifier>(maj|sus|dim|m))?
@@ -21,65 +19,11 @@ CHORD_RE = re.compile(
             (?P<basskey>[A-G])
             (?P<bassalteration>[b#])?
         )?
+        $
     """,
     re.VERBOSE
     )
 
-def _parse_chords(string):
-    """Parse a list of chords.
-
-    Iterate over :class:`ast.Chord` objects.
-    """
-    for chord in string.split():
-        match = CHORD_RE.match(chord)
-        if match is None:
-            TODO
-        yield ast.Chord(**match.groupdict())
-
-def _parse_define(key, basefret, frets, fingers):
-    """Parse a `{define: KEY base-fret BASE frets FRETS fingers FINGERS}` directive
-
-    Return a :class:`ast.Define` object.
-    """
-    # pylint: disable=too-many-branches
-    key = list(_parse_chords(key))
-    if len(key) != 1:
-        TODO
-    else:
-        processed_key = key[0]
-
-    if basefret is None:
-        processed_basefret = None
-    else:
-        processed_basefret = int(basefret)
-
-    if frets is None:
-        processed_frets = None
-    else:
-        processed_frets = []
-        for fret in frets.split():
-            if fret == "x":
-                processed_frets.append(None)
-            else:
-                processed_frets.append(int(fret))
-
-    if fingers is None:
-        processed_fingers = None
-    else:
-        processed_fingers = []
-        for finger in fingers.split():
-            if finger == '-':
-                processed_fingers.append(None)
-            else:
-                processed_fingers.append(int(finger))
-
-    return ast.Define(
-        key=processed_key,
-        basefret=processed_basefret,
-        frets=processed_frets,
-        fingers=processed_fingers,
-        )
-
 class ChordproParser(Parser):
     """ChordPro parser class"""
     # pylint: disable=too-many-public-methods
@@ -124,8 +68,51 @@ class ChordproParser(Parser):
         """
         symbols[0] = None
 
-    @staticmethod
-    def p_directive(symbols):
+    def _parse_define(self, groups, *, symbols):
+        """Parse a `{define: KEY base-fret BASE frets FRETS fingers FINGERS}` directive
+
+        Return a :class:`ast.Define` object.
+        """
+        # pylint: disable=too-many-branches
+        key = list(self._parse_chords(groups['key'], symbols=symbols))
+        if len(key) != 1:
+            return None
+        else:
+            key = key[0]
+
+        if groups['basefret'] is None:
+            basefret = None
+        else:
+            basefret = int(groups['basefret'])
+
+        if groups['frets'] is None:
+            frets = None
+        else:
+            frets = []
+            for fret in groups['frets'].split():
+                if fret == "x":
+                    frets.append(None)
+                else:
+                    frets.append(int(fret))
+
+        if groups['fingers'] is None:
+            fingers = None
+        else:
+            fingers = []
+            for finger in groups['fingers'].split():
+                if finger == '-':
+                    fingers.append(None)
+                else:
+                    fingers.append(int(finger))
+
+        return ast.Define(
+            key=key,
+            basefret=basefret,
+            frets=frets,
+            fingers=fingers,
+            )
+
+    def p_directive(self, symbols):
         """directive : LBRACE KEYWORD directive_next RBRACE
                      | LBRACE SPACE KEYWORD directive_next RBRACE
         """
@@ -139,18 +126,38 @@ class ChordproParser(Parser):
         if keyword == "define":
             match = re.compile(
                 r"""
+                    ^
                     (?P<key>[^\ ]*)\ *
                     (base-fret\ *(?P<basefret>[2-9]))?\ *
                     frets\ *(?P<frets>((\d+|x)\ *)+)\ *
                     (fingers\ *(?P<fingers>(([0-4-])\ *)*))?
+                    $
                 """,
                 re.VERBOSE
                 ).match(argument)
 
             if match is None:
-                TODO
+                if argument.strip():
+                    self.error(
+                        line=symbols.lexer.lineno,
+                        message="Invalid chord definition '{}'.".format(argument),
+                        )
+                else:
+                    self.error(
+                        line=symbols.lexer.lineno,
+                        message="Invalid empty chord definition.",
+                        )
+                symbols[0] = ast.Error()
+                return
+
+            symbols[0] = self._parse_define(match.groupdict(), symbols=symbols)
+            if symbols[0] is None:
+                self.error(
+                    line=symbols.lexer.lineno,
+                    message="Invalid chord definition '{}'.".format(argument),
+                    )
+                symbols[0] = ast.Error()
 
-            symbols[0] = _parse_define(**match.groupdict())
         else:
             symbols[0] = ast.Directive(keyword, argument)
 
@@ -158,12 +165,15 @@ class ChordproParser(Parser):
     def p_directive_next(symbols):
         """directive_next : SPACE COLON TEXT
                           | COLON TEXT
+                          | COLON
                           | empty
         """
         if len(symbols) == 3:
             symbols[0] = symbols[2].strip()
         elif len(symbols) == 4:
             symbols[0] = symbols[3].strip()
+        elif len(symbols) == 2 and symbols[1] == ":":
+            symbols[0] = ""
         else:
             symbols[0] = None
 
@@ -196,10 +206,24 @@ class ChordproParser(Parser):
         """space : SPACE"""
         symbols[0] = ast.Space()
 
-    @staticmethod
-    def p_chord(symbols):
+    def _parse_chords(self, string, *, symbols):
+        """Parse a list of chords.
+
+        Iterate over :class:`ast.Chord` objects.
+        """
+        for chord in string.split():
+            match = CHORD_RE.match(chord)
+            if match is None:
+                self.error(
+                    line=symbols.lexer.lineno,
+                    message="Invalid chord '{}'.".format(chord),
+                    )
+                continue
+            yield ast.Chord(**match.groupdict())
+
+    def p_chord(self, symbols):
         """chord : CHORD"""
-        symbols[0] = ast.ChordList(*_parse_chords(symbols[1]))
+        symbols[0] = ast.ChordList(*self._parse_chords(symbols[1], symbols=symbols))
 
     @staticmethod
     def p_chorus(symbols):

patacrep/songs/chordpro/test/invalid_chord.sgc

@@ -0,0 +1,3 @@
+This is [H] invalid.
+This [A+]too.
+And [Amm]as well.

patacrep/songs/chordpro/test/invalid_chord.txt

@@ -0,0 +1,6 @@
+
+{start_of_verse}
+  This is  invalid.
+  This [A]too.
+  And []as well.
+{end_of_verse}

patacrep/songs/chordpro/test/invalid_customchord.sgc

@@ -0,0 +1,5 @@
+{define : }
+{define: H base-fret 7 frets 0 1 3 3 x x}
+{define: E4 base-fret H frets 0 1 3 3 x x}
+{define: E4 base-fret 7 frets 0 1 3 3 x A}
+{define: E5 base-fret 7 frets 0 1 3 3 x x fingers - 1 2 3 - A}

patacrep/songs/syntax.py

@@ -20,14 +20,27 @@ class Parser:
         column = (token.lexpos - last_cr) + 1
         return column
 
+    @staticmethod
+    def error(*, line, column=None, message=""):
+        """Display an error message"""
+        text = "Line {}".format(line)
+        if column is not None:
+            text += ", column {}".format(column)
+        if message:
+            text += ": " + message
+        else:
+            text += "."
+        LOGGER.error(text)
+
     def p_error(self, token):
         """Manage parsing errors."""
-        if token:
-            LOGGER.error(
-                "Error in file {}, line {}:{}.".format(
-                    str(self.filename),
-                    token.lineno,
-                    self.__find_column(token),
+        if token is None:
+            self.error(
+                line=token.lineno,
+                message="Unexpected end of file.",
                 )
+        else:
+            self.error(
+                line=token.lineno,
+                column=self.__find_column(token),
                 )
-