Import lines not sorted #155

New Issue

2018-06-20T11:30:38+02:00

lingnand commented

2018-06-20 11:30:38 +02:00

(Migrated from github.com)

Thought this would be an easy fix?

lspitzner commented

2018-06-20 23:06:35 +02:00

(Migrated from github.com)

In no particular order, some thoughts:

Probably also want to uniq after sorting (?)
Imports often appear in groups. For example, I think it is somewhat common to have base imports, externals and internal (same library) imports in distinct groups (separated by blank lines) (not necessarily in that order..) Do we want cross-group reordering?
In case we want cross-group ordering, what should happen to the blank lines?
Probably also want to sort the items in explicit or hiding statements (import MyMod(unordered, mess, here), import MyMod hiding (don't, want, these))
Could also automatically structure into groups based on some heuristic (common module prefix etc.) but this seems to have a high risk with clashing with the user's preferred style.
Comments in general will be hard to treat correctly when re-ordering. Might make sense only to sort when there are no comments connected to any affected lines.

It is an important feature that brittany preserves user formatting (blank lines and comments) and sorting at least seems to have the potential to clash with that. But I also agree that unsorted imports tend to get messy over time.

In no particular order, some thoughts: - Probably also want to `uniq` after sorting (?) - Imports often appear in groups. For example, I think it is somewhat common to have base imports, externals and internal (same library) imports in distinct groups (separated by blank lines) (not necessarily in that order..) Do we want cross-group reordering? - In case we want cross-group ordering, what should happen to the blank lines? - Probably also want to sort the items in explicit or hiding statements (`import MyMod(unordered, mess, here)`, `import MyMod hiding (don't, want, these)`) - Could also automatically structure into groups based on some heuristic (common module prefix etc.) but this seems to have a high risk with clashing with the user's preferred style. - Comments in general will be hard to treat correctly when re-ordering. Might make sense only to sort when there are no comments connected to any affected lines. It is an important feature that brittany preserves user formatting (blank lines and comments) and sorting at least seems to have the potential to clash with that. But I also agree that unsorted imports tend to get messy over time.

👍 1

eschnett commented

2018-06-20 23:10:28 +02:00

(Migrated from github.com)

clang-format sorts the equivalent of import lines (#include and using namespace). It only sorts contiguous blocks. I think this is a good default behaviour that people may have come to expect.

`clang-format` sorts the equivalent of import lines (`#include` and `using namespace`). It only sorts contiguous blocks. I think this is a good default behaviour that people may have come to expect.

👍 13

ElvishJerricco commented

2018-06-21 19:06:07 +02:00

(Migrated from github.com)

I agree; I think people will expect their groups to be preserved. The group itself should be sorted, but the groups and the order of the groups with respect to each other should be preserved.

lingnand commented

2018-06-21 20:01:52 +02:00

(Migrated from github.com)

I currently use haskell-mode in emacs to sort the imports - it uses fairly simple rules like above mentioned i.e., sort the imports in continuous block of text not separated by whitespace.

I currently use `haskell-mode` in emacs to sort the imports - it uses fairly simple rules like above mentioned i.e., sort the imports in continuous block of text not separated by whitespace.

Josef-Thorne-A commented

2018-09-06 19:00:12 +02:00

(Migrated from github.com)

Translating haskell-mode's sort function into haskell seems like a good idea to start with.

Translating haskell-mode's [sort function](https://github.com/haskell/haskell-mode/blob/master/haskell-sort-imports.el) into haskell seems like a good idea to start with.

tfausak commented

2019-06-18 15:04:56 +02:00

(Migrated from github.com)

Although not exactly related, this is similar to #201.

hasufell commented

2020-02-01 17:03:02 +01:00

(Migrated from github.com)

The problem currently is that brittany breaks a single import across multiple lines, so you cannot select all imports and hit "sort". If there was an option to keep imports on a single line, this would be an ok workaround.

lspitzner commented

2020-03-31 01:32:51 +02:00

(Migrated from github.com)

I have been working on this, but handling comments is complex as usual. :( Any input on the following example case:

import XXX

-- my important comment

import CCC
-- comment about b
import BBB

The annoying part here is that the first comment is connected to the CCC import by exactprint right now, so even if we respect the "contiguous blocks" aspect and keep XXX at the top, the most straight-forward implementation still turns this into..

import XXX

-- comment about b
import BBB
-- my important comment

import CCC

But that can't be right. However I don't know how to decide when we want a comment to "belong" to an import statement next to it. I suppose in this case we would want this output:

import XXX

-- my important comment

-- comment about b
import BBB
import CCC

but the general case logic is hard to capture. Also, this will most likely require completely reinterpreting the output from ghc-exactprint - we need to cleverly attribute each comment to the most fitting neighbor (which exactprint gets wrong) while also marking "free-standing" comments as "independent"/"first-class" so they don't get affected by sorting at all.

I have been working on this, but handling comments is complex as usual. :( Any input on the following example case: ~~~~.hs import XXX -- my important comment import CCC -- comment about b import BBB ~~~~ The annoying part here is that the first comment is connected to the `CCC` import by exactprint right now, so even if we respect the "contiguous blocks" aspect and keep XXX at the top, the most straight-forward implementation still turns this into.. ~~~~.hs import XXX -- comment about b import BBB -- my important comment import CCC ~~~~ But that can't be right. However I don't know how to decide when we want a comment to "belong" to an import statement next to it. I suppose in this case we would want this output: ~~~~.hs import XXX -- my important comment -- comment about b import BBB import CCC ~~~~ but the general case logic is hard to capture. Also, this will most likely require completely reinterpreting the output from ghc-exactprint - we need to cleverly attribute each comment to the most fitting neighbor (which exactprint gets wrong) while _also_ marking "free-standing" comments as "independent"/"first-class" so they don't get affected by sorting at all.

lspitzner commented

2020-03-31 01:47:11 +02:00

(Migrated from github.com)

Also, I just notice that writing an algorithm on the plain textual data for imports would a) be relatively easy and b) make handling comments much less prone to error. Of course imports are simply a flat structure, from syntax-tree perspective, so it is somewhat expectable that you don't need a full parser here, but it is sad how ghc-exactprint turns this into a really complex exercise.

And this approach - working on the textual data with very superficial parsing - is probably exactly what stylish-haskell does. Hmm.

Also, I just notice that writing an algorithm on the plain textual data for imports would a) be relatively easy and b) make handling comments much less prone to error. Of course imports are simply a flat structure, from syntax-tree perspective, so it is somewhat expectable that you don't need a full parser here, but it is sad how ghc-exactprint turns this into a really complex exercise. And this approach - working on the textual data with very superficial parsing - is probably exactly what `stylish-haskell` does. Hmm.

tfausak commented

2020-12-11 18:32:33 +01:00

(Migrated from github.com)

Done as part of #330 and released in version 0.13.1.0.

Sign in to join this conversation.

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#155