brittany doesn't format newlines #250

Open
opened 2019-08-13 02:15:47 +02:00 by samuela · 6 comments
samuela commented 2019-08-13 02:15:47 +02:00 (Migrated from github.com)

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.

It seems as though brittany doesn't do anything to normalize the number of newlines between definitions: ```haskell module Lib ( Tensor(..) ) where data Tensor = Tensor {val :: String, foo :: String} deriving (Show) ``` isn't changed at all by brittany.
tfausak commented 2019-08-13 03:52:28 +02:00 (Migrated from github.com)

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.

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: ``` hs -- 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 2019-08-13 22:54:13 +02:00 (Migrated from github.com)

@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.

@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 2019-08-19 22:32:56 +02:00 (Migrated from github.com)

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.

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 2019-08-22 16:29:23 +02:00 (Migrated from github.com)

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.
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 2019-08-22 19:10:48 +02:00 (Migrated from github.com)

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.

> 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 ```haskell 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 2019-08-22 20:46:35 +02:00 (Migrated from github.com)

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:

  • How to handle comments? I know formatters for other languages that have this feature enabled by default, and that move comments rather strictly down to the next decl. E.g.

    foo :: Int
    foo = 42
    -- 42 is a nice number                       -- move this down?
    
    
    adder :: Int -> Int
    adder = (+5)
    

    Do you want to move the comment? What if there is a newline before the comment, too?

  • I assume that we want to keep type signature and corresponding equation together, no matter what. Even though both are distinct top-level elements.

    There also is the question of how to treat this:

    foo, bar :: Int
    foo = 10
    bar = 12
    

    This might not be so bad if we only ever delete too-many newlines, but for standardisation this is very relevant.

  • This applies to nested decls too, right? Or not? members of type class defs and instances too? between let/where elements? Is this configurable separately?

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: - How to handle comments? I know formatters for other languages that have this feature enabled by default, and that move comments rather strictly down to the next decl. E.g. ~~~~ foo :: Int foo = 42 -- 42 is a nice number -- move this down? adder :: Int -> Int adder = (+5) ~~~~ Do you want to move the comment? What if there is a newline before the comment, too? - I assume that we want to keep type signature and corresponding equation together, no matter what. Even though both are distinct top-level elements. There also is the question of how to treat this: ~~~~ foo, bar :: Int foo = 10 bar = 12 ~~~~ This might not be so bad if we only ever delete too-many newlines, but for standardisation this is very relevant. - This applies to nested decls too, right? Or not? members of type class defs and instances too? between let/where elements? Is this configurable separately?
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#250
There is no content yet.