Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
jean-lourenco:master
Oct 13, 2017
Merged

Better compile error output when using arguments instead of types #45122
bors merged 1 commit into
rust-lang:masterfrom
jean-lourenco:master

Conversation

@jean-lourenco

Copy link
Copy Markdown
Contributor

Following @estebank sugestion on issue #18945 (comment)

@rust-highfive

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
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
Copy Markdown
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

jean-lourenco commented Oct 10, 2017

Copy link
Copy Markdown
Contributor Author

The recommended changes were implemented.

EDIT: Nevermind, I broke some tests 😅

@estebank

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Oct 11, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit db91b00 has been approved by nikomatsakis

@nikomatsakis

Copy link
Copy Markdown
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