Repository: timothycrosley/isort
Pull request #395 (2016-02-21 19:21:00)

Fix multiline sorting with force-alphabetical-sort
Raised in https://github.com/timothycrosley/isort/issues/364

The separate code for --force-alphabetical-sort has been removed and
a boolean flag added to handle sorting with or without case in the
_module_key function (which handles the sorting).

Note this commit alters these 2 tests:
test_alphabetic_sorting_no_newlines, test_alphabetic_sorting
This results in the lone imports being moved to the top of the file.
I think this is better as it means the force_alphabetical_sort tests
now behave similarly to the other tests in the file.

Please advise if it is ok to change the tests like this. 

The big red commit in isort.py is because lines 444 -> 468 were removed and the 
if statement was removed


Patch:
diff --git a/isort/isort.py b/isort/isort.py
index cc956eb1..21d73142 100644
--- a/isort/isort.py
+++ b/isort/isort.py
@@ -269,9 +269,13 @@ class SortImports(object):
         return self.index == self.number_of_lines
 
     @staticmethod
-    def _module_key(module_name, config, sub_imports=False):
+    def _module_key(module_name, config, sub_imports=False, ignore_case=False):
         prefix = ""
-        module_name = str(module_name)
+        if ignore_case:
+            module_name = str(module_name).lower()
+        else:
+            module_name = str(module_name)
+
         if sub_imports and config['order_by_type']:
             if module_name.isupper() and len(module_name) > 1:
                 prefix = "A"
@@ -332,14 +336,14 @@ class SortImports(object):
                 section_output.extend(comments_above)
             section_output.append(self._add_comments(self.comments['straight'].get(module), import_definition))
 
-    def _add_from_imports(self, from_modules, section, section_output):
+    def _add_from_imports(self, from_modules, section, section_output, ignore_case):
         for module in from_modules:
             if module in self.remove_imports:
                 continue
 
             import_start = "from {0} import ".format(module)
             from_imports = list(self.imports[section]['from'][module])
-            from_imports = nsorted(from_imports, key=lambda key: self._module_key(key, self.config, True))
+            from_imports = nsorted(from_imports, key=lambda key: self._module_key(key, self.config, True, ignore_case))
             if self.remove_imports:
                 from_imports = [line for line in from_imports if not "{0}.{1}".format(module, line) in
                                 self.remove_imports]
@@ -441,67 +445,44 @@ class SortImports(object):
         (at the index of the first import) sorted alphabetically and split between groups
 
         """
-        if self.config.get('force_alphabetical_sort', False):
-            from_output = []
-            straight_output = []
-            for section in itertools.chain(self.sections, self.config['forced_separate']):
-                straight_modules = list(self.imports[section]['straight'])
-                from_modules = list(self.imports[section]['from'].keys())
-
-                self._add_from_imports(from_modules, section, from_output)
-                self._add_straight_imports(straight_modules, section, straight_output)
-
-            new_from_output = []
-            new_straight_output = []
-            for line in from_output:
-                for element in line.split('\n'):
-                    new_from_output.append(element)
-            for line in straight_output:
-                for element in line.split('\n'):
-                    new_straight_output.append(element)
-
-
-            sorted_from = sorted(new_from_output, key=lambda import_string: import_string.lower())
-            sorted_straight = sorted(new_straight_output, key=lambda import_string: import_string.lower())
-            output = (sorted_from + [''] + sorted_straight) if (sorted_from and sorted_straight) else \
-                     (sorted_from or sorted_straight)
-        else:
-            output = []
-            for section in itertools.chain(self.sections, self.config['forced_separate']):
-                straight_modules = list(self.imports[section]['straight'])
-                straight_modules = nsorted(straight_modules, key=lambda key: self._module_key(key, self.config))
-                from_modules = sorted(list(self.imports[section]['from'].keys()))
-                from_modules = nsorted(from_modules, key=lambda key: self._module_key(key, self.config, ))
-
-                section_output = []
-                if self.config.get('from_first', False):
-                    self._add_from_imports(from_modules, section, section_output)
-                    self._add_straight_imports(straight_modules, section, section_output)
-                else:
-                    self._add_straight_imports(straight_modules, section, section_output)
-                    self._add_from_imports(from_modules, section, section_output)
-
-                if self.config.get('force_sort_within_sections', False):
-                    def by_module(line):
-                        line = re.sub('^from ', '', line)
-                        line = re.sub('^import ', '', line)
-                        if not self.config['order_by_type']:
-                            line = line.lower()
-                        return line
-                    section_output = nsorted(section_output, key=by_module)
-
-                if section_output:
-                    section_name = section
-                    if section_name in self.place_imports:
-                        self.place_imports[section_name] = section_output
-                        continue
-
-                    section_title = self.config.get('import_heading_' + str(section_name).lower(), '')
-                    if section_title:
-                        section_comment = "# {0}".format(section_title)
-                        if not section_comment in self.out_lines[0:1]:
-                            section_output.insert(0, section_comment)
-                    output += section_output + ([''] * self.config['lines_between_sections'])
+        sort_ignore_case = self.config.get('force_alphabetical_sort', False)
+
+        output = []
+        for section in itertools.chain(self.sections, self.config['forced_separate']):
+            straight_modules = list(self.imports[section]['straight'])
+            straight_modules = nsorted(straight_modules, key=lambda key: self._module_key(key, self.config))
+            from_modules = sorted(list(self.imports[section]['from'].keys()))
+            from_modules = nsorted(from_modules, key=lambda key: self._module_key(key, self.config, ))
+
+            section_output = []
+            if self.config.get('from_first', False):
+                self._add_from_imports(from_modules, section, section_output, sort_ignore_case)
+                self._add_straight_imports(straight_modules, section, section_output)
+            else:
+                self._add_straight_imports(straight_modules, section, section_output)
+                self._add_from_imports(from_modules, section, section_output, sort_ignore_case)
+
+            if self.config.get('force_sort_within_sections', False):
+                def by_module(line):
+                    line = re.sub('^from ', '', line)
+                    line = re.sub('^import ', '', line)
+                    if not self.config['order_by_type']:
+                        line = line.lower()
+                    return line
+                section_output = nsorted(section_output, key=by_module)
+
+            if section_output:
+                section_name = section
+                if section_name in self.place_imports:
+                    self.place_imports[section_name] = section_output
+                    continue
+
+                section_title = self.config.get('import_heading_' + str(section_name).lower(), '')
+                if section_title:
+                    section_comment = "# {0}".format(section_title)
+                    if not section_comment in self.out_lines[0:1]:
+                        section_output.insert(0, section_comment)
+                output += section_output + ([''] * self.config['lines_between_sections'])
 
         while [character.strip() for character in output[-1:]] == [""]:
             output.pop()


Test patch:
diff --git a/test_isort.py b/test_isort.py
index 06c06ca3..0ddaffc3 100644
--- a/test_isort.py
+++ b/test_isort.py
@@ -1494,24 +1494,35 @@ def test_comment_at_top_of_file():
 
 def test_alphabetic_sorting():
     """Test to ensure isort correctly handles top of file comments"""
-    test_input = ("from django.contrib.gis.geos import GEOSException\n"
+    test_input = ("import unittest\n"
+                  "\n"
+                  "import ABC\n"
+                  "import Zope\n"
+                  "from django.contrib.gis.geos import GEOSException\n"
                   "from plone.app.testing import getRoles\n"
                   "from plone.app.testing import ManageRoles\n"
                   "from plone.app.testing import setRoles\n"
                   "from Products.CMFPlone import utils\n"
-                  "\n"
-                  "import ABC\n"
-                  "import unittest\n"
-                  "import Zope\n")
+                  )
     options = {'force_single_line': True,
                'force_alphabetical_sort': True, }
-    assert SortImports(file_contents=test_input, **options).output == test_input
+
+    output = SortImports(file_contents=test_input, **options).output
+    assert output == test_input
 
     test_input = ("# -*- coding: utf-8 -*-\n"
                   "from django.db import models\n")
     assert SortImports(file_contents=test_input).output == test_input
 
 
+def test_alphabetic_sorting_multi_line():
+    """Test to ensure isort correctly handles multiline import see: issue 364"""
+    test_input = ("from a import (CONSTANT_A, cONSTANT_B, CONSTANT_C, CONSTANT_D, CONSTANT_E,\n"
+                  "               CONSTANT_F, CONSTANT_G, CONSTANT_H, CONSTANT_I, CONSTANT_J)\n")
+    options = {'force_alphabetical_sort': True, }
+    assert SortImports(file_contents=test_input, **options).output == test_input
+
+
 def test_comments_not_duplicated():
     """Test to ensure comments aren't duplicated: issue 303"""
     test_input = ('from flask import url_for\n'
@@ -1596,11 +1607,11 @@ def test_alphabetic_sorting_no_newlines():
     test_output = SortImports(file_contents=test_input,force_alphabetical_sort=True).output
     assert test_input == test_output
 
-    test_input = ('from a import b\n'
-                  '\n'
-                  'import os\n'
+    test_input = ('import os\n'
                   'import unittest\n'
                   '\n'
+                  'from a import b\n'
+                  '\n'
                   '\n'
                   'print(1)\n')
     test_output = SortImports(file_contents=test_input,force_alphabetical_sort=True, lines_after_imports=2).output