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

Add mergeFlipped #84

Open
sigma-andex opened this issue Jun 9, 2022 · 5 comments
Open

Add mergeFlipped #84

sigma-andex opened this issue Jun 9, 2022 · 5 comments

Comments

@sigma-andex
Copy link

Add mergeFlipped and an infix operator //

As a user I'm typically more thinking in terms of mergeFlipped than merge, as I want to overwrite a record with some new fields. E.g. in Typescript I can do

const newRecord = { ...otherRecord, more: 23 }

and in dhall // is avaiable as a mergeFlipped operator:

{ home       = "/home/bill"
, privateKey = "/home/bill/.ssh/id_ed25519"
, publicKey  = "/home/blil/.ssh/id_ed25519.pub"
} // { home = "/home/john" }

So in Purescript I would also like to do the following and I think it makes for a nice addition to purescript-record:

person :: { age :: Int
, firstName :: Maybe String
, professional :: String
}
person = 
  { firstName: "John", professional: true } //
  { professional: "Software Engineer", age: 58 } // 
  { firstName: Just "Mike" }

I have created a PR to illustrate this #83

@sigma-andex sigma-andex mentioned this issue Jun 9, 2022
4 tasks
@JordanMartinez
Copy link
Contributor

I think I'm fine with mergeFlipped (though flip merge is shorter...) because we have other similar functions throughout other core libs (e.g. composeKleisliFlipped). I'm more hesitant about the // infix operator. I get that Dhall uses it for record merging, but I'm not sure whether we should follow in that regard.
As a separate thought, a weak argument against adding it is that // indicates a comment in JavaScript.

@sigma-andex
Copy link
Author

I think there is a need for an operator to make working with records easier and it has come up in the discord before. E.g. @i-am-the-slime and @mikesol also proposed operators for frequently used record operations (though they were taking about disjointUnion and disjoint).
Personally I think // is my favourite for mergeFlipped and aligns nicely with dhall which we already use in spago. I think that // is a comment in JS is not really something I consider relevant.
An alternative could be ... as in Typescript:

{ x: 1, y: 2 } ... { y: "y" } ... { z: true })

but tbh I think if I wouldn't know it I think // as in dhall is more obvious what its intention is.

@natefaubion
Copy link
Contributor

I think I'm fine with mergeFlipped (though flip merge is shorter...)

I don't think there's any point to having mergeFlipped if you aren't going to have the operator, for the same reason that nobody ever references composeKleisliFlipped without the operator.

The non operator left-to-right alternative is:

  { firstName: "John", professional: true }
    # merge { professional: "Software Engineer", age: 58 }
    # merge { firstName: Just "Mike" }

I'm personally ambivalent towards this. I don't think the operator is more clear than # merge by any means, and I think it's a little weird to say that mergeFlipped deserves an operator but merge doesn't, but I understand it satisfies the behavior for how people usually think of record merging working.

@JordanMartinez
Copy link
Contributor

I don't think there's any point to having mergeFlipped if you aren't going to have the operator, for the same reason that nobody ever references composeKleisliFlipped without the operator.

Good point.

The non operator left-to-right alternative is:

  { firstName: "John", professional: true }
   # merge { professional: "Software Engineer", age: 58 }
   # merge { firstName: Just "Mike" }

Just wanted to say that I really like this style of syntax.

@sigma-andex
Copy link
Author

sigma-andex commented Jun 23, 2022

I'm personally ambivalent towards this. I don't think the operator is more clear than # merge by any means, and I think it's a little weird to say that mergeFlipped deserves an operator but merge doesn't, but I understand it satisfies the behavior for how people usually think of record merging working.

I would argue that // is a bit more beginner-friendly, but of course this is maybe a subjective opinion/impression.

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

3 participants