Open epsilonhalbe opened 4 years ago
Ormolu doesn't take length of lines into account. This problem you reported happens because those per-import comments are not attached to their imports, unlike Haddocks. Yet, we sort the imports in the import section, so we would need to sort comments as well to preserve the correspondence, which is a bit tricky to do with our current approach, but doable.
In this style of code the comments are also not preserved:
import Protolude
-- base
import Control.Monad (when)
import Control.Exception (bracket)
-- GLFW-b, qualified for clarity
import qualified Graphics.UI.GLFW as GLFW
-- gl, all types and funcs here will already start with "gl"
import Graphics.GL.Core33
import Graphics.GL.Types
With Ormolu the result is:
-- base
import Control.Exception (bracket)
import Control.Monad (when)
-- GLFW-b, qualified for clarity
-- gl, all types and funcs here will already start with "gl"
import Graphics.GL.Core33
import Graphics.GL.Types
import qualified Graphics.UI.GLFW as GLFW
import Protolude
And I wanted to ask if is it possible to preserve the order. I would prefer Protolude to be the first.
I strongly support the claim of @rainbyte that when a programmer lets blank lines or comments in the middle of their import list, they do it voluntarily and it carries some information that should not be erased by the formatter.
I agree with @GuillaumeGen and @rainbyte and also strongly support respecting empty lines in import blocks. I'd also like to raise a particularly bad consequence of Ormolu's current behavior.
Say we're writing a library that extends QuickCheck
, which means lots of our modules will share the Test.QuickCheck
path. Say the programmer wrote:
module X where
-- Qualified due to complex reasons described in this comment
import qualified Test.QuickCheck.Exception as QCE
import Test.QuickCheck.Features
import Test.QuickCheck.Function
-- important comment about cyclic dependencies or something
import Test.QuickCheck.DecisionTreeTest.Types
import Test.QuickCheck.DecisionTreeTest.Impl
import DecisionTree.API
If we run this through Ormolu, we get:
module X where
-- Qualified due to complex reasons described in this comment
-- important comment about cyclic dependencies or something
import DecisionTree.API
import Test.QuickCheck.DecisionTreeTest.Impl
import Test.QuickCheck.DecisionTreeTest.Types
import qualified Test.QuickCheck.Exception as QCE
import Test.QuickCheck.Features
import Test.QuickCheck.Function
Not only I lost the separation of imports, my imports got mixed up with QuickCheck's because they share a qualified name prefix but I also completely lost the meaning of the two important comments. This example was crafted to showcase this happening in a minimal fashion, but we actually had this problem in our codebase and unfortunately lost important comments attached to imports.
It could be reasonable to define the rules as:
import X
import Y
import A
gets grouped and sorted, since space by itself is not enough indication the programmer had any special thoughts; But,
import X
import Y
-- Important comment! I thought about this
import A
Is left as is. The space AND comment clearly indicates a motive here. The tricky bit happens when we have to decide what to do with:
import X
import Y
-- Important comment! I thought about this
import A
import B
I'd actually be happy with grouping and sorting B
in either import block; if I want to keep it separate, I'll place another comment. Conservatively, could make sense to group it to the A
import block since its the closes to it.
Tagging @amesgen here as we spoke about this today.
I would like to use Ormolu in a codebase, but this is a blocker for now. Agree with @VictorCMiraldo's proposed solution.
formatting
will produce
which is not quite what I would expect but in multiple imports this is still clear that
MyClass
belongs toMyModule
unfortunately the following is not so clear anymorewill be formatted as
my assumption is that this is due to the length of class+module name