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

Syntax for updating multiple fields? #49

Open
edsko opened this issue Mar 14, 2022 · 5 comments
Open

Syntax for updating multiple fields? #49

edsko opened this issue Mar 14, 2022 · 5 comments

Comments

@edsko
Copy link
Contributor

edsko commented Mar 14, 2022

This is not a bug report, but a point for discussion.

Version 0.2.2 changed the behaviour of record-dot-preprocessor, so that r{ x = .. } is no longer recognized as a record update. I'm just wondering if this should be reconsidered. The problem is that if we allow no space before or after the curly brace, it becomes quite difficult to find a reasonable layout for multiple record updates.

For example, consider this code, which is from part 3 of the Beam tutorial but adapted to use record-dot-preprocessor:

shoppingCart3Db :: DatabaseSettings be ShoppingCart3Db
shoppingCart3Db = defaultDbSettings `withDbModification`
    dbModification{shoppingCart3UserAddresses =
                         setEntityName "addresses"
                      <> modifyTableFields tableModification{addressLine1 = "address1"
                                                            ,addressLine2 = "address2"
                                                            }
                  , shoppingCart3Products =
                         setEntityName "products"
                  , shoppingCart3Orders =
                         setEntityName "orders"
                      <> modifyTableFields tableModification{orderShippingInfo = ShippingInfoId "shipping_info__id"}
                  , shoppingCart3ShippingInfos =
                         setEntityName "shipping_info"
                      <> modifyTableFields tableModification{shippingInfoId             = "id"
                                                            ,shippingInfoCarrier        = "carrier"
                                                            ,shippingInfoTrackingNumber = "tracking_number"
                                                            }
                  , shoppingCart3LineItems =
                         setEntityName "line_items"
                  }

I would much prefer to write this as

shoppingCart3Db :: DatabaseSettings be ShoppingCart3Db
shoppingCart3Db = defaultDbSettings `withDbModification` dbModification{
      shoppingCart3UserAddresses =
           setEntityName "addresses"
        <> modifyTableFields tableModification{
               addressLine1 = "address1",
               addressLine2 = "address2"
             }
    , shoppingCart3Products =
           setEntityName "products"
    , shoppingCart3Orders =
           setEntityName "orders"
        <> modifyTableFields tableModification{
               orderShippingInfo = ShippingInfoId "shipping_info__id"
             }
    , shoppingCart3ShippingInfos =
           setEntityName "shipping_info"
        <> modifyTableFields tableModification{
               shippingInfoId = "id"
             , shippingInfoCarrier = "carrier"
             , shippingInfoTrackingNumber = "tracking_number"
             }
    , shoppingCart3LineItems =
           setEntityName "line_items"
    }

but I cannot, because then RDP would not recognize the update anymore.

Either requiring no space before (which is what it used to be, I think, and what the above second snippet assumes) or requiring no space after (in which case we can move the curly brace to the next line in that example) would be work, but requiring no space before and after means that there is basically no good way to layout code that does multiple record updates, on multiple lines.

@ndmitchell
Copy link
Owner

Looking at #26, and the documentation that went with it, I think the intention was that a {b=c} isn't an update, but a{ b = c } is. These rules are present in the second bullet point at https://github.com/ndmitchell/record-dot-preprocessor/blob/master/README.md#what-magic-is-available-precisely. So I think it sounds like what you want to write should work? In which case, this sounds like it is a bug report, as it doesn't match the spec?

@edsko
Copy link
Contributor Author

edsko commented Apr 7, 2022

I took a look at fixing this, but I think it's actually not fixable, at least not for older ghc. The relevant fragment I think is this one:

-- Turn a{b=c, ...} into setField calls
onExp (L o upd@RecordUpd{rupd_expr,rupd_flds=fld:flds})
    | adjacentBy 1 rupd_expr fld
    = ..

Notice how it's not actually looking at the position of the curly brace at all, but rather checking whether the record being updated and the first field are separated by exactly one space (presumably, the curly). I think there's no way around this: the ghc AST does not give us the position of the curly (I could be wrong). We can probably do better in (very) recent ghc that have ghc-exactprint merged, but otherwise i don't see a good way out here :/

@edsko
Copy link
Contributor Author

edsko commented Apr 7, 2022

My ghc expert colleagues just told me I'm wrong about this, and the information is available. I'll take another look.

@edsko
Copy link
Contributor Author

edsko commented Apr 7, 2022

Ok, the information should be available in https://downloads.haskell.org/ghc/8.8.4/docs/html/libraries/ghc-8.8.4/GHC.html#t:ParsedModule apparently, but that's a bigger change to the plugin that I don't currently have time for.

@ndmitchell
Copy link
Owner

I have a great deal of difficulty developing this plugin, since its on Windows, and segfaults most of the time when I try and do stuff with it. I made the last few updates against the CI with a bit of hope, so I probably won't do this either.

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

No branches or pull requests

2 participants