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

[Super errors] Better missing param msg #2554

Merged
merged 2 commits into from Feb 28, 2018
Merged

Conversation

chenglou
Copy link
Member

@chenglou chenglou commented Feb 28, 2018

Continuation of #2548

Mini usability testing result out. Apparently the previous message was too confusing, so I'm adding back the fallback type clash msg.

Additionally, I'm now using Ctype.matches, which considers option('a) and option(string) as equal. This, along with the original code in #2548, fixes reasonml-community/error-message-improvement#14

But since this produces false positives "missing parameters" msg for such code below:

let a: int => float => option(int) = (a) => Some("");

Where option(int) matches option('a) inferred from Some(""), I've also tweaked the msg to indicate that we're guessing that it might be because of missing parameters (but that we're not sure). The fallback type clash msg gives us leeway to guess.

screenshot 2018-02-28 06 11 52

cc @cristianoc

Continuation of rescript-lang#2548

Mini usability testing result out. Apparently the previous message was too confusing, so I'm adding back the fallback type clash msg.

Additionally, I'm now using `Ctype.matches`, which considers `option('a)` and `option(string)` as equal. This, along with the original code in rescript-lang#2548, fixes reasonml-community/error-message-improvement#14

But since this produces false positives "missing parameters" msg for such code below:

```reason
let a: int => float => option(int) = (a) => Some("");
```

Where `option(int)` matches `option('a)` inferred from `Some("")`, I've also tweaked the msg to indicate that we're _guessing_ that it _might_ be because of missing parameters (but that we're not sure). The fallback type clash msg gives us leeway to guess.

cc @cristianoc
@chenglou
Copy link
Member Author

chenglou commented Feb 28, 2018

Much better for ReasonReact. Before:
screenshot 2018-02-28 06 09 56

After:
screenshot 2018-02-28 06 09 27

Quality of life dramatically improved, lol

@chenglou chenglou merged commit 08426fa into rescript-lang:master Feb 28, 2018
@chenglou chenglou deleted the react branch February 28, 2018 23:58
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.

error messages for partial JSX-style function applications are improvable
1 participant