Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Print Ppat_constraint #585

Closed
mununki opened this issue Jun 27, 2022 · 9 comments
Closed

Print Ppat_constraint #585

mununki opened this issue Jun 27, 2022 · 9 comments

Comments

@mununki
Copy link
Member

mununki commented Jun 27, 2022

I'd like to print this AST, the intended output is below.

(* AST *)
Pat.record lid
Pat.constraint_
  (Pat.var lid)
  (Typ.constr ~attrs:optionalAttr
      {
        txt = lid;
        loc = Location.none;
      }
      []) )

(* original code in OCaml as intended*)
name: name [@optional]

But the printer generates the output

name: (name: @optional name)

I think this branch needs to be fixed with additional pattern matching for Ppat_constraint.

| longident, pattern ->
I'm not sure whether it is correct or error.

@cristianoc
Copy link
Contributor

Can you give the repro in source syntax instead of AST?

@mununki
Copy link
Member Author

mununki commented Jun 27, 2022

@cristianoc
Copy link
Contributor

Have you tried after this? #584
I assume the issue is still there, but this change touches exactly that area of code.

I'd need a little test snippet in ReScript syntax that gets reprinted to what you've shown.

cristianoc added a commit that referenced this issue Jun 27, 2022
@cristianoc
Copy link
Contributor

cristianoc commented Jun 27, 2022

(* original code in OCaml as intended*)
name: name [@optional]

To generate that , you don't want a constraint, you want a Ppat_var with the same name as the label.
Constraint is instead used for type constraints.

cristianoc added a commit that referenced this issue Jun 27, 2022
cristianoc added a commit that referenced this issue Jun 27, 2022
cristianoc added a commit that referenced this issue Jun 27, 2022
@mununki
Copy link
Member Author

mununki commented Jun 27, 2022

(* original code in OCaml as intended*)
name: name [@optional]

To generate that , you don't want a constraint, you want a Ppat_var with the same name as the label. Constraint is instead used for type constraints.

Ouch, you are correct. Thanks!

@mununki mununki closed this as completed Jun 27, 2022
@cristianoc
Copy link
Contributor

After this #588, assuming all goes well, the change required will be simply to use @ns.optional instead of @optional which will be removed.

@mununki
Copy link
Member Author

mununki commented Jun 27, 2022

Got it, thanks!

@mununki
Copy link
Member Author

mununki commented Jun 27, 2022

Is @ns.optional working same in compiler if I sync the master of submodule syntax? I'm making the binding for React under Js.* There is type domProps = { ... } which is having many fields to have the optional attribute.

@cristianoc
Copy link
Contributor

Is @ns.optional working same in compiler if I sync the master of submodule syntax? I'm making the binding for React under Js.* There is type domProps = { ... } which is having many fields to have the optional attribute.

Not ready yet, but nearly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants