Always show end line of multiline annotations #41136
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
My personal opinion - this looks like an improvement - I find the new error more visually attractive, however, I don't think it adds anything to the explanatory power of the error message and everything else being equal, I prefer the more compact form (disclaimer, I'm only looking at the example in the comment here, perhaps there are other errors where this is more advantageous?) |
|
@nrc I'm of the opinion that errors like the example should look like this: error[E0046]: not all trait items implemented, missing: `Item`
--> $DIR/issue-23729.rs:20:9
|
20 | impl Iterator for Recurrence {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Item` in implementation
|
= note: `Item` from trait: `type Item;`(as proposed in PR #38328, and issues #35965 and #38246) and leave multiline spans only for situations where they are actually useful, like: --> <anon>:2:5
|
2 | fn foo(
| _____^ starting here...
3 | | a: usize,
4 | | b: usize,
5 | | c: usize)
6 | | -> usize {
| |________________^ ...ending here: foo definition span |
|
@estebank perhaps I misunderstood - from the first comment I thought this PR did exactly the opposite of that? |
|
@nrc this PR makes affects every multiline span. A separate aim I have is to reduce how many multiline spans are actually used in diagnostics. I believe that every |
|
An example of a valid multiline span usage IMO: note: the anonymous lifetime #1 defined on the body at 30:83...
--> file.rs:30:84
|
30 | pub fn child_at_index_mut(&mut self, index: &NodeChildIndex) -> &mut NodeChild {
| ____________________________________________________________________________________^ starting here...
31 | | self.children[index.index].borrow_mut()
32 | | }
| |_____^ ...ending here
note: ...does not necessarily outlive the lifetime 'a as defined on the body at 30:83
--> file.rs:30:84
|
30 | pub fn child_at_index_mut(&mut self, index: &NodeChildIndex) -> &mut NodeChild {
| ____________________________________________________________________________________^ starting here...
31 | | self.children[index.index].borrow_mut()
32 | | }
| |_____^ ...ending here |
estebank
force-pushed
the
multiline
branch
3 times, most recently
from
13015f4 to
e1f190f
Compare
|
@estebank - for that example do you have what we show today? |
|
@jonathandturner, on the description of the PR there's an example of after and before. If a span is under 8 lines, we show the entire span, otherwise we only point at the first character. This PR changes this to show in full spans shorter or equal to 7 lines, and longer spans show the first 4 lines and last two. (I thought about showing the last line only, but that would in most cases be just |
```rust
error[E0046]: not all trait items implemented, missing: `Item`
--> $DIR/issue-23729.rs:20:9
|
20 | impl Iterator for Recurrence {
| _________^ starting here...
21 | | //~^ ERROR E0046
22 | | //~| NOTE missing `Item` in implementation
23 | | //~| NOTE `Item` from trait: `type Item;`
... |
36 | | }
37 | | }
| |_________^ ...ending here: missing `Item` in implementation
|
= note: `Item` from trait: `type Item;`
```
instead of
```rust
error[E0046]: not all trait items implemented, missing: `Item`
--> $DIR/issue-23729.rs:20:9
|
20 | impl Iterator for Recurrence {
| ^ missing `Item` in implementation
|
= note: `Item` from trait: `type Item;`
```
|
📌 Commit 4bc7f5b has been approved by |
Always show end line of multiline annotations
```rust
error[E0046]: not all trait items implemented, missing: `Item`
--> $DIR/issue-23729.rs:20:9
|
20 | impl Iterator for Recurrence {
| _________^ starting here...
21 | | //~^ ERROR E0046
22 | | //~| NOTE missing `Item` in implementation
23 | | //~| NOTE `Item` from trait: `type Item;`
... |
36 | | }
37 | | }
| |_________^ ...ending here: missing `Item` in implementation
|
= note: `Item` from trait: `type Item;`
```
instead of
```rust
error[E0046]: not all trait items implemented, missing: `Item`
--> $DIR/issue-23729.rs:20:9
|
20 | impl Iterator for Recurrence {
| ^ missing `Item` in implementation
|
= note: `Item` from trait: `type Item;`
```
|
☀️ Test successful - status-appveyor, status-travis |
| // | ||
| // After this we will have: | ||
| // | ||
| // 2 | fn foo() { | ||
| // | __________ | ||
| // | | | | ||
| // | | | ||
| // 3 | | |
instead of