Skip to content

Cleanup pipe ppx #5587

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

Merged
merged 3 commits into from
Jul 23, 2022
Merged

Cleanup pipe ppx #5587

merged 3 commits into from
Jul 23, 2022

Conversation

cristianoc
Copy link
Collaborator

No description provided.

The syntax `a |. M.(f b c) ` only exists in OCaml.
And if one converts it to ReScript, the ppx does not apply to it.
So just remove it and simplify the implementation of `->`.
@mununki
Copy link
Member

mununki commented Jul 23, 2022

Might be not relevant to these changes, but I've noticed that these changes affect this expression, x->Belt.Option.Some. This expression should be handled here?

@mununki
Copy link
Member

mununki commented Jul 23, 2022

Might be not relevant to these changes, but I've noticed that these changes affect this expression, x->Belt.Option.Some. This expression should be handled here?

That expression does not exist in ReScript syntax, I think?

If you mean this example, I think it is still there.

module M = {
  type t = A(string) | B
}
let x = "name"->M.A

@cristianoc
Copy link
Collaborator Author

cristianoc commented Jul 23, 2022

This works just fine:

module X = {
  type t = A(int) | B
}

let q = z => z->X.A

@mununki
Copy link
Member

mununki commented Jul 23, 2022

This works just fine:

module X = {
  type t = A(int) | B
}

let q = z => z->X.A

Doesn't it eat up the attributes by this change?

EDIT:
I am wrong, It is Pexp_construct case. It should be handled.

@cristianoc cristianoc merged commit fa330a1 into master Jul 23, 2022
@cristianoc cristianoc deleted the cleanup-pipe-ppx branch July 23, 2022 17:18
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