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

Better compile error output when using arguments instead of types #45122

Merged
merged 1 commit into from
Oct 13, 2017
Merged

Better compile error output when using arguments instead of types #45122

merged 1 commit into from
Oct 13, 2017

Conversation

jean-lourenco
Copy link
Contributor

Following @estebank sugestion on issue #18945 (comment)

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@@ -4,31 +4,31 @@ error: chained comparison operators require parentheses
12 | println!("{:?}", (0..13).collect<Vec<i32>>());
| ^^^^^^^^
|
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments
= help: use `::<...>` instead of `<...>` if you meant to specify type arguments or use `(...)` if you meant to specify fn arguments
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will probably look better as a second help: on a separate line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I also would prefer a second help

@carols10cents carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 9, 2017
@jean-lourenco
Copy link
Contributor Author

jean-lourenco commented Oct 10, 2017

The recommended changes were implemented.

EDIT: Nevermind, I broke some tests 😅

@estebank
Copy link
Contributor

@jean-lourenco the broken test is to be expected and correct, if you could fix it and squash all your commits into one we can approve.

@nikomatsakis
Copy link
Contributor

Looks like you have to update parse-fail/require-parens-for-chained-comparison.rs to something like this:

    f<X>();
    //~^ ERROR: chained comparison operators require parentheses
    //~| HELP: use `::<...>` instead of `<...>`
    //~| HELP: or use `(...)`

output message is shown in another 'help:' block

line with +100 columns formatted

test adjusted
@jean-lourenco
Copy link
Contributor Author

Now the tests are green 🎉
Thank you guys for your help!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 11, 2017

📌 Commit db91b00 has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

Thanks @jean-lourenco

kennytm added a commit to kennytm/rust that referenced this pull request Oct 11, 2017
Better compile error output when using arguments instead of types

Following @estebank sugestion on issue rust-lang#18945 (comment)
kennytm added a commit to kennytm/rust that referenced this pull request Oct 12, 2017
Better compile error output when using arguments instead of types

Following @estebank sugestion on issue rust-lang#18945 (comment)
kennytm added a commit to kennytm/rust that referenced this pull request Oct 13, 2017
Better compile error output when using arguments instead of types

Following @estebank sugestion on issue rust-lang#18945 (comment)
bors added a commit that referenced this pull request Oct 13, 2017
Rollup of 14 pull requests

- Successful merges: #44855, #45110, #45122, #45133, #45173, #45178, #45189, #45203, #45209, #45221, #45236, #45240, #45245, #45253
- Failed merges:
@bors bors merged commit db91b00 into rust-lang:master Oct 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants