-
Notifications
You must be signed in to change notification settings - Fork 68
Support sum type layouting #294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This is a naive first pass at sum type support. This currently breaks 11
tests, some from comment misplacement, others in more exotic forms that
previously required high levels of context to be correctly laid out. The
solution for comment misplacement is probably trivial, but the highly
contextualized forms may need a bit more alchemy.
```
1) data type declarations record no matching single line layout
expected: Right
{-# LANGUAGE ScopedTypeVariables #-}
-- brittany { lconfig_allowSinglelineRecord: true }
data MyRecord = forall a . Show a => Bar
{ foo :: abittoolongbutnotvery -> abittoolongbutnotvery
}
but got: Right
{-# LANGUAGE ScopedTypeVariables #-}
-- brittany { lconfig_allowSinglelineRecord: true }
data MyRecord = forall a
. Show a =>
Bar
{ foo :: abittoolongbutnotvery -> abittoolongbutnotvery
}
2) data type declarations record forall constraint multiline
expected: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord
= forall a
. LooooooooooooooooooooongConstraint a =>
LoooooooooooongConstructor
{ foo :: abittoolongbutnotvery -> abittoolongbutnotvery
}
but got: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord = forall a
. LooooooooooooooooooooongConstraint a =>
LoooooooooooongConstructor
{ foo :: abittoolongbutnotvery -> abittoolongbutnotvery
}
3) data type declarations record forall constraint multiline more
expected: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord
= forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor
{ a :: a
, b :: b
}
but got: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord = forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor
{ a :: a
, b :: b
}
4) data type declarations plain with forall and constraint
expected: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyStruct
= forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor (ToBriDocM BriDocNumbered)
(ToBriDocM BriDocNumbered)
(ToBriDocM BriDocNumbered)
but got: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyStruct = forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor (ToBriDocM BriDocNumbered)
(ToBriDocM BriDocNumbered)
(ToBriDocM BriDocNumbered)
5) data type declarations record with many features
expected: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord
= forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor
{ foo, foo2
:: loooooooooooooooooooooooooooooooong
-> loooooooooooooooooooooooooooooooong
, bar :: a
, bazz :: b
}
deriving Show
but got: Right
{-# LANGUAGE ScopedTypeVariables #-}
data MyRecord = forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor
{ foo, foo2
:: loooooooooooooooooooooooooooooooong
-> loooooooooooooooooooooooooooooooong
, bar :: a
, bazz :: b
}
deriving Show
6) data type declarations single record existential
expected: Right
{-# LANGUAGE ExistentialQuantification #-}
data Foo = forall a . Show a => Bar
{ foo :: a
}
but got: Right
{-# LANGUAGE ExistentialQuantification #-}
data Foo = forall a
. Show a =>
Bar
{ foo :: a
}
7) data type declarations record multiple types existential
expected: Right
{-# LANGUAGE ExistentialQuantification #-}
data Foo = forall a b . (Show a, Eq b) => Bar
{ foo :: a
, bars :: b
}
but got: Right
{-# LANGUAGE ExistentialQuantification #-}
data Foo = forall a b
. (Show a, Eq b) =>
Bar
{ foo :: a
, bars :: b
}
8) data type declarations record newline comment
expected: Right
data MyRecord = MyRecord
{ a :: Int
-- comment
, b :: Int
}
but got: Right
data MyRecord = MyRecord
{ a :: Int
-- comment
, b :: Int
}
9) data type declarations comment before equal sign
expected: Right
{-# LANGUAGE ExistentialQuantification #-}
data MyRecord
-- test comment
= forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor a b
but got: Right
{-# LANGUAGE ExistentialQuantification #-}
data MyRecord
-- test comment
= forall a b
. ( Loooooooooooooooooooooooooooooooong a
, Loooooooooooooooooooooooooooooooong b
) =>
MyConstructor a b
10) data type declarations large record with a comment
expected: Right
data XIILqcacwiuNiu = XIILqcacwiuNiu
{ oyyFtvbepgbOge_pebzVmuftEijwuj :: Jgtoyuh HessJvNlo
, wloQsiskdoxJop_xatiKrwedOxtu :: Jgtoyuh [Inotg]
, mmmJjcqtemyIyo_ovosDoreKeeoyamvove :: Jgtoyuh Eujo
, mbiIatelofxOzr_uluxNngiiMjah :: Jgtoyuh HessJvNlo
, obxIskfcxpkIkb_uuviTuevcSkrgo :: Jgtoyuh Int
, wqrAtuvuecoHwr_ilotNxbuPleo :: Jgtoyuh Ufaxdeq
, lofAfuebdhpLuv_cnekPoyFxmg :: Jgtoyuh Ufaxdeq
, ouoFugtawzvUpk_oupiLzptugy :: Jgtoyuh Eujo
, iqiXjtziwogNsa_uiyvSunaTtgUsf3 :: Jgtoyuh Oaivn
, odbIriaqnojUlz_onotoWuunehIpuy :: Jgtoyuh Eujo
, opjUxtkxzkiKse_luqjuZazt
:: Jgtoyuh [(Eujo, Int, Int, Int, Int, Int, NELUxro)]
-- , jcqRaqznxfhIpa_ywevMezmoYkutuwa :: Jgtoyuh ()
, vayOmuasyphOfd_bcsVljmvt :: Jgtoyuh Eujo
, rifArahilooRax_ufikecqdImsv :: Jgtoyuh Oaivn
, raqKtopcpszDwb_oqocubasZuqjcryoDojGkw :: Jgtoyuh Oaivn
, mluJiilpcijUtt_gaisklifVekfeyagRmfbyzz :: Jgtoyuh Oaivn
, oqhPaahjupaSmi_gamwwoovKyxznecvEayluc :: Jgtoyuh Oaivn
, mazFubimwebZpa_itidehDodiDlboz :: Jgtoyuh Vrep
, jeyOcuesexaYoy_vpqn :: Jgtoyuh ()
}
but got: Right
data XIILqcacwiuNiu = XIILqcacwiuNiu
{ oyyFtvbepgbOge_pebzVmuftEijwuj :: Jgtoyuh HessJvNlo
, wloQsiskdoxJop_xatiKrwedOxtu :: Jgtoyuh [Inotg]
, mmmJjcqtemyIyo_ovosDoreKeeoyamvove :: Jgtoyuh Eujo
, mbiIatelofxOzr_uluxNngiiMjah :: Jgtoyuh HessJvNlo
, obxIskfcxpkIkb_uuviTuevcSkrgo :: Jgtoyuh Int
, wqrAtuvuecoHwr_ilotNxbuPleo :: Jgtoyuh Ufaxdeq
, lofAfuebdhpLuv_cnekPoyFxmg :: Jgtoyuh Ufaxdeq
, ouoFugtawzvUpk_oupiLzptugy :: Jgtoyuh Eujo
, iqiXjtziwogNsa_uiyvSunaTtgUsf3 :: Jgtoyuh Oaivn
, odbIriaqnojUlz_onotoWuunehIpuy :: Jgtoyuh Eujo
, opjUxtkxzkiKse_luqjuZazt
:: Jgtoyuh [(Eujo, Int, Int, Int, Int, Int, NELUxro)]
-- , jcqRaqznxfhIpa_ywevMezmoYkutuwa :: Jgtoyuh ()
, vayOmuasyphOfd_bcsVljmvt :: Jgtoyuh Eujo
, rifArahilooRax_ufikecqdImsv :: Jgtoyuh Oaivn
, raqKtopcpszDwb_oqocubasZuqjcryoDojGkw :: Jgtoyuh Oaivn
, mluJiilpcijUtt_gaisklifVekfeyagRmfbyzz :: Jgtoyuh Oaivn
, oqhPaahjupaSmi_gamwwoovKyxznecvEayluc :: Jgtoyuh Oaivn
, mazFubimwebZpa_itidehDodiDlboz :: Jgtoyuh Vrep
, jeyOcuesexaYoy_vpqn :: Jgtoyuh ()
}
11) data type declarations records in sum
expected: Right
-- brittany {lconfig_indentPolicy: IndentPolicyLeft }
data Foo
= Bar
{ foo :: Int -- hello
, bar :: Foo
-- how are you
}
| Baz
| Biz
{ foo :: Int
, bar :: Foo
}
but got: Right
-- brittany {lconfig_indentPolicy: IndentPolicyLeft }
data Foo
= Bar
{ foo :: Int -- hello
, bar :: Foo
-- how are you
}
| Baz
| Biz
{ foo :: Int
, bar :: Foo
}
```
|
Super excited to see progress here 👏, thanks Evan! I just happened to notice something in the test cases that might be worth a little bike-shedding, if it's still easy to change things at this stage: This is interesting to me in terms of consistency, given the possible non-sum styles of: In our own code base, we've shifted between (1) and (2): our "before my time" Style Guide has examples in style (2), but we aren't sure it was intentional and most of the current team prefers (1), so we most often use that style now. Doing (1) for non-sums feels consistent with this sum test case: i.e. you only drop the Doing (2) for non-sum feels inconsistent to me since the I actually think aesthetically I do like this PR as-is, but I thought it was worthwhile to raise the consistency point. At the end of the day, "I don't care as long as it's automated" always wins (and sums-with-records is something we avoid generally), but if there's ever a time to actually dig in on a style discussion, it's probably when that automation itself is being written 😄 |
|
Sorry that I have not given any feedback yet, and thanks for starting on this feature. First I got sidetracked by refactoring around butcher/czipwith/barbies/commandline-/config-parsing and today I started restructuring the test-suite. This PR is a good start. but it needs more tests. Both to clear up the discussion on desired layout similar to what @pbrisbin mentions (and some existential/constraint/multiple-constructor/non-single-line combinations are non-trivial) and to get a tiny bit of coverage over the implementation. I noticed the (there are also some redundant patterns in this PR's code, but I'll just clean them up while fixing comment behaviour probably.) My workspace is currently a half-broken mess; I'll try to push something in the next days. |
|
Hello, is there any progress on this PR or is it stuck on something? It would be absolutely fantastic to have support for sum types in Brittany. |
|
@vaclavsvejcar This isn't stuck for any particular reason. I think myself and @lspitzner have just not had the time to dedicate to it. |
@lspitzner Getting the ball rolling on this topic.
This is a naive first pass at sum type support. This currently breaks 11
tests, some from comment misplacement, others in more exotic forms that
previously required high levels of context to be correctly laid out. The
solution for comment misplacement is probably trivial, but the highly
contextualized forms may need a bit more alchemy.