Skip to content

Conversation

danbroooks
Copy link
Contributor

This PR refactors the way that the initial entity definition blocks are parsed by the parse function before they then get turned into an UnboundEntityDef. This is an initial change that hopefully will make it easier to implement #1297 and #1296.

I have added an example test file that illustrates various possible comment styles and what the parse currently is or isn't picking up. This test can be used when tackling the issues linked above. The content of this test is here:

https://github.com/yesodweb/persistent/blob/54be9d68125984249587f81971269f1c7c599a4a/persistent/test/Database/Persist/TH/CommentsSpec.hs

I'm not sure exactly what behavior is expected for comments, so I'd welcome any input on what the behavior should be as I might have made some assumptions that are not desired for the library. But I'd assume everything I've included there persistent would want to be parsed as comments.

The only actual behavioral change regarding comments in this change is illustrated here:

54be9d6#diff-a2bb675c726f8d466d83a85584878a9053dd6304d85aedf0511b8affb8074c56

With this change, that style of comment on the fields is now picked up by the parser, everything else should be the same.

I've also re-worked some of the unit tests, as I've changed some of the types exposed by this module. I've tried to make the tests a bit more flexible as whenever making a change to this module there is often quite a lot to change in the tests. Hopefully ParsedEntityDef and ParsedFieldDef can better replace LinesWithComments which is a lot more low level. I appreciate the coverage you get from the lower level tests, so I have tried to balance things here between flexibility and coverage, I've been through all the tests I had to remove and ensured that we're still covered by a new test based around the types I've added here.


Before submitting your PR, check that you've:

  • Documented new APIs with Haddock markup
  • Added @since declarations to the Haddock
  • Ran fourmolu on any changed files (restyled will do this for you, so
    accept the suggested changes if it makes them)
  • Adhered to the code style (see the .editorconfig and fourmolu.yaml files 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)

@parsonsmatt
Copy link
Collaborator

Oh, shoot, I'm sorry - we are about to land a PR that will port the parser to megaparsec

@danbroooks
Copy link
Contributor Author

Ha 😅 no worries, I think that is for the best definitely.

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.

2 participants