Skip to content
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

Support NoFieldSelectors and DuplicateRecordFields #1379

Merged
merged 3 commits into from
Apr 4, 2022

Conversation

fumieval
Copy link
Contributor

@fumieval fumieval commented Apr 4, 2022

  • Getters are defined in terms of explicit record pattern match instead of field selector functions
  • It defines record updates verbosely in order to disambiguate duplicate fields

Before submitting your PR, check that you've:

  • Documented new APIs with Haddock markup
  • Added @since declarations to the Haddock
  • Ran stylish-haskell on any changed files.
  • Adhered to the code style (see the .editorconfig file for details)

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Bumped the version number if there isn't an (unreleased) on the Changelog
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

* Getters are defined in terms of explicit record pattern match instead of a field selector function
* It defines record updates verbosely in order to disambiguate duplicate fields
@@ -78,10 +78,10 @@ import Data.Aeson
, Value(Object)
, eitherDecodeStrict'
, object
, withObject
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is due to stylish-haskell

@parsonsmatt
Copy link
Collaborator

This is a great improvement - I've definitely run into issues before where you'd have a model file like:

User
    profileName Text

UserProfile
    name Text

and it would fail to compile.

Thanks for the PR!

@parsonsmatt
Copy link
Collaborator

Would you be willing to add a test module here? Mostly so we can avoid breaking this with a regression in the future

Probably something like:

#if __GLASGOW_HASKELL__ >= 902
{-# LANGUAGE NoFieldSelectors #-}
{-# LANGUAGE DuplicateRecordFields #-}
#endif    

module NoFieldSelectorTest where

#if __GLASGOW_HASKELL__ >= 902

import ...

mkPersist [persistLowerCase|

something that triggers a bug

|]

spec :: Spec
spec = it "compiles" True

#else

spec :: Spec 
spec = do
    it "only works with GHC 9.2 or greater" $ do
        pendingWith "only works with GHC 9.2 or greater"    

#endif

@fumieval
Copy link
Contributor Author

fumieval commented Apr 4, 2022

Added (using your example almost as-is). Unfortunately it turns out that disambiguating fields isn't enough to solve the User vs UserProfile problem because it also defines type constructors (UserProfile).

@parsonsmatt
Copy link
Collaborator

Alas! Good point on that 😄

I'll get this merged and released once CI is done. This should be a patch-level bump.

@parsonsmatt parsonsmatt merged commit f43b24e into yesodweb:master Apr 4, 2022
@parsonsmatt
Copy link
Collaborator

Released as persistent-2.13.3.4, thank you!

@fumieval
Copy link
Contributor Author

fumieval commented Apr 4, 2022

Thanks! This is a big step for my ambition for greater records!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants