Skip to content

Comments

Delete serde_macros#566

Merged
dtolnay merged 2 commits intomasterfrom
del
Sep 28, 2016
Merged

Delete serde_macros#566
dtolnay merged 2 commits intomasterfrom
del

Conversation

@dtolnay
Copy link
Member

@dtolnay dtolnay commented Sep 28, 2016

Fixes #565.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 28, 2016

Unfortunately compiletest can't do multi-line error messages, so I had to do this:

#[derive(Serialize)] //~ ERROR: custom derive attribute panicked
#[serde(abc="xyz")] // ERROR: unknown serde container attribute `abc`
struct A {
    x: u32,
}

We are only checking that the thing fails to compile because "custom derive attribute panicked", not the specific message. I left the messages there so that it is clear what is being tested.

@oli-obk
Copy link
Member

oli-obk commented Sep 28, 2016

Unfortunately compiletest can't do multi-line error messages

What do you mean by multi-line error messages?
Multiple ones per line? Or error messages with a span going over multiple lines?

@dtolnay
Copy link
Member Author

dtolnay commented Sep 28, 2016

Here is what the message looks like:

error: custom derive attribute panicked
 --> src/main.rs:9:10
  |
9 | #[derive(Serialize)]
  |          ^^^^^^^^^
  |
  = help: message: unknown serde container attribute `abc`

Compiletest only sees the error at the top, not the "help". I think related to rust-lang/rust#31380.

@oli-obk
Copy link
Member

oli-obk commented Sep 28, 2016

Compiletest only sees a help when you ask for a help in the code. So adding a //~^ HELP below the error line should work

@dtolnay
Copy link
Member Author

dtolnay commented Sep 28, 2016

Good call, I didn't know about that. I pushed a fix.

Copy link
Member

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Lgtm

@dtolnay dtolnay merged commit 22690ce into master Sep 28, 2016
@dtolnay dtolnay deleted the del branch September 28, 2016 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants