Bit more context for errors, including a sweet caret.

Partially reverted the grayscale() fix, since it masks errors in Sass
functions.

3 files changed, 64 insertions, 32 deletions

diff --git a/scss/__init__.py b/scss/__init__.py
index 3cad57c..23ec754 100644
--- a/scss/__init__.py
+++ b/scss/__init__.py
@@ -518,7 +518,7 @@ class Scss(object):
         try:
             return self._manage_children_impl(rule, p_children, scope)
         except SassError, e:
-            e.set_rule(rule)
+            e.add_rule(rule)
             raise
         except Exception, e:
             raise SassError(e, rule=rule)
diff --git a/scss/errors.py b/scss/errors.py
index 6e946e5..e06bc87 100644
--- a/scss/errors.py
+++ b/scss/errors.py
@@ -1,29 +1,53 @@
 import sys
 import traceback
 
+
+def add_error_marker(text, position, start_line=1):
+    """Add a caret marking a given position in a string of input.
+
+    Returns (new_text, caret_line).
+    """
+    indent = "    "
+    lines = []
+    caret_line = start_line
+    for line in text.split("\n"):
+        lines.append(indent + line)
+
+        if 0 <= position <= len(line):
+            lines.append(indent + (" " * position) + "^")
+            caret_line = start_line
+
+        position -= len(line)
+        position -= 1  # for the newline
+        start_line += 1
+
+    return "\n".join(lines), caret_line
+
+
 class SassError(Exception):
     """Error class that wraps another exception and attempts to bolt on some
     useful context.
     """
-    def __init__(self, exc, rule=None, expression=None):
+    def __init__(self, exc, rule=None, expression=None, expression_pos=None):
         self.exc = exc
-        self.rule = rule
+
+        self.rule_stack = []
+        if rule:
+            self.rule_stack.append(rule)
+
         self.expression = expression
-        _, _, self.original_traceback = sys.exc_info()
+        self.expression_pos = expression_pos
 
-    def set_rule(self, rule):
-        """Set the current rule.
+        _, _, self.original_traceback = sys.exc_info()
 
-        If this error already has a rule, it's left alone; this is because
-        exceptions propagate outwards and so the first rule seen is the
-        "innermost".
+    def add_rule(self, rule):
+        """Add a new rule to the "stack" of rules -- this is used to track,
+        e.g., how a file was ultimately imported.
         """
-        if not self.rule:
-            self.rule = rule
+        self.rule_stack.append(rule)
 
     def format_prefix(self):
-        # TODO pointer, yadda
-        # TODO newlines, yadda
+        # TODO this contains NULs and line numbers; could be much prettier
         if self.rule:
             return "Error parsing block:\n" + "    " + self.rule.unparsed_contents
         else:
@@ -34,28 +58,39 @@ class SassError(Exception):
 
         ret = [prefix, "\n\n"]
 
-        if self.rule:
-            ret.extend(("From ", self.rule.file_and_line, "\n\n"))
+        if self.rule_stack:
+            # TODO this looks very neat-o but doesn't actually work because
+            # manage_children doesn't recurse for imports  :)
+            ret.extend(("From ", self.rule_stack[0].file_and_line, "\n"))
+            last_file = self.rule_stack[0].source_file
+
+            for rule in self.rule_stack[1:]:
+                if rule.source_file is not last_file:
+                    ret.extend(("...imported from ", rule.file_and_line, "\n"))
+                last_file = rule.source_file
+
+            ret.append("\n")
 
         ret.append("Traceback:\n")
         ret.extend(traceback.format_tb(self.original_traceback))
         ret.extend((type(self.exc).__name__, ": ", str(self.exc), "\n"))
         return ''.join(ret)
 
+
 class SassParseError(SassError):
     """Error raised when parsing a Sass expression fails."""
 
     def format_prefix(self):
-        # TODO pointer
-        # TODO deal with newlines, etc
-        return """Error parsing expression:\n    %s""" % (self.expression,)
+        decorated_expr, line = add_error_marker(self.expression, self.expression_pos or -1)
+        return """Error parsing expression:\n""" + decorated_expr
 
 
 class SassEvaluationError(SassError):
     """Error raised when evaluating a parsed expression fails."""
 
     def format_prefix(self):
-        # TODO pointer
-        # TODO ast needs to know where each node came from for that to work  :(
-        # TODO deal with newlines, etc
-        return """Error evaluating expression:\n    %s""" % (self.expression,)
+        # TODO would be nice for the AST to have position information
+        # TODO might be nice to print the AST and indicate where the failure
+        # was?
+        decorated_expr, line = add_error_marker(self.expression, self.expression_pos or -1)
+        return """Error evaluating expression:\n""" + decorated_expr
diff --git a/scss/expression.py b/scss/expression.py
index 5c6dd15..73096b6 100644
--- a/scss/expression.py
+++ b/scss/expression.py
@@ -140,7 +140,7 @@ class Calculator(object):
                 ast = parser.goal()
             except SyntaxError, e:
                 if config.DEBUG:
-                    raise SassParseError(e, expression=expr)
+                    raise SassParseError(e, expression=expr, expression_pos=parser._char_pos)
                 else:
                     return None
             else:
@@ -316,8 +316,8 @@ class CallOp(Expression):
         except KeyError:
             try:
                 if kwargs:
-                    raise KeyError
-                # Fallback to single parameter:
+                    raise
+                # DEVIATION: Fall back to single parameter
                 funct = calculator.namespace.function(func_name, 1)
                 args = [args]
             except KeyError:
@@ -325,13 +325,7 @@ class CallOp(Expression):
                     log.error("Function not found: %s:%s", func_name, argspec_len, extra={'stack': True})
                     raise
         if funct:
-            try:
-                return funct(*args, **kwargs)
-            except Exception:
-                # TODO hoist me up since the rule is gone
-                #log.exception("Exception raised: %s in `%s' (%s)", e, expr, rule.file_and_line)
-                if not is_builtin_css_function(func_name):
-                    raise
+            return funct(*args, **kwargs)
 
         rendered_args = []
         for var, value in evald_argpairs:
@@ -487,10 +481,12 @@ class Parser(object):
     def __init__(self, scanner):
         self._scanner = scanner
         self._pos = 0
+        self._char_pos = 0
 
     def reset(self, input):
         self._scanner.reset(input)
         self._pos = 0
+        self._char_pos = 0
 
     def _peek(self, types):
         """
@@ -508,6 +504,7 @@ class Parser(object):
         Returns the matched text, and moves to the next token
         """
         tok = self._scanner.token(self._pos, set([type]))
+        self._char_pos = tok[0]
         if tok[2] != type:
             raise SyntaxError("SyntaxError[@ char %s: %s]" % (repr(tok[0]), "Trying to find " + type))
         self._pos += 1