Repository: timothycrosley/isort
Pull request #631 (2018-01-19 15:29:19)

Don't add un-neccesary line break in grouped vertical grid.
Prior to this change, isort was always adding 1 to the current line length when
determining whether or not the current line had got too long when outputting a
vertical grid.

- For imports that are not last, this is fine.  We need to account for the comma
  after the import.
- For the final import in vertical grid mode this is also fine.  We need to
  account for the closing `)`.
- For the final import in the grouped vertical grid we put the closing ) on the
  next line, so adding 1 to the line length causes isort to add un-neccesary
  line breaks when the final import would only just fit onto the line by one
  character.

For example, with a line length of 20:

```
from x import (
    abcd, efgh, ijkl
)
```

will end up reformatted as

```
from x import (
    abcd, efgh,
    ijkl
)
```

This is causing problems for us, as we run yapf on our code as well as isort,
and our imports change depending on which tool ran last.


Patch:
diff --git a/isort/isort.py b/isort/isort.py
index 99c3b23d..2e0223d1 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -693,12 +693,17 @@ class SortImports(object):
             "," if self.config['include_trailing_comma'] else "",
          )
 
-    def _output_vertical_grid_common(self, statement, imports, white_space, indent, line_length, comments):
+    def _output_vertical_grid_common(self, statement, imports, white_space, indent, line_length, comments, need_trailing_char):
         statement += self._add_comments(comments, "(") + self.line_separator + indent + imports.pop(0)
         while imports:
             next_import = imports.pop(0)
             next_statement = "{0}, {1}".format(statement, next_import)
-            if len(next_statement.split(self.line_separator)[-1]) + 1 > line_length:
+            current_line_length = len(next_statement.split(self.line_separator)[-1])
+            if imports or need_trailing_char:
+                # If we have more imports we need to account for a comma after this import
+                # We might also need to account for a closing ) we're going to add.
+                current_line_length += 1
+            if current_line_length > line_length:
                 next_statement = "{0},{1}{2}{3}".format(statement, self.line_separator, indent, next_import)
             statement = next_statement
         if self.config['include_trailing_comma']:
@@ -706,10 +711,10 @@ class SortImports(object):
         return statement
 
     def _output_vertical_grid(self, statement, imports, white_space, indent, line_length, comments):
-        return self._output_vertical_grid_common(statement, imports, white_space, indent, line_length, comments) + ")"
+        return self._output_vertical_grid_common(statement, imports, white_space, indent, line_length, comments, True) + ")"
 
     def _output_vertical_grid_grouped(self, statement, imports, white_space, indent, line_length, comments):
-        return self._output_vertical_grid_common(statement, imports, white_space, indent, line_length, comments) \
+        return self._output_vertical_grid_common(statement, imports, white_space, indent, line_length, comments, False) \
                + self.line_separator + ")"
 
     def _output_noqa(self, statement, imports, white_space, indent, line_length, comments):


Test patch:
diff --git a/test_isort.py b/test_isort.py
index de4f8c9c..3e16fae8 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -36,6 +36,8 @@ SHORT_IMPORT = "from third_party import lib1, lib2, lib3, lib4"
 
 SINGLE_FROM_IMPORT = "from third_party import lib1"
 
+SINGLE_LINE_LONG_IMPORT = "from third_party import lib1, lib2, lib3, lib4, lib5, lib5ab"
+
 REALLY_LONG_IMPORT = ("from third_party import lib1, lib2, lib3, lib4, lib5, lib6, lib7, lib8, lib9, lib10, lib11,"
                       "lib12, lib13, lib14, lib15, lib16, lib17, lib18, lib20, lib21, lib22")
 REALLY_LONG_IMPORT_WITH_COMMENT = ("from third_party import lib1, lib2, lib3, lib4, lib5, lib6, lib7, lib8, lib9, "
@@ -352,6 +354,13 @@ def test_output_modes():
                               multi_line_output=WrapModes.NOQA).output
     assert output_noqa == "from third_party import lib1, lib2, lib3, lib4, lib5, lib6, lib7, lib8, lib9, lib10, lib11, lib12, lib13, lib14, lib15, lib16, lib17, lib18, lib20, lib21, lib22  # NOQA comment\n"  # NOQA
 
+    test_output_vertical_grid_grouped_doesnt_wrap_early = SortImports(file_contents=SINGLE_LINE_LONG_IMPORT,
+                                                                      multi_line_output=WrapModes.VERTICAL_GRID_GROUPED,
+                                                                      line_length=40, indent='    ').output
+    assert test_output_vertical_grid_grouped_doesnt_wrap_early == ("from third_party import (\n"
+                                                                   "    lib1, lib2, lib3, lib4, lib5, lib5ab\n"
+                                                                   ")\n")
+
 
 def test_qa_comment_case():
     test_input = "from veryveryveryveryveryveryveryveryveryveryvery import X  # NOQA"