lspitzner / brittany

haskell source code formatter
GNU Affero General Public License v3.0
691 stars 67 forks source link

brittany doesn't format newlines #250

Open samuela opened 5 years ago

samuela commented 5 years ago

It seems as though brittany doesn't do anything to normalize the number of newlines between definitions:

module Lib
    ( Tensor(..)
    )
where

data Tensor = Tensor {val :: String, foo :: String} deriving (Show)

isn't changed at all by brittany.

tfausak commented 5 years ago

I think this is intentional. The idea is that if you want to put some new lines between tricky bits of code that aren't necessarily separate declarations, Brittany will allow that. Also it allows for many styles of top-level formatting:

-- dense
foo = this
bar = that
baz = other

-- normal
foo = this

bar = that

baz = other

-- spacious (like python?)
foo = this

bar = that

baz = other

I think that's the current rationale, anyway. It would be nice if this was somehow configurable.

samuela commented 5 years ago

@tfausak Right, though it would be nice if there was some kind of consistency, ie. "at most 1 line between declarations" seems like a decent rule.

ElvishJerricco commented 5 years ago

I actually rather prefer that brittany doesn't ever mess with my blank lines. Sometimes a lot of space is intentional. I wouldn't mind seeing this as an config option though.

eborden commented 5 years ago

My preference is, no messing with new lines and no configuration.

  1. Extra whitespace or lack there of, as previously mentioned, is usually intentional.
  2. Extra configuration just adds more complexity to brittany and makes it more challenging to maintain and evolve. As it currently stands I'd like to shed some of the existing configuration.
samuela commented 5 years ago

Extra whitespace or lack there of, as previously mentioned, is usually intentional.

I would say that this may be the case on smaller projects with fewer contributors, but once you get loads of people contributing to the same codebase with different ideas of what spacing should look like it quickly becomes a mess. I also think it's worth pointing out that the same argument could be made for the rest of brittany's feature set or for any code formatting at all, eg. the user's idea of formatting

x-  (  y/ 3)--blah blah

was probably intentional, and therefore we shouldn't touch it.

In my view the whole point of code formatting is uniformity, and that's most people tend to expect from tools in this category.

lspitzner commented 5 years ago

I think this feature can be implemented in a way that does not intersect with other stuff, either by using a mapping on the annotations (we already pre-process them, so composing one more function is not really complex) or by giving the BDAnnotationPrior a new argument. The latter would touch a few places, but still be fairly contained.

So in this case I am currently leaning towards "this additional complexity might be worth it".

I agree that this should be disabled by default.

However, there are a couple of annoying design/requirement questions that I would like to address before approaching an implementation: