Repository: sqlfluff/sqlfluff

Issue:
CLI output has more newlines than it should.
I was testing a query to debug #389 and realised that the output of sqlfluff has changed unexpectedly.

# 0.3.5
```
$ sqlfluff lint test10.sql
== [test10.sql] FAIL
L:   4 | P:   3 | L003 | Indentation not hanging or a multiple of 4 spaces
L:   5 | P:   3 | L003 | Indentation not hanging or a multiple of 4 spaces
L:   5 | P:  77 | L008 | Commas should be followed by a single whitespace unless
                       | followed by a comment.
```

# 0.3.6
```
$ sqlfluff lint test10.sql

== [test10.sql] FAIL
L:   4 | P:   3 | L003 | Indentation not hanging or a multiple of 4 spaces
L:   5 | P:   3 | L003 | Indentation not hanging or a multiple of 4 spaces
L:   5 | P:  77 | L008 | Commas should be followed by a single whitespace unless
                       | followed by a comment.


```

Note the additional blank lines, which have found their way in.

This issue is to chase down where they're coming from and a) remove them and b) put in place a test case which catches this kind of thing in future.

Discussion:

Patch:
diff --git a/src/sqlfluff/cli/commands.py b/src/sqlfluff/cli/commands.py
index 002507991..b2497e7d1 100644
--- a/src/sqlfluff/cli/commands.py
+++ b/src/sqlfluff/cli/commands.py
@@ -13,11 +13,14 @@ import pstats
 from io import StringIO
 from benchit import BenchIt
 
+# To enable colour cross platform
+import colorama
+
 from .formatters import (
     format_rules,
     format_violation,
     format_linting_result_header,
-    format_linting_result_footer,
+    format_linting_stats,
     colorize,
     format_dialect_warning,
     format_dialects,
@@ -35,7 +38,7 @@ class RedWarningsFilter(logging.Filter):
     def filter(self, record):
         """Filter any warnings (or above) to turn them red."""
         if record.levelno >= logging.WARNING:
-            record.msg = colorize(record.msg, "red")
+            record.msg = colorize(record.msg, "red") + " "
         return True
 
 
@@ -56,9 +59,14 @@ def set_logging_level(verbosity, logger=None):
     # Don't propagate logging
     fluff_logger.propagate = False
 
+    # Enable colorama
+    colorama.init()
+
     # Set up the log handler to log to stdout
     handler = logging.StreamHandler(stream=sys.stdout)
-    handler.setFormatter(logging.Formatter("%(levelname)-10s %(message)s"))
+    # NB: the unicode character at the beginning is to squash any badly
+    # tamed ANSI colour statements, and return us to normality.
+    handler.setFormatter(logging.Formatter("\u001b[0m%(levelname)-10s %(message)s"))
     # Set up a handler to colour warnings red.
     handler.addFilter(RedWarningsFilter())
     if logger:
@@ -294,14 +302,14 @@ def lint(paths, format, nofail, logger=None, **kwargs):
 
     # Set up logging.
     set_logging_level(verbosity=verbose, logger=logger)
-
     # add stdin if specified via lone '-'
     if ("-",) == paths:
         # TODO: Remove verbose
         result = lnt.lint_string_wrapped(sys.stdin.read(), fname="stdin")
     else:
         # Output the results as we go
-        click.echo(format_linting_result_header(verbose=verbose))
+        if verbose >= 1:
+            click.echo(format_linting_result_header())
         try:
             # TODO: Remove verbose
             result = lnt.lint_paths(paths, ignore_non_existent_files=False)
@@ -316,7 +324,8 @@ def lint(paths, format, nofail, logger=None, **kwargs):
             )
             sys.exit(1)
         # Output the final stats
-        click.echo(format_linting_result_footer(result, verbose=verbose))
+        if verbose >= 1:
+            click.echo(format_linting_stats(result, verbose=verbose))
 
     if format == "json":
         click.echo(json.dumps(result.as_records()))
diff --git a/src/sqlfluff/cli/formatters.py b/src/sqlfluff/cli/formatters.py
index c185a70a1..014ea2596 100644
--- a/src/sqlfluff/cli/formatters.py
+++ b/src/sqlfluff/cli/formatters.py
@@ -89,50 +89,40 @@ def format_linting_stats(result, verbose=0):
     """Format a set of stats given a `LintingResult`."""
     text_buffer = StringIO()
     all_stats = result.stats()
-    if verbose >= 1:
-        text_buffer.write("==== summary ====\n")
-        if verbose >= 2:
-            output_fields = [
-                "files",
-                "violations",
-                "clean files",
-                "unclean files",
-                "avg per file",
-                "unclean rate",
-                "status",
-            ]
-            special_formats = {"unclean rate": "{0:.0%}"}
-        else:
-            output_fields = ["violations", "status"]
-            special_formats = {}
-        # Generate content tuples, applying special formats for some fields
-        summary_content = [
-            (
-                key,
-                special_formats[key].format(all_stats[key])
-                if key in special_formats
-                else all_stats[key],
-            )
-            for key in output_fields
+    text_buffer.write("==== summary ====\n")
+    if verbose >= 2:
+        output_fields = [
+            "files",
+            "violations",
+            "clean files",
+            "unclean files",
+            "avg per file",
+            "unclean rate",
+            "status",
         ]
-        # Render it all as a table
-        text_buffer.write(cli_table(summary_content, max_label_width=14))
+        special_formats = {"unclean rate": "{0:.0%}"}
+    else:
+        output_fields = ["violations", "status"]
+        special_formats = {}
+    # Generate content tuples, applying special formats for some fields
+    summary_content = [
+        (
+            key,
+            special_formats[key].format(all_stats[key])
+            if key in special_formats
+            else all_stats[key],
+        )
+        for key in output_fields
+    ]
+    # Render it all as a table
+    text_buffer.write(cli_table(summary_content, max_label_width=14))
     return text_buffer.getvalue()
 
 
-def format_linting_result_header(verbose=0):
+def format_linting_result_header():
     """Format the header of a linting result output."""
     text_buffer = StringIO()
-    if verbose >= 1:
-        text_buffer.write("==== readout ====\n")
-    return text_buffer.getvalue()
-
-
-def format_linting_result_footer(result, verbose=0):
-    """Format the footer of a linting result output given a `LintingResult`."""
-    text_buffer = StringIO()
-    text_buffer.write("\n")
-    text_buffer.write(format_linting_stats(result, verbose=verbose))
+    text_buffer.write("==== readout ====\n")
     return text_buffer.getvalue()
 
 
diff --git a/src/sqlfluff/cli/helpers.py b/src/sqlfluff/cli/helpers.py
index 11dd7212d..28f14b44b 100644
--- a/src/sqlfluff/cli/helpers.py
+++ b/src/sqlfluff/cli/helpers.py
@@ -3,16 +3,17 @@
 from io import StringIO
 import sys
 import textwrap
+from colorama import Fore, Style
 
 from .. import __version__ as pkg_version
 
 
 color_lookup = {
     # Unicode literals here are important for PY2
-    "red": u"\u001b[31m",
-    "green": u"\u001b[32m",
-    "blue": u"\u001b[36m",
-    "lightgrey": u"\u001b[30;1m",
+    "red": Fore.RED,
+    "green": Fore.GREEN,
+    "blue": Fore.BLUE,
+    "lightgrey": Fore.BLACK + Style.BRIGHT,
 }
 
 
@@ -23,7 +24,7 @@ def colorize(s, color=None):
     """
     if color:
         start_tag = color_lookup[color]
-        end_tag = u"\u001b[0m"
+        end_tag = Style.RESET_ALL
         return start_tag + s + end_tag
     else:
         return s
diff --git a/src/sqlfluff/core/parser/segments_base.py b/src/sqlfluff/core/parser/segments_base.py
index 7b120a0be..64fe110c6 100644
--- a/src/sqlfluff/core/parser/segments_base.py
+++ b/src/sqlfluff/core/parser/segments_base.py
@@ -373,22 +373,20 @@ class BaseSegment:
         """
         return ""
 
-    def _preface(self, ident, tabsize, pos_idx, raw_idx):
+    def _preface(self, ident, tabsize):
         """Returns the preamble to any logging."""
-        preface = " " * (ident * tabsize)
-        if self.is_meta:
-            preface += "[META] "
-        preface += self.__class__.__name__ + ":"
-        preface += " " * max(pos_idx - len(preface), 0)
-        if self.pos_marker:
-            preface += str(self.pos_marker)
-        else:
-            preface += "-"
-        sfx = self._suffix()
-        if sfx:
-            return preface + (" " * max(raw_idx - len(preface), 0)) + sfx
-        else:
-            return preface
+        padded_type = "{padding}{modifier}{type}".format(
+            padding=" " * (ident * tabsize),
+            modifier="[META] " if self.is_meta else "",
+            type=self.type + ":",
+        )
+        preface = "{pos:17}|{padded_type:60}  {suffix}".format(
+            pos=str(self.pos_marker) if self.pos_marker else "-",
+            padded_type=padded_type,
+            suffix=self._suffix() or "",
+        )
+        # Trim unnecessary whitespace before returning
+        return preface.rstrip()
 
     @property
     def _comments(self):
@@ -400,12 +398,10 @@ class BaseSegment:
         """Returns only the non-comment elements of this segment."""
         return [seg for seg in self.segments if seg.type != "comment"]
 
-    def stringify(self, ident=0, tabsize=4, pos_idx=60, raw_idx=80, code_only=False):
+    def stringify(self, ident=0, tabsize=4, code_only=False):
         """Use indentation to render this segment and it's children as a string."""
         buff = StringIO()
-        preface = self._preface(
-            ident=ident, tabsize=tabsize, pos_idx=pos_idx, raw_idx=raw_idx
-        )
+        preface = self._preface(ident=ident, tabsize=tabsize)
         buff.write(preface + "\n")
         if not code_only and self.comment_seperate and len(self._comments) > 0:
             if self._comments:
@@ -415,8 +411,6 @@ class BaseSegment:
                         seg.stringify(
                             ident=ident + 2,
                             tabsize=tabsize,
-                            pos_idx=pos_idx,
-                            raw_idx=raw_idx,
                             code_only=code_only,
                         )
                     )
@@ -427,8 +421,6 @@ class BaseSegment:
                         seg.stringify(
                             ident=ident + 2,
                             tabsize=tabsize,
-                            pos_idx=pos_idx,
-                            raw_idx=raw_idx,
                             code_only=code_only,
                         )
                     )
@@ -440,8 +432,6 @@ class BaseSegment:
                         seg.stringify(
                             ident=ident + 1,
                             tabsize=tabsize,
-                            pos_idx=pos_idx,
-                            raw_idx=raw_idx,
                             code_only=code_only,
                         )
                     )
@@ -966,11 +956,9 @@ class RawSegment(BaseSegment):
             self.__class__.__name__, self.pos_marker, self.raw
         )
 
-    def stringify(self, ident=0, tabsize=4, pos_idx=60, raw_idx=80, code_only=False):
+    def stringify(self, ident=0, tabsize=4, code_only=False):
         """Use indentation to render this segment and it's children as a string."""
-        preface = self._preface(
-            ident=ident, tabsize=tabsize, pos_idx=pos_idx, raw_idx=raw_idx
-        )
+        preface = self._preface(ident=ident, tabsize=tabsize)
         return preface + "\n"
 
     def _suffix(self):


Test patch:
diff --git a/test/cli/commands_test.py b/test/cli/commands_test.py
index 79347dd6c..23b64d4f4 100644
--- a/test/cli/commands_test.py
+++ b/test/cli/commands_test.py
@@ -35,11 +35,17 @@ def invoke_assert_code(ret_code=0, args=None, kwargs=None, cli_input=None):
     return result
 
 
+expected_output = """== [test/fixtures/linter/indentation_error_simple.sql] FAIL
+L:   2 | P:   4 | L003 | Indentation not hanging or a multiple of 4 spaces
+L:   5 | P:  10 | L010 | Inconsistent capitalisation of keywords.
+"""
+
+
 def test__cli__command_directed():
     """Basic checking of lint functionality."""
     result = invoke_assert_code(
         ret_code=65,
-        args=[lint, ["-n", "test/fixtures/linter/indentation_error_simple.sql"]],
+        args=[lint, ["test/fixtures/linter/indentation_error_simple.sql"]],
     )
     # We should get a readout of what the error was
     check_a = "L:   2 | P:   4 | L003"
@@ -47,6 +53,8 @@ def test__cli__command_directed():
     check_b = "Indentation"
     assert check_a in result.output
     assert check_b in result.output
+    # Finally check the WHOLE output to make sure that unexpected newlines are not added
+    assert result.output == expected_output
 
 
 def test__cli__command_dialect():
diff --git a/test/cli/formatters_test.py b/test/cli/formatters_test.py
index ab437c065..eaebaf79b 100644
--- a/test/cli/formatters_test.py
+++ b/test/cli/formatters_test.py
@@ -21,12 +21,6 @@ def test__cli__formatters__filename_nocol():
     assert escape_ansi(res) == "== [blahblah] PASS"
 
 
-def test__cli__formatters__filename_col():
-    """Explicity test color codes."""
-    res = format_filename("blah", success=False)
-    assert res == u"== [\u001b[30;1mblah\u001b[0m] \u001b[31mFAIL\u001b[0m"
-
-
 def test__cli__formatters__violation():
     """Test formatting violations.
 
diff --git a/test/core/parser/segments_base_test.py b/test/core/parser/segments_base_test.py
index 00643b6c8..af76857b3 100644
--- a/test/core/parser/segments_base_test.py
+++ b/test/core/parser/segments_base_test.py
@@ -50,8 +50,8 @@ def test__parser__base_segments_raw(raw_seg):
     # Check Formatting and Stringification
     assert str(raw_seg) == repr(raw_seg) == "<RawSegment: ([3](1, 1, 4)) 'foobar'>"
     assert (
-        raw_seg.stringify(ident=1, tabsize=2, pos_idx=20, raw_idx=35)
-        == "  RawSegment:       [3](1, 1, 4)   'foobar'\n"
+        raw_seg.stringify(ident=1, tabsize=2)
+        == "[3](1, 1, 4)     |  raw:                                                        'foobar'\n"
     )
     # Check tuple
     assert raw_seg.to_tuple() == ("raw", ())
@@ -76,10 +76,10 @@ def test__parser__base_segments_base(raw_seg_list):
     )
     # Check Formatting and Stringification
     assert str(base_seg) == repr(base_seg) == "<DummySegment: ([3](1, 1, 4))>"
-    assert base_seg.stringify(ident=1, tabsize=2, pos_idx=20, raw_idx=35) == (
-        "  DummySegment:     [3](1, 1, 4)\n"
-        "    RawSegment:     [3](1, 1, 4)   'foobar'\n"
-        "    RawSegment:     [9](1, 1, 10)  '.barfoo'\n"
+    assert base_seg.stringify(ident=1, tabsize=2) == (
+        "[3](1, 1, 4)     |  dummy:\n"
+        "[3](1, 1, 4)     |    raw:                                                      'foobar'\n"
+        "[9](1, 1, 10)    |    raw:                                                      '.barfoo'\n"
     )