Skip to content

Make fail message more presentable #15239

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

Closed
SiegeLord opened this issue Jun 28, 2014 · 6 comments · Fixed by #30402
Closed

Make fail message more presentable #15239

SiegeLord opened this issue Jun 28, 2014 · 6 comments · Fixed by #30402

Comments

@SiegeLord
Copy link
Contributor

Given this code,

fn main()
{
    fail!("This is why you fail.")
}

the failure message looks like this

task '<main>' failed at 'This is why you fail.', test.rs:3

This is problematic as the useful information (the message passed to fail!)is buried inside the sentence. It'd be better if the failure message was on its own line, something akin to:

task '<main>' failed at test.rs:3, saying:

This is why you fail.

This format also allows for space to mention other useful information (e.g. issue #11704).

@jooert
Copy link
Contributor

jooert commented Dec 15, 2015

Is this still something we want to do? I have a small patch that implements something like this and would also fix #11704, but I'm not sure how many people really want another format for panic messages. Changing the format might also break existing tools (for example, there are several run-tests in this repo that expect a panic).

@SiegeLord
Copy link
Contributor Author

I'd be all for it, I still find those messages hard to read even after all these years.

@steveklabnik
Copy link
Member

@jooert you should submit it

bors added a commit that referenced this issue Jan 26, 2016
This splits the output of panics into two lines as proposed in #15239 and adds a
note about how to get a backtrace. Because the default panic message consists of
multiple lines now, this changes the test runner's failure output to not indent
the first line anymore.

Fixes #15239 and fixes #11704.
@SiegeLord
Copy link
Contributor Author

The pull request didn't really fix this bug... should I make another bug like this, or is this a wontfix now?

@jooert
Copy link
Contributor

jooert commented Jan 27, 2016

Oh sorry, that's because I didn't change the original pull request message. Perhaps @alexcrichton can comment on this?

@alexcrichton
Copy link
Member

This is somewhat of a wontfix for now, the discussion of #30402 concluded that we likely don't want to radically change the format for now due to the concurrency/verbosity reasons mentioned. If this becomes extra onerous in the future, however, we can consider reformatting these messages.

bors added a commit to rust-lang-ci/rust that referenced this issue Aug 21, 2023
…eykril

internal : rewrite DeMorgan assist

fixes rust-lang#15239 , rust-lang#15240 . This PR is a rewrite of the DeMorgan assist that essentially rids of all the string manipulation and modifies syntax trees to apply demorgan on a binary expr. The main reason for the rewrite is that I wanted to use `Expr::needs_parens_in` method to see if the expr on which the assist is applied would still need the parens it had once the parent expression's operator had equal precedence with that of the expression. I used `.clone_(subtree|for_update)` left and right and probably more than I should have, so I would also be happy to hear how I could have prevented redundant cloning.
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 a pull request may close this issue.

4 participants