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

Parentheses when assigning optional value to optional field #714

Closed
cknitt opened this issue Nov 1, 2022 · 2 comments · Fixed by #718
Closed

Parentheses when assigning optional value to optional field #714

cknitt opened this issue Nov 1, 2022 · 2 comments · Fixed by #718

Comments

@cknitt
Copy link
Member

cknitt commented Nov 1, 2022

Currently, the input

{
  paddingTop: ?(someBool ? Some(42.) : None),
}

prints as

{
  paddingTop: ?someBool ? Some(42.) : None,
}

which still parses fine, but looks quite weird, as if the first ? was applied to someBool only.

I think this should be printed with parentheses as in the original input.

@cristianoc
Copy link
Contributor

@cknitt see #718
Can you think of other examples where the printing should be different w.r.t. no ?
Now that the printing path is special-cased we can see if there's more to add.

One can argue whether if-then-else should be in parens also without ?.

@cknitt
Copy link
Member Author

cknitt commented Nov 3, 2022

What about functions?

{
  x: ?() => Js.log("test"),
}

This is a problem that we wouldn't have if we had used x?: ... instead of x: ?....
But I remember we discussed this before and there were good reasons for this choice.

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

Successfully merging a pull request may close this issue.

2 participants