JustusAdam / mustache

Haskell implementation of mustache templates
BSD 3-Clause "New" or "Revised" License
99 stars 32 forks source link

Proposal: Integral a => ToMustache a #46

Closed jchia closed 4 years ago

jchia commented 4 years ago
instance Integral a => ToMustache a where
  toMustache = Number . fromIntegral

This replaces the existing instances for Int and Integer and generalizes to other integral types like Word16.

Currently when putting a Word16 into a Value it can't be done directly and something like toMustache . toInteger is needed.

With this change, toMustache can be applied directly to a Word16 value.

JustusAdam commented 4 years ago

Sure that sounds sensible. Would you like to draft a PR for this?

jchia commented 4 years ago

This instance definition gets errors like this:

The constraint ‘Integral a’ is no smaller than the instance head ‘ToMustache a’

After enabling UndecidableInstances, other errors related to overlapping instances appeared. I'm just going to do it the naive way, defining instances for individual integral types.

jchia commented 4 years ago

47

JustusAdam commented 4 years ago

Small proposal: Why not define a function integralToMustache :: Integral a => a -> Value that is exposed and also makes up the implementation of ToMustache for the actual numeric values? This way we have the best of both worlds. Instances for most numeric types and a generic conversion function. :wink:

jchia commented 4 years ago

Sure, we can add integralToMustache and it can be called directly to convert some value whose Integral type α does not have a ToMustache instance. However, integralToMustache still cannot be called directly for converting [α], (α, Maybe a) etc to a Value. You would need respectively toMustache . fmap integralToMustcache and toMustache . bimap toMustache (fmap toMustache).

I wonder if there's a better way.

jchia commented 4 years ago

I have an alternative implementation involving UndecidableInstances. UndecidableInstances at face value seems scary but should be OK in this case as there are no actual instance loops. https://github.com/jchia/mustache/commit/2ac345a7a5ef7978f76c12e831f06517bd3b4bff

Unfortunately, I don't think I can also have a similar generalization for RealFloat because of duplicate instances.

@JustusAdam What do you think?

JustusAdam commented 4 years ago

Well the idea would be to use integralToMustache to define your own instances. I.e. if there is a type T with an Integral instance, you can write instance ToMustache T where toMustache = integralToMustache and then using toMustache on [T] or Maybe T should work just fine.

I don't much like the idea of using UndecidableInstances, even though there should never be an instance loop. I don't really know why, but I'd like if people could choose to implement their toMustache instance differently if they choose so.

I think it would make more sense to eventually write a conversion based on Generic that'll just take care of converting any type that has a Generic instance.

jchia commented 4 years ago

OK. I've already force-pushed to the PR to have integralToMustache.