Repository: andialbrecht/sqlparse

Issue:
Staircase formatting on INSERT with multiple values
Here's an example where formatting results in a staircase effect: 
```
>>> sql = """insert into foo values (1, 'foo'), (2, 'bar'), (3, 'baz');"""
>>> print(sqlparse.format(sql, reindent=True))
insert into foo
values (1,
        'foo'), (2,
                 'bar'), (3,
                          'baz');
```


Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index 5d5b670..3e09159 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,13 +4,20 @@ python:
   - "3.4"
   - "3.5"
   - "3.6"
-  # - "3.7" # see https://github.com/travis-ci/travis-ci/issues/9815
   - "nightly"
   - "pypy"
   - "pypy3"
+# Enable 3.7 without globally enabling sudo and dist: xenial for other build jobs
+# See https://github.com/travis-ci/travis-ci/issues/9815
+matrix:
+  include:
+    - python: 3.7
+      dist: xenial
+      sudo: true
 
 install:
-  - pip install pytest pytest-cov codecov
+  - pip install -Uq pytest pytest-cov codecov
+  - pytest --version
 
 script:
   - pytest --cov=sqlparse
diff --git a/CHANGELOG b/CHANGELOG
index bd98537..17a7c9e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,9 +1,13 @@
 Development Version
 -------------------
 
+Notable Changes
+
+* Remove support for Python 3.3.
+
 Enhancements
 
-* New formatting option "--indent\_after_first" (pr345, by johshoff).
+* New formatting option "--indent_after_first" (pr345, by johshoff).
 * New formatting option "--indent_columns" (pr393, by digitalarbeiter).
 * Add UPSERT keyword (issue408).
 
@@ -13,14 +17,11 @@ Bug Fixes
 * Fix incorrect splitting of strings containing new lines (pr396, by fredyw).
 * Fix reindent issue for parenthesis (issue427, by fredyw).
 * Fix from( parsing issue (issue446, by fredyw)	.
-* Fix for get\_real_name() to return correct name (issue369, by fredyw).
+* Fix for get_real_name() to return correct name (issue369, by fredyw).
 * Wrap function params when wrap_after is set (pr398, by soloman1124).
 * Fix parsing of "WHEN name" clauses (pr418, by andrew deryabin).
 * Add missing EXPLAIN keyword (issue421).
-
-Internal Changes
-
-* Remove support for Python 3.3.
+* Fix issue with strip_comments causing a syntax error (fixes #425, by fredyw).
 
 
 Release 0.2.4 (Sep 27, 2017)
diff --git a/sqlparse/engine/grouping.py b/sqlparse/engine/grouping.py
index 2cc7601..afc9123 100644
--- a/sqlparse/engine/grouping.py
+++ b/sqlparse/engine/grouping.py
@@ -225,8 +225,8 @@ def group_identifier_list(tlist):
     m_role = T.Keyword, ('null', 'role')
     sqlcls = (sql.Function, sql.Case, sql.Identifier, sql.Comparison,
               sql.IdentifierList, sql.Operation)
-    ttypes = (T_NUMERICAL + T_STRING + T_NAME +
-              (T.Keyword, T.Comment, T.Wildcard))
+    ttypes = (T_NUMERICAL + T_STRING + T_NAME
+              + (T.Keyword, T.Comment, T.Wildcard))
 
     def match(token):
         return token.match(T.Punctuation, ',')
@@ -327,6 +327,18 @@ def align_comments(tlist):
         tidx, token = tlist.token_next_by(i=sql.Comment, idx=tidx)
 
 
+def group_values(tlist):
+    tidx, token = tlist.token_next_by(m=(T.Keyword, 'VALUES'))
+    start_idx = tidx
+    end_idx = -1
+    while token:
+        if isinstance(token, sql.Parenthesis):
+            end_idx = tidx
+        tidx, token = tlist.token_next(tidx)
+    if end_idx != -1:
+        tlist.group_tokens(sql.Values, start_idx, end_idx, extend=True)
+
+
 def group(stmt):
     for func in [
         group_comments,
@@ -354,6 +366,7 @@ def group(stmt):
 
         align_comments,
         group_identifier_list,
+        group_values,
     ]:
         func(stmt)
     return stmt
diff --git a/sqlparse/engine/statement_splitter.py b/sqlparse/engine/statement_splitter.py
index d610b5e..444b46a 100644
--- a/sqlparse/engine/statement_splitter.py
+++ b/sqlparse/engine/statement_splitter.py
@@ -65,8 +65,8 @@ class StatementSplitter(object):
             self._begin_depth = max(0, self._begin_depth - 1)
             return -1
 
-        if (unified in ('IF', 'FOR', 'WHILE') and
-                self._is_create and self._begin_depth > 0):
+        if (unified in ('IF', 'FOR', 'WHILE')
+                and self._is_create and self._begin_depth > 0):
             return 1
 
         if unified in ('END IF', 'END FOR', 'END WHILE'):
diff --git a/sqlparse/filters/others.py b/sqlparse/filters/others.py
index df4d861..408f5d2 100644
--- a/sqlparse/filters/others.py
+++ b/sqlparse/filters/others.py
@@ -23,9 +23,16 @@ class StripCommentsFilter(object):
             nidx, next_ = tlist.token_next(tidx, skip_ws=False)
             # Replace by whitespace if prev and next exist and if they're not
             # whitespaces. This doesn't apply if prev or next is a parenthesis.
-            if (prev_ is None or next_ is None or
-                    prev_.is_whitespace or prev_.match(T.Punctuation, '(') or
-                    next_.is_whitespace or next_.match(T.Punctuation, ')')):
+            if (prev_ is None or next_ is None
+                    or prev_.is_whitespace or prev_.match(T.Punctuation, '(')
+                    or next_.is_whitespace or next_.match(T.Punctuation, ')')):
+                # Insert a whitespace to ensure the following SQL produces
+                # a valid SQL (see #425). For example:
+                #
+                # Before: select a--comment\nfrom foo
+                # After: select a from foo
+                if prev_ is not None and next_ is None:
+                    tlist.tokens.insert(tidx, sql.Token(T.Whitespace, ' '))
                 tlist.tokens.remove(token)
             else:
                 tlist.tokens[tidx] = sql.Token(T.Whitespace, ' ')
diff --git a/sqlparse/filters/reindent.py b/sqlparse/filters/reindent.py
index 68593c0..02a60df 100644
--- a/sqlparse/filters/reindent.py
+++ b/sqlparse/filters/reindent.py
@@ -133,7 +133,7 @@ class ReindentFilter(object):
             first = next(identifiers.pop(0).flatten())
             num_offset = 1 if self.char == '\t' else self._get_offset(first)
 
-        if not tlist.within(sql.Function):
+        if not tlist.within(sql.Function) and not tlist.within(sql.Values):
             with offset(self, num_offset):
                 position = 0
                 for token in identifiers:
@@ -206,6 +206,23 @@ class ReindentFilter(object):
             if end_idx is not None:
                 tlist.insert_before(end_idx, self.nl())
 
+    def _process_values(self, tlist):
+        tlist.insert_before(0, self.nl())
+        tidx, token = tlist.token_next_by(i=sql.Parenthesis)
+        first_token = token
+        while token:
+            ptidx, ptoken = tlist.token_next_by(m=(T.Punctuation, ','),
+                                                idx=tidx)
+            if ptoken:
+                if self.comma_first:
+                    adjust = -2
+                    offset = self._get_offset(first_token) + adjust
+                    tlist.insert_before(ptoken, self.nl(offset))
+                else:
+                    tlist.insert_after(ptoken,
+                                       self.nl(self._get_offset(token)))
+            tidx, token = tlist.token_next_by(i=sql.Parenthesis, idx=tidx)
+
     def _process_default(self, tlist, stmts=True):
         self._split_statements(tlist) if stmts else None
         self._split_kwds(tlist)
diff --git a/sqlparse/keywords.py b/sqlparse/keywords.py
index 985334d..6e22c8e 100644
--- a/sqlparse/keywords.py
+++ b/sqlparse/keywords.py
@@ -13,10 +13,10 @@ from sqlparse import tokens
 
 def is_keyword(value):
     val = value.upper()
-    return (KEYWORDS_COMMON.get(val) or
-            KEYWORDS_ORACLE.get(val) or
-            KEYWORDS_PLPGSQL.get(val) or
-            KEYWORDS.get(val, tokens.Name)), value
+    return (KEYWORDS_COMMON.get(val)
+            or KEYWORDS_ORACLE.get(val)
+            or KEYWORDS_PLPGSQL.get(val)
+            or KEYWORDS.get(val, tokens.Name)), value
 
 
 SQL_REGEX = {
diff --git a/sqlparse/sql.py b/sqlparse/sql.py
index 5acce5c..1f56117 100644
--- a/sqlparse/sql.py
+++ b/sqlparse/sql.py
@@ -159,19 +159,23 @@ class TokenList(Token):
     def _get_repr_name(self):
         return type(self).__name__
 
-    def _pprint_tree(self, max_depth=None, depth=0, f=None):
+    def _pprint_tree(self, max_depth=None, depth=0, f=None, _pre=''):
         """Pretty-print the object tree."""
-        indent = u' | ' * depth
+        token_count = len(self.tokens)
         for idx, token in enumerate(self.tokens):
             cls = token._get_repr_name()
             value = token._get_repr_value()
 
+            last = idx == (token_count - 1)
+            pre = u'`- ' if last else u'|- '
+
             q = u'"' if value.startswith("'") and value.endswith("'") else u"'"
-            print(u"{indent}{idx:2d} {cls} {q}{value}{q}"
+            print(u"{_pre}{pre}{idx} {cls} {q}{value}{q}"
                   .format(**locals()), file=f)
 
             if token.is_group and (max_depth is None or depth < max_depth):
-                token._pprint_tree(max_depth, depth + 1, f)
+                parent_pre = u'   ' if last else u'|  '
+                token._pprint_tree(max_depth, depth + 1, f, _pre + parent_pre)
 
     def get_token_at_offset(self, offset):
         """Returns the token that is on position offset."""
@@ -235,8 +239,9 @@ class TokenList(Token):
         ignored too.
         """
         # this on is inconsistent, using Comment instead of T.Comment...
-        funcs = lambda tk: not ((skip_ws and tk.is_whitespace) or
-                                (skip_cm and imt(tk, t=T.Comment, i=Comment)))
+        funcs = lambda tk: not ((skip_ws and tk.is_whitespace)
+                                or (skip_cm and imt(tk,
+                                                    t=T.Comment, i=Comment)))
         return self._token_matching(funcs)[1]
 
     def token_next_by(self, i=None, m=None, t=None, idx=-1, end=None):
@@ -272,8 +277,9 @@ class TokenList(Token):
         if idx is None:
             return None, None
         idx += 1  # alot of code usage current pre-compensates for this
-        funcs = lambda tk: not ((skip_ws and tk.is_whitespace) or
-                                (skip_cm and imt(tk, t=T.Comment, i=Comment)))
+        funcs = lambda tk: not ((skip_ws and tk.is_whitespace)
+                                or (skip_cm and imt(tk,
+                                                    t=T.Comment, i=Comment)))
         return self._token_matching(funcs, idx, reverse=_reverse)
 
     def token_index(self, token, start=0):
@@ -615,3 +621,7 @@ class Begin(TokenList):
 
 class Operation(TokenList):
     """Grouping of operations"""
+
+
+class Values(TokenList):
+    """Grouping of values"""


Test patch:
diff --git a/tests/test_format.py b/tests/test_format.py
index f0dbcf2..a95b3d1 100644
--- a/tests/test_format.py
+++ b/tests/test_format.py
@@ -50,6 +50,19 @@ class TestFormat(object):
         sql = 'select-- foo\nfrom -- bar\nwhere'
         res = sqlparse.format(sql, strip_comments=True)
         assert res == 'select from where'
+        sql = 'select *-- statement starts here\n\nfrom foo'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == 'select * from foo'
+        sql = 'select * from foo-- statement starts here\nwhere'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == 'select * from foo where'
+        sql = 'select a-- statement starts here\nfrom foo'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == 'select a from foo'
+        sql = '--comment\nselect a-- statement starts here\n' \
+              'from foo--comment\nf'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == 'select a from foo f'
 
     def test_strip_comments_invalid_option(self):
         sql = 'select-- foo\nfrom -- bar\nwhere'
@@ -106,8 +119,8 @@ class TestFormat(object):
         assert f(s1) == "SELECT some_column LIKE 'value\r'"
         assert f(s2) == "SELECT some_column LIKE 'value\r'\nWHERE id = 1\n"
         assert f(s3) == "SELECT some_column LIKE 'value\\'\r' WHERE id = 1\n"
-        assert (f(s4) ==
-                "SELECT some_column LIKE 'value\\\\\\'\r' WHERE id = 1\n")
+        assert (f(s4)
+                == "SELECT some_column LIKE 'value\\\\\\'\r' WHERE id = 1\n")
 
 
 class TestFormatReindentAligned(object):
@@ -541,6 +554,49 @@ class TestFormatReindent(object):
             '       nvl(1)',
             'from dual'])
 
+    def test_insert_values(self):
+        # issue 329
+        f = lambda sql: sqlparse.format(sql, reindent=True)
+        s = 'insert into foo values (1, 2)'
+        assert f(s) == '\n'.join([
+            'insert into foo',
+            'values (1, 2)'])
+
+        s = 'insert into foo values (1, 2), (3, 4), (5, 6)'
+        assert f(s) == '\n'.join([
+            'insert into foo',
+            'values (1, 2),',
+            '       (3, 4),',
+            '       (5, 6)'])
+
+        s = 'insert into foo(a, b) values (1, 2), (3, 4), (5, 6)'
+        assert f(s) == '\n'.join([
+            'insert into foo(a, b)',
+            'values (1, 2),',
+            '       (3, 4),',
+            '       (5, 6)'])
+
+        f = lambda sql: sqlparse.format(sql, reindent=True,
+                                        comma_first=True)
+        s = 'insert into foo values (1, 2)'
+        assert f(s) == '\n'.join([
+            'insert into foo',
+            'values (1, 2)'])
+
+        s = 'insert into foo values (1, 2), (3, 4), (5, 6)'
+        assert f(s) == '\n'.join([
+            'insert into foo',
+            'values (1, 2)',
+            '     , (3, 4)',
+            '     , (5, 6)'])
+
+        s = 'insert into foo(a, b) values (1, 2), (3, 4), (5, 6)'
+        assert f(s) == '\n'.join([
+            'insert into foo(a, b)',
+            'values (1, 2)',
+            '     , (3, 4)',
+            '     , (5, 6)'])
+
 
 class TestOutputFormat(object):
     def test_python(self):
diff --git a/tests/test_grouping.py b/tests/test_grouping.py
index f26f8b3..c55df7c 100644
--- a/tests/test_grouping.py
+++ b/tests/test_grouping.py
@@ -49,10 +49,13 @@ def test_grouping_identifiers():
     assert str(parsed) == s
     assert isinstance(parsed.tokens[-1].tokens[3], sql.Identifier)
 
-    s = "INSERT INTO `test` VALUES('foo', 'bar');"
-    parsed = sqlparse.parse(s)[0]
-    types = [l.ttype for l in parsed.tokens if not l.is_whitespace]
-    assert types == [T.DML, T.Keyword, None, T.Keyword, None, T.Punctuation]
+    for s in ["INSERT INTO `test` VALUES('foo', 'bar');",
+              "INSERT INTO `test` VALUES(1, 2), (3, 4), (5, 6);",
+              "INSERT INTO `test(a, b)` VALUES(1, 2), (3, 4), (5, 6);"]:
+        parsed = sqlparse.parse(s)[0]
+        types = [l.ttype for l in parsed.tokens if not l.is_whitespace]
+        assert types == [T.DML, T.Keyword, None, None, T.Punctuation]
+        assert isinstance(parsed.tokens[6], sql.Values)
 
     s = "select 1.0*(a+b) as col, sum(c)/sum(d) from myschema.mytable"
     parsed = sqlparse.parse(s)[0]
diff --git a/tests/test_parse.py b/tests/test_parse.py
index 4fad526..f2a2bda 100644
--- a/tests/test_parse.py
+++ b/tests/test_parse.py
@@ -313,59 +313,59 @@ def test_pprint():
 
     p._pprint_tree(f=output)
     pprint = '\n'.join([
-        " 0 DML 'select'",
-        " 1 Whitespace ' '",
-        " 2 IdentifierList 'a0, b0...'",
-        " |  0 Identifier 'a0'",
-        " |  |  0 Name 'a0'",
-        " |  1 Punctuation ','",
-        " |  2 Whitespace ' '",
-        " |  3 Identifier 'b0'",
-        " |  |  0 Name 'b0'",
-        " |  4 Punctuation ','",
-        " |  5 Whitespace ' '",
-        " |  6 Identifier 'c0'",
-        " |  |  0 Name 'c0'",
-        " |  7 Punctuation ','",
-        " |  8 Whitespace ' '",
-        " |  9 Identifier 'd0'",
-        " |  |  0 Name 'd0'",
-        " | 10 Punctuation ','",
-        " | 11 Whitespace ' '",
-        " | 12 Float 'e0'",
-        " 3 Whitespace ' '",
-        " 4 Keyword 'from'",
-        " 5 Whitespace ' '",
-        " 6 Identifier '(selec...'",
-        " |  0 Parenthesis '(selec...'",
-        " |  |  0 Punctuation '('",
-        " |  |  1 DML 'select'",
-        " |  |  2 Whitespace ' '",
-        " |  |  3 Wildcard '*'",
-        " |  |  4 Whitespace ' '",
-        " |  |  5 Keyword 'from'",
-        " |  |  6 Whitespace ' '",
-        " |  |  7 Identifier 'dual'",
-        " |  |  |  0 Name 'dual'",
-        " |  |  8 Punctuation ')'",
-        " |  1 Whitespace ' '",
-        " |  2 Identifier 'q0'",
-        " |  |  0 Name 'q0'",
-        " 7 Whitespace ' '",
-        " 8 Where 'where ...'",
-        " |  0 Keyword 'where'",
-        " |  1 Whitespace ' '",
-        " |  2 Comparison '1=1'",
-        " |  |  0 Integer '1'",
-        " |  |  1 Comparison '='",
-        " |  |  2 Integer '1'",
-        " |  3 Whitespace ' '",
-        " |  4 Keyword 'and'",
-        " |  5 Whitespace ' '",
-        " |  6 Comparison '2=2'",
-        " |  |  0 Integer '2'",
-        " |  |  1 Comparison '='",
-        " |  |  2 Integer '2'",
+        "|- 0 DML 'select'",
+        "|- 1 Whitespace ' '",
+        "|- 2 IdentifierList 'a0, b0...'",
+        "|  |- 0 Identifier 'a0'",
+        "|  |  `- 0 Name 'a0'",
+        "|  |- 1 Punctuation ','",
+        "|  |- 2 Whitespace ' '",
+        "|  |- 3 Identifier 'b0'",
+        "|  |  `- 0 Name 'b0'",
+        "|  |- 4 Punctuation ','",
+        "|  |- 5 Whitespace ' '",
+        "|  |- 6 Identifier 'c0'",
+        "|  |  `- 0 Name 'c0'",
+        "|  |- 7 Punctuation ','",
+        "|  |- 8 Whitespace ' '",
+        "|  |- 9 Identifier 'd0'",
+        "|  |  `- 0 Name 'd0'",
+        "|  |- 10 Punctuation ','",
+        "|  |- 11 Whitespace ' '",
+        "|  `- 12 Float 'e0'",
+        "|- 3 Whitespace ' '",
+        "|- 4 Keyword 'from'",
+        "|- 5 Whitespace ' '",
+        "|- 6 Identifier '(selec...'",
+        "|  |- 0 Parenthesis '(selec...'",
+        "|  |  |- 0 Punctuation '('",
+        "|  |  |- 1 DML 'select'",
+        "|  |  |- 2 Whitespace ' '",
+        "|  |  |- 3 Wildcard '*'",
+        "|  |  |- 4 Whitespace ' '",
+        "|  |  |- 5 Keyword 'from'",
+        "|  |  |- 6 Whitespace ' '",
+        "|  |  |- 7 Identifier 'dual'",
+        "|  |  |  `- 0 Name 'dual'",
+        "|  |  `- 8 Punctuation ')'",
+        "|  |- 1 Whitespace ' '",
+        "|  `- 2 Identifier 'q0'",
+        "|     `- 0 Name 'q0'",
+        "|- 7 Whitespace ' '",
+        "`- 8 Where 'where ...'",
+        "   |- 0 Keyword 'where'",
+        "   |- 1 Whitespace ' '",
+        "   |- 2 Comparison '1=1'",
+        "   |  |- 0 Integer '1'",
+        "   |  |- 1 Comparison '='",
+        "   |  `- 2 Integer '1'",
+        "   |- 3 Whitespace ' '",
+        "   |- 4 Keyword 'and'",
+        "   |- 5 Whitespace ' '",
+        "   `- 6 Comparison '2=2'",
+        "      |- 0 Integer '2'",
+        "      |- 1 Comparison '='",
+        "      `- 2 Integer '2'",
         ""])
     assert output.getvalue() == pprint