avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 147 forks source link

Imports: local vs. external packages #193

Open toastal opened 8 years ago

toastal commented 8 years ago

I'd like to see elm-format distinguish local imports from packages from the package manager. I think that it would really help a reader quickly be able to reference the where to find more information on the current import. Currently I'm ordering them by discipline, but I can't put a separating space between the two categories since elm-format removes the newline.

Example:

module Main exposing (main)

import Html exposing (..)
import Html.Attributes exposing (..)

import Model exposing (Model)
import Msg exposing (Msg(..))

main =
    ...
jinjor commented 7 years ago

I'm doing the same thing. Separating categories by new line seems good for reading. So, it would be nice to change 2 or more new lines to 1 new line.

About ordering, I personally don't need ordering by elm-format. Because ordering strategy may vary. Just for your information, I have the following preference in ordering.

  1. core modules (e.g. Dict, Task)
  2. modules from elm-lang packages (e.g. Html)
  3. modules from other packages (e.g. List.Extra)
  4. local common modules (e.g. Model.Something)
  5. local page modules (e.g. Page.Model.Something)
pixelprizm commented 7 years ago

Added detail, now that 0.7.0-exp sorts imports:

Before 0.7.0-exp, I did what jinjor describes (separate external and internal imports) by using comments. elm-format set the spacing like the following:

module App exposing (..)

-- Others'

import Array
import Debug
import Html as H
import Html.Attributes as HA
import Keyboard.Extra as KE
import Mouse
import Svg as S
import Svg.Attributes as SA
import Task
import Window

-- Mine

import Grid
import Config exposing (Config)
import ConfigPanel

type alias Model =
...

I thought there was too much whitespace, but it worked fine because there is at least some division.

Now, in 0.7.0-exp, elm-format sorts the imports, so they are all in one clump, with the comments drawn to the top:

module App exposing (..)

-- Others'
-- Mine

import Array
import Config exposing (Config)
import ConfigPanel
import Debug
import Grid
import Html as H
import Html.Attributes as HA
import Keyboard.Extra as KE
import Mouse
import Svg as S
import Svg.Attributes as SA
import Task
import Window

type alias Model =
...

So perhaps elm-format could sort each block of contiguous imports, using comments as separators (i.e. sort everything above -- Mine independently of everything below -- Mine). However, that may not be ideal since it's more complex to have multiple sorted blocks.

It would be really cool if elm-format could somehow read elm-package.json and/or the file system to determine what modules are external vs internal.

basile-henry commented 7 years ago

The original PR #279 for import sorting did split the sorted imports using comments as suggested by @egauderman.

I believe the reason this feature wasn't kept is because the new import sorting mechanism is a bit more involved: it merges redundant imports into one. This might be difficult to achieve while keeping sorting boundaries on comments. If the same import appears in two commented import blocks, which one do you keep?

It would be great to find a solution for this though, so if you come up with something I can try to implement it! :smile:

avh4 commented 7 years ago

Can you give some more details about why keeping imports in distinct groups is useful? Currently, elm-format is trying to remove the burden of managing imports so you can focus on the actual code. What are the scenarios that having separate import groups helps with? After trying ungrouped imports, what problems have arisen?

pixelprizm commented 7 years ago

I think there's a pretty big logical distinction between my own imports and external imports -- for internal imports, the reader/coder can go edit that source code to fix bugs or change functionality, whereas for external imports, the reader/coder is not expected to edit the source code.

That said, I'm recognizing that my main reason for wanting a separation is to maintain a habit I have from the past. And I do really like how the sorting feature means I don't have to worry about arranging the imports myself -- to me, that's worth losing the ability to distinguish internal vs external. But it would be great to have both -- that's why I think it would be best for elm-format to automatically split external and internal imports, as suggested by @toastal .

But I haven't really programmed much since 0.7.0-exp, so I personally don't have any concrete problems. I'll add them if I find them, or perhaps others like @toastal and @jinjor have some.

(side thought) However, this could even be a language-level syntax change, which would have such benefits as text editors, linters, and even elm-format being able to easily distinguish between the internal & external imports, which would have a lot of nice uses. For instance, text editors could easily implement a powerful "jump to definition" feature, or make it so you could just ctrl+click on an internal import to open it.

avh4 commented 7 years ago

For some more background, elm-format doesn't automatically group internal/external imports because of one of the original goals which was that formatting should depend only on the input code (and not on other files in the environment). Reading other files would be necessary to determine if a file were internal or external (and there would be a question of what to do if a particular import couldn't be found at all).

The reasons for that goal were performance, ease of configuration, and consistency in that you could be sure the same Elm code would always format the same way.

jinjor commented 7 years ago

For your information, I just found this statement from Python style guide.

https://www.python.org/dev/peps/pep-0008/#imports

Imports should be grouped in the following order:

  1. standard library imports
  2. related third party imports
  3. local application/library specific imports

You should put a blank line between each group of imports.

Also there is import-order package to check it.

I don't know much about Python so I cannot say this is the right reference for this discussion. But at least it seems some people are thinking in the same way.

rtfeldman commented 7 years ago

Anecdotally, I've been happy with how they are grouped now (in the experimental build). I don't give them any thought when adding them, and I've had no trouble scanning them to see what things are imported later.

robx commented 6 years ago

Coming from golang, there's a similar convention to python (standard library, third party, local). I like how gofmt handles this, which is to sort alphabetically within groups separated by empty lines. It's up to the developer to group using blank lines, the formatter need not determine where a package comes from.

And yes, I agree that sorting everything together the way elm-format does now makes for a hard-to-read import block.

rtfeldman commented 6 years ago

It's up to the developer to group using blank lines, the formatter need not determine where a package comes from.

I advocated for this at one point, but I now prefer the status quo.

I don't have any problems reading imports, and I spend zero time thinking about how they're ordered. I'd rather not have the option of spending nonzero time on them; for me, it'd be introducing the sort of stylistic distraction elm-format exists to prevent.

That said, I appreciate that others may feel differently about the readability of the status qo!

finegeometer commented 3 months ago

Just as a datapoint, I find this merging of imports to be sufficient reason to disable elm-format, when I would otherwise use it. I find it actively misleading to have local imports mixed with standard library imports.

I would be happy with any solution that lets me separate local imports from third-party ones. I do not care whether this is done automatically or whether it has to be done manually, as long as there is a way to do it at all.