Newline after instance declaration

[pkamenarsky]

instance A.FromJSON Service where
  parseJSON =
    A.genericParseJSON $
      A.defaultOptions
        { A.fieldLabelModifier = A.camelTo2 '_' . drop 2
        }

gets formatted to

instance A.FromJSON Service where

  parseJSON =
    A.genericParseJSON $
      A.defaultOptions
        { A.fieldLabelModifier = A.camelTo2 '_' . drop 2
        }

Is this intentional? This is of course subjective, but the extra newline makes it look like instance A.FromJSON Service where is a separate thing from parseJSON, which it is not.

[mrkkrp]

It is intentional: this was my request. My logic here is that if there are several methods, then they should be separated by newlines and it's odd to "glue" the first method with instance header by omitting a newline there. Feel free to argue otherwise, we might change it later.

[mboes]

@mrkkrp The explanation makes sense, except that if we were to apply it consistently, we would do the same after where in a function definition. Most existing code doesn't do that - whether in a function or in instance declarations.

[pkamenarsky]

Maybe related: https://github.com/tweag/ormolu/issues/74.

A single instance ... line looks like a deriving instance or an empty instance at a quick glance to me.

A compromise would be to not collapse empty lines present in the original layout or conversely, introduce new ones where there were none before. This would allow the programmer to group semantic constructs as they see fit, which I'd argue is paramount for readability.

[mrkkrp]

My plan currently is to return to more subtle stylistic questions after initial release and making sure we can format all existing code. So, this one (and others tagged "style") is lower-priority for now.

[neongreen]

Agreed with the OP, seeing code like this is kinda weird:

-- | A category was found.
data CSRCategory
  = CSRCategory
      { csrcInfo :: CCategoryInfo
      , csrcDescription :: CMarkdown
      }
  deriving (Show, Generic)

instance A.ToJSON CSRCategory where

  toJSON = A.genericToJSON jsonOptions

instance ToSchema CSRCategory where

  declareNamedSchema = genericDeclareNamedSchema schemaOptions

[mrkkrp]

Perhaps the leading newline should be inserted only sometimes:

• When there is a single definition in the declaration (or in other words, there are no "separated" declarations there) we do not insert the newline.
• Otherwise we insert it, because there will be others between the functions anyway.

[Ciantic]

What makes this odd, is that it makes long list of these hard to read, e.g.

data Foo a = Foo a

data Boo a = Boo a

instance Functor Foo where
  fmap f foo = undefined

instance Applicative Foo where

  pure x = undefined

  f <*> foo = undefined

instance Functor Boo where
  fmap f foo = undefined

instance Applicative Boo where

  pure x = undefined

  f <*> foo = undefined

[mboes]

Omitting the blank line in all cases after the instance head makes sense.