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.

Sign up for GitHub

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

Jump to bottom

Unnecessarily wraps an anonymous fun function in parens #2032

Closed
bordoley opened this issue Jun 29, 2018 · 1 comment · Fixed by #2033
Closed

Unnecessarily wraps an anonymous fun function in parens #2032

bordoley opened this issue Jun 29, 2018 · 1 comment · Fixed by #2033
Labels
KIND: FEATURE REQUEST Printer things that have to do with turning an AST into Reason code SKILL: VERY DIFFICULT

Comments

@bordoley
Copy link 

    let predicate =
      predicate === Functions.alwaysTrue1 ?
        defaultPredicate :
        fun
          | None => false
          | Some(exn) => predicate(exn);

gets refmt'd to:

 let predicate =
      predicate === Functions.alwaysTrue1 ?
        defaultPredicate :
        (
          fun
          | None => false
          | Some(exn) => predicate(exn)
        );

Notice the additional unnecessary parens.
@anmonteiro
Copy link
Member

Thanks for reporting. I'm fixing this one.

anmonteiro added a commit to anmonteiro/reason that referenced this issue Jun 30, 2018
@anmonteiro
Unnecessarily wraps an anonymous fun function in parens
1894f09 
fixes reasonml#2032

before:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    (
      fun
      | None => false
      | Some(exn) => predicate(exn)
    );
```

after:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    fun
    | None => false
    | Some(exn) => predicate(exn);
```
@anmonteiro anmonteiro mentioned this issue Jun 30, 2018
Merged
@IwanKaramazow IwanKaramazow added KIND: FEATURE REQUEST SKILL: VERY DIFFICULT Printer things that have to do with turning an AST into Reason code labels Jul 1, 2018
anmonteiro added a commit to anmonteiro/reason that referenced this issue Jul 2, 2018
@anmonteiro
Unnecessarily wraps an anonymous fun function in parens
f1a3acf 
fixes reasonml#2032

before:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    (
      fun
      | None => false
      | Some(exn) => predicate(exn)
    );
```

after:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    fun
    | None => false
    | Some(exn) => predicate(exn);
```
anmonteiro added a commit to anmonteiro/reason that referenced this issue Jul 6, 2018
@anmonteiro
Unnecessarily wraps an anonymous fun function in parens
d256f9b 
fixes reasonml#2032

before:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    (
      fun
      | None => false
      | Some(exn) => predicate(exn)
    );
```

after:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    fun
    | None => false
    | Some(exn) => predicate(exn);
```
anmonteiro added a commit to anmonteiro/reason that referenced this issue Jul 29, 2018
@anmonteiro
Unnecessarily wraps an anonymous fun function in parens
5877b65 
fixes reasonml#2032

before:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    (
      fun
      | None => false
      | Some(exn) => predicate(exn)
    );
```

after:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    fun
    | None => false
    | Some(exn) => predicate(exn);
```
chenglou pushed a commit that referenced this issue Aug 9, 2018
@anmonteiro @chenglou
Only wrap Pexp_function in parens when necessary (#2033)
37108c7 
* Unnecessarily wraps an anonymous fun function in parens

fixes #2032

before:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    (
      fun
      | None => false
      | Some(exn) => predicate(exn)
    );
```

after:

```reason
let predicate =
  predicate === Functions.alwaysTrue1 ?
    defaultPredicate :
    fun
    | None => false
    | Some(exn) => predicate(exn);
```

* add more test cases and fix precedence in the presence of infix operators

* even more test cases, demonstrating ternary + infix bind
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
KIND: FEATURE REQUEST Printer things that have to do with turning an AST into Reason code SKILL: VERY DIFFICULT
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Only wrap Pexp_function in parens when necessary anmonteiro/reason
3 participants
@anmonteiro @bordoley @IwanKaramazow