sebastiaanvisser / clay

A CSS preprocessor as embedded Haskell.
Other
360 stars 73 forks source link

Add a bunch of instances. #166

Closed mankyKitty closed 3 years ago

mankyKitty commented 6 years ago

I'm trying to build support for Clay in another package, and I found the lack of particular instances to be a bit of a hindrance. This isn't in reference to a particular issue, just trying to help out a bit. :)

Added instances for 'Eq', 'Functor', and 'Foldable' for several of the data types. This is to allow for a bit more flexibility when trying to build things on top of this package. You can now compare two Selector values for equality, for example. Should you need such a thing.

Also, replaced the custom Fix type with a prepacked one from the fixplate package. Aside from leaning on already written code, there is a bunch of NiceThings(TM) that come with the fixplate package.

I've included a updateClasses function as example.

I would like to utilise some features from lens as well. But I'm considering making that a separate package as some people aren't a fan of pulling down the entire package.

seanparsons commented 6 years ago

So I'd rather not include fixplate and change the public types as a result, it'll mean this is binary incompatible and require a version bump for that.

The additional instances are great, but I would ask that you not reformat the imports however.

mankyKitty commented 6 years ago

Gah, import mangling is from editor muscle memory. I'll fix that up.

I should have realised I needed a version bump for the fixplate changes, my bad. Would you be open to discussing this change if it included the version change? I realise that this won't line up to any current WIP for the library.

seanparsons commented 6 years ago

@mankyKitty I'm fine with the use of fixplate, it seems mostly low impact.

mankyKitty commented 6 years ago

Neat! I'll keep at it.

mankyKitty commented 5 years ago

Oh darn I missed that, yeah that's not good. I'll see what I can do.

On Sun., 17 Nov. 2019, 8:36 pm Manuel Bärenz, notifications@github.com wrote:

@turion requested changes on this pull request.

That's a lot of good effort you put in there, and it's nice to have all those instances. But fixplate seems not to have seen updates in 3 years, and I'd rather not have it as a dependency if it's not actively maintained. Is there another way to add all instances you need without that dependency? (If really need be, with another, maintained, dependency?)

In src/Clay/Property.hs https://github.com/sebastiaanvisser/clay/pull/166#discussion_r347133695:

@@ -41,7 +43,7 @@ quote t = "\"" <> replace "\"" "\\"" t <> "\""


newtype Key a = Key { unKeys :: Prefixed }

  • deriving (Show, Monoid, IsString)

  • deriving (Show, Monoid, IsString,Eq)

⬇️ Suggested change

  • deriving (Show, Monoid, IsString,Eq)

  • deriving (Show, Monoid, IsString, Eq)


In src/Clay/Property.hs https://github.com/sebastiaanvisser/clay/pull/166#discussion_r347133718:

@@ -58,7 +60,7 @@ instance Val Text where

value t = Value (Plain t)

newtype Literal = Literal Text

  • deriving (Show, Monoid, IsString)

  • deriving (Show, Monoid, IsString,Eq)

Same formatting issue

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sebastiaanvisser/clay/pull/166?email_source=notifications&email_token=AAGKAA6YWVFWM2W6LXYUQ6TQUE3DJA5CNFSM4EZ5VTJKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCL2I7TA#pullrequestreview-318017484, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGKAA22DMZR2UXDZJT5EXLQUE3DJANCNFSM4EZ5VTJA .

github-actions[bot] commented 3 years ago

This issue has not seen any activity in a long time. If no further activity occurs, it will be closed after ten weeks.