groundhog icon indicating copy to clipboard operation
groundhog copied to clipboard

Record fields with underscores give TH machinery trouble

Open ozataman opened this issue 10 years ago • 6 comments

The below yields a large error complaining about:

    Illegal data constructor"
<- " name: `_pgHostField'
    When splicing a TH declaration:
      instance Database.Groundhog.Core.PersistEntity Compute.Backends.PGCommon.PGConfig
    where data Database.Groundhog.Core.Key Compute.Backends.PGCommon.PGConfig

Example code:

data PGConfig = PGConfig
    { _pgHost     :: Text
    , _pgPort     :: Int
    , _pgUser     :: Text
    , _pgPass     :: Text
    , _pgDatabase :: Text
    , _pgSchema   :: Text
    , _pgSSL      :: SSLMode
    } deriving (Eq,Show,Generic)
- entity: PGConfig
  dbName: xxxxx
  constructors:
    - name: PGConfig

ozataman avatar May 15 '14 22:05 ozataman

The naming schema tried to create a field (GADT constructor) _pgHostField which is an invalid constructor name. I would suggest modifying the naming schema. There are many ways to make the name valid (strip _, add prefix) but putting these heuristics into the default schemas I think would make the library more "magical"

lykahb avatar May 15 '14 23:05 lykahb

Understood. However, we may want to expose some functionality that provides this prefix _ stripping, especially as lens becomes more and more prevalent and field names starting with _ are the norm there.

ozataman avatar May 15 '14 23:05 ozataman

Good point. My comment was about names normalization in general. If this is a common case, a NamingStyle adjustment function would be handy. Do you think that stripping underscores is the best option here?

lykahb avatar May 15 '14 23:05 lykahb

I guess so. It is not at all idiomatic to use underscores in general, so perhaps we can assume they are safe to strip. If someone has a special use case, they can always implement their own naming schema, right?

ozataman avatar May 16 '14 00:05 ozataman

That's right. I am not sure though if this case is popular enough to be included into the TH module. Otherwise you could use this schema modification just for your own project.

lykahb avatar May 16 '14 00:05 lykahb

Workaround:

mkPersist
  (defaultCodegenConfig
   {  namingStyle = lowerCaseSuffixNamingStyle {
                      mkExprSelectorName =
                        \a b c d -> "Z" ++ (mkExprSelectorName lowerCaseSuffixNamingStyle) a b c d
                      }
   })
   [groundhog|
   ...

yaitskov avatar Oct 01 '20 03:10 yaitskov