Add position info to EntityDef #1569
Conversation
|
idk if i parsed the PVP correctly here, this might actually be non breaking so perhaps should be the C instead of B component, but it will blow up anyone using the internals of the quasiquoter/TH stuff. |
This is an initial implementation that we can API-compatibly improve in the future by providing more accurate information. However, to do that, we would have to rewrite the entity definition parser with e.g. megaparsec. This is a reasonable course of action but rewriting it is going to be more work and we can ship position info we could improve later with the existing parser.
lf-
force-pushed
the
jade/entitydef-location
branch
from
e917484 to
0aa83dd
Compare
| * [#1569](https://github.com/yesodweb/persistent/pull/1569) | ||
| * Version bounds for Persistent 2.15.0.0, also synchronize version | ||
| number with `persistent`. |
There was a problem hiding this comment.
We don't need to worry about this sync. If it's just version bounds then it can be a patch bump or even an (unreleased) and I'll make a revision on Hackage
| @@ -279,7 +281,7 @@ embedEntityDefsMap existingEnts rawEnts = | |||
| -- instead of @['EntityDef']@. | |||
| -- | |||
| -- @since 2.5.3 | |||
| parseReferences :: PersistSettings -> Text -> Q Exp | |||
| parseReferences :: PersistSettings -> [(Maybe SourceLoc, Text)] -> Q Exp | |||
There was a problem hiding this comment.
Ah, dang, this is listed as -- * Internal but is not itself part of an .Internal module. So this is a breaking change and we need 2.15 for this.
| -- | A pair of (start line/col, end line/col) coordinates. The end column will | ||
| -- be one past the final character (i.e. the span (1,1)->(1,1) is zero | ||
| -- characters long). | ||
| -- | ||
| -- Spans are 1-indexed in both lines and columns. | ||
| -- | ||
| -- Conceptually identical to GHC's @RealSourceSpan@. | ||
| -- | ||
| -- @since 2.15.0.0 | ||
| data Span = Span | ||
| { spanFile :: !Text | ||
| , spanStartLine :: !Int | ||
| , spanStartCol :: !Int | ||
| , spanEndLine :: !Int | ||
| , spanEndCol :: !Int | ||
| } | ||
| deriving (Show, Eq, Read, Ord, Lift) |
There was a problem hiding this comment.
I'd prefer to avoid growing this module any further - any way we can put this somewhere else?
| , entitySpan :: !(Maybe Span) | ||
| -- ^ Source code span occupied by this entity. May be absent if it is not | ||
| -- known. |
There was a problem hiding this comment.
I guess we wouldn't have this if we skip the quasiquoter entirely and write the instances ourselves. Valid
| @@ -157,7 +157,7 @@ test-suite test | |||
| hs-source-dirs: | |||
| test/ | |||
|
|
|||
| ghc-options: -Wall | |||
| ghc-options: -Wall -Wno-incomplete-uni-patterns -dth-dec-file | |||
There was a problem hiding this comment.
What's -dth-dec-file do?
There was a problem hiding this comment.
that's a mistake! shouldn't be in there. however you can consult the blog post from when i had the terrible misfortune to be debugging persistent's quasiquoter in my first or second month learning haskell; it might actually be useful tips. https://jade.fyi/blog/debugging-template-haskell/
There was a problem hiding this comment.
Ohhh that's super cool! Thanks for the reference 😄
(full circle: i wrote the bug you diagnosed 😂)
| personDefBeforeLoc :: SourceLoc | ||
| personDefBeforeLoc = $(TH.lift @_ @SourceLoc . sourceLocFromTHLoc =<< TH.location) |
There was a problem hiding this comment.
I think there may be a better way of doing this -
do
firstLoc <- [d| firstLocation = $(TH.location) |]
decls <- share [mkPersistWith sqlSettings ...] [persistUpperCase| ... |]
secondLoc <- [d| secondLocation = $(TH.location) |]
pure (concat [firstLoc, decls, secondLoc])This would require a little less spooky magic
| @@ -76,7 +76,7 @@ library | |||
| , monad-logger >= 0.3.25 | |||
| , mtl | |||
| , path-pieces >= 0.2 | |||
| , persistent >= 2.14 && < 2.15 | |||
| , persistent >= 2.15 && < 2.16 | |||
There was a problem hiding this comment.
Since there are no source changes, we can probably just do
| , persistent >= 2.15 && < 2.16 | |
| , persistent >= 2.14 && < 2.16 |
|
Build failure is due to |
Internal Mercury bug: DUX-3027
Fixes: #1567
This is an initial implementation that we can API-compatibly improve in
the future by providing more accurate information. However, to do that,
we would have to rewrite the entity definition parser with e.g.
megaparsec. This is a reasonable course of action but rewriting it is
going to be more work and we can ship position info we could improve
later with the existing parser.
Currently this gives users the entire file in which the model is defined, but not anything closer since the parser throws all that stuff out.
Before submitting your PR, check that you've:
@sincedeclarations to the Haddockstylish-haskellon any changed files..editorconfigfile for details)After submitting your PR:
(unreleased)on the Changelog