-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
dump!: debugging macro #12426
dump!: debugging macro #12426
Conversation
dump! is a shorter way of quickly outputing some values to debug programs. Example: dump!(val_a, 2 + 2); => "val_a = true, 2 + 2 = 4" fixes rust-lang#12015
LGTM, but I'm going to hold off r+ until someone with more authority decides if this is desired. One note: it is, in fact, possible to test the output. You can redirect task-local stdout to a buffer, perform your |
I just copied what |
Those should probably be fixed as well, either as part of this PR or as a new one. |
#12019 should be considered when merging this. |
@kballard i added formatting tests for |
t!(r.read_line().unwrap(), "hi~[0u8]hello\n"); | ||
t!(r.read_line().unwrap(), "this is a test\n"); | ||
t!(r.read_line().unwrap(), "bar\n"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should verify at this point that the channel is closed. You can use .recv_opt
and ensure it returns None
. This is effectively testing to make sure there isn't any more output after what was expected.
@alexcrichton I don't understand enough of that issue to know what to do about it. |
@kballard I amended that commit with tests for |
r=me if someone else is willing to make the call that we definitely want this. I don't think anything concrete needs to be done about #12019 at the moment with regards to this. If reflection gets a lint, hopefully that lint will see through the macro and flag the calls to |
Though I do understand the utility, I'm firmly on the fence about this. That everybody is doing debug logging with |
The problem with |
It's cool that it can be implemented in a library. Maybe we could have a (I can't think of anything else, so the crate might be rather empty.) |
@huonw |
That's not a bad idea. Having reflection in a crate named On Thu, Feb 20, 2014 at 9:01 PM, Kevin Ballard notifications@gh.neting.ccwrote:
|
So, should I make a new PR with this + moving breakpoint/reflect into libdebug? |
Closing due to a lack of activity, but a |
I was thinking today we might make "info" the default level to encourage its use for things like this. |
interesting, i was about to ask for something like this; I was also after the equivalent of |
… r=Veykril feature: `Merge imports` assist can merge multiple selected imports. The selected imports have to have a common prefix in paths. Select imports or use trees to merge: ```rust $0use std::fmt::Display; use std::fmt::Debug; use std::fmt::Write;$0 ``` Apply `Merge imports`: ```rust use std::fmt::{Display, Debug, Write}; ``` Closes rust-lang#12426
…-ctxt, r=Manishearth Don't lint `redundant_field_names` across macro boundaries Fixes rust-lang#12426 The `field.span.eq_ctxt(field.ident.span)` addition is the relevant line for the bugfix The current implementation checks that the field's name and the path are in the same context by comparing the idents, but not that the two are in the same context as the entire field itself, so in local macros `SomeStruct { $ident: $ident }` would get linted changelog: none
dump! is a shorter way of quickly outputing some values to debug programs.
Example:
fixes #12015