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

Add help notes (and improve some error explanations too) #28231

Merged
merged 2 commits into from
Sep 5, 2015

Conversation

GuillaumeGomez
Copy link
Member

msg.description());
span_help!(self.tcx.sess, msg.span,
"Dividing by 0 always leads to an error and is \
certainly not what you want");
Copy link
Member

Choose a reason for hiding this comment

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

"certainly not what you want" doesn't sound like a nice way to phrase it. In this case, there's no single suggestion for fixing that stands out, so we shouldn't be adding this help in the first place (that's what the long diagnostics are for)

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case, I found it useful because there is no case you might want to divide by 0. So instead of removing it, do you have a better sentence maybe ?

Copy link
Member

Choose a reason for hiding this comment

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

But that's the point. In this case from the error itself that one should somehow fix it. This line doesn't really suggest anything and restates the obvious.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I remove this one then.

@Manishearth
Copy link
Member

r+ after you remove the first span_help

@GuillaumeGomez
Copy link
Member Author

@Manishearth: Done !

@Manishearth
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 5, 2015

📌 Commit 771ab35 has been approved by Manishearth

Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 5, 2015
bors added a commit that referenced this pull request Sep 5, 2015
@bors bors merged commit 771ab35 into rust-lang:master Sep 5, 2015
@GuillaumeGomez GuillaumeGomez deleted the help_note branch September 5, 2015 19:14
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.

3 participants