Simplify TH configuration #169

Open
mgsloan opened this Issue Dec 5, 2012 · 3 comments

Projects

None yet

3 participants

@mgsloan
Collaborator

The current datatypes for configuring the template haskell generation are quite complicated:

(LensFlag) https://github.com/ekmett/lens/blob/master/src/Control/Lens/TH.hs#L77
(LensRules) https://github.com/ekmett/lens/blob/master/src/Control/Lens/TH.hs#L145

Specifically, if we want to add support for prisms, this configuration becomes even more complicated. Since this is the addition that is motivating these changes, I'll mostly be discussing it and Iso.

First off, here are the current configuration options for Isos:

-- | Handle singleton constructors specially.
handleSingletons  :: Simple Lens LensRules Bool

-- | When building a singleton 'Iso' (or 'Lens') for a record constructor, build both
-- the 'Iso' (or 'Lens') for the record and the one for the field.
singletonAndField :: Simple Lens LensRules Bool

-- | Use 'Iso' for singleton constructors (otherwise 'Lens' is used).
singletonIso      :: Simple Lens LensRules Bool

-- | Expect a single constructor, single field newtype or data type.
singletonRequired :: Simple Lens LensRules Bool

defaultRules uses SingletonIso and SingletonAndField.
isoRules uses SingletonRequired, HandleSingletons, and SingletonAndField.
classyRules and lensRules explicitly disable generating isomorphisms.

One possibility for wedging in would be removing handleSingletons, and adding the following:

-- | Handle constructors specially.
handleConstructors :: Simple Lens LensRules Bool

-- | Generate a 'Prism' for each constructor.
compoundPrism :: Simple Lens LensRules Bool

It occurs to me that if we're going to do tuples for prisms, then may as well also do tuples for Isos! So now we have singletonIso, compoundIso, singletonPrism, compoundPrism. Feels like too many degrees of freedom to me!

So, really what all these options come down to is "which type do I use to represent constructors"? The options are the usual culprits: lens, traversal, iso, or prism.

Here are my current thoughts on how it might be sliced:

data CommonFlag
  = SimpleTypes  -- Used to be "SimpleLenses"
  | PartialFunctions -- Used to be "PartialLenses"
  | GenerateSignatures

data CtorFlag
  = CtorIso    -- ^ Isos will be used for single constructor datatypes
  | CtorPrism  -- ^ Prisms will be used for multiple constructor datatypes (and single if CtorIso is off)
-- CtorLens / CtorTraversal? Useful?

data FieldFlag
  = FieldTraversals  -- Used to be "BuildTraversals"
  | CreateInstance
  | CreateClass
  | ClassRequired

I've split the flags into three ADTs because generating lensy things for constructors is really quite different than generating lenses for fields. The code is even split into the two functions makeIsoLenses and makeFieldLenses. The LensRules datatype, repeated below, also needs to change.

data LensRules = LensRules
  { _lensIso   :: String -> Maybe String
  , _lensField :: String -> Maybe String
  , _lensClass :: String -> Maybe (String, String)
  , _lensFlags :: Set LensFlag
  }

Perhaps to:

data FieldRules = FieldRules
  { _fieldNaming :: String -> Maybe String
  , _classNaming :: String -> Maybe (String, String)
  , _fieldCommonFlags :: Set CommonFlag
  , _fieldFlags :: Set FieldFlag
  }

data CtorRules = LensRules
  { _ctorNaming :: String -> Maybe String
  , _ctorCommonFlags :: Set CommonFlag
  , _ctorType :: Set CtorFlag
  }

Then, if you want lenses for constructors, and lenses for fields (a rare situation I think, related to the old SingletonAndField flag), you'd have to invoke two functions, or maybe a function which does both with a default configuration.

More thoughts: What if when "PartialFunctions" is enabled, the generated type signatures use LensLike? Might be a cool way to indicate that they're not well behaved.

@ekmett
Owner

Thoughts so far:

We currently have a flag that says to use Lenses instead of Isos for the singleton constructors, so the Ctor stuff would probably need to be fully fleshed out.

We've avoided random contractions in lens thus far, so i'd rather avoid Ctor and just spell out Constructor.

The "partial functions" changes you to LensLike idea is cute, but the type alias is currently invisible unless the user -ddump-splices.

@mgsloan mgsloan was assigned Dec 5, 2012
@ekmett
Owner

I do agree that Control.Lens.TH is a pretty scary pile of interdependent code and should be cleaned up and streamlined.

@aristidb
Collaborator

So there's #143, the original makeProjections (now makePrisms) issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment