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

Position span label correctly when it isn't last #42616

Merged
merged 1 commit into from
Jun 16, 2017

Conversation

estebank
Copy link
Contributor

Fix #42595.

Before:

15 |     map.entry("e").or_insert(0) += 1;
   |     ---------------------------^^^^^ot use `+=` on type `&mut {integer}`

After:

15 |     map.entry("e").or_insert(0) += 1;
   |     ---------------------------^^^^^
   |     |
   |     cannot use `+=` on type `&mut {integer}`

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

15 | map.entry("e").or_insert(0) += 1;
| ---------------------------^^^^^
| |
| cannot use `+=` on type `&mut {integer}`
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little confused as to the intent of this message. Is it trying to just point at the += (which seems correct)? If so, the underline-like arrow beneath the rest of the line looks really odd to me. Might just not be used to it though.. nor do I have any ideas as to what else we could do.

Copy link
Contributor Author

@estebank estebank Jun 12, 2017

Choose a reason for hiding this comment

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

@Mark-Simulacrum, the primary span which doesn't have a label is as follows:

15 |     map.entry("e").or_insert(0) += 1;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

My assumption is that there should be only one span in this error, either:

15 |     map.entry("e").or_insert(0) += 1;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot use `+=` on type `&mut {integer}`

or

15 |     map.entry("e").or_insert(0) += 1;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot use `+=` on type `&mut {integer}`

Once we fix that, I'll have to find a new way to have a test for this bug, though.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I would have expected the span to be on +=, actually, not the whole expression or some part of it. Obviously that's a separate discussion though -- no need to fix here.

@estebank estebank force-pushed the span-fix branch 3 times, most recently from 0984fa0 to 3171248 Compare June 13, 2017 05:30
@eddyb
Copy link
Member

eddyb commented Jun 13, 2017

Argh why do I keep getting picked on these PRs. r? @jonathandturner

@rust-highfive rust-highfive assigned sophiajt and unassigned eddyb Jun 13, 2017
@sophiajt
Copy link
Contributor

This is a Niko one, I think he did the drawing layout bit.

r? @nikomatsakis

--> test.rs:3:3
|
3 | a bc d
| ^^^^----
Copy link
Contributor

Choose a reason for hiding this comment

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

it's progress for sure. I still find these sorts of overlap scenarios pretty mind boggling regardless, though i'm not sure how best to fix in general. I'd sort of like to separate the ^^^ and --- on different lines. i.e., if I was highlighting this by hand I would probably do:

a  bc  d
^^^^
    ----

but anyway.

@nikomatsakis
Copy link
Contributor

I thought that "expected output" looked wrong...

[01:03:10] ---- test_snippet::multiple_labels_secondary_without_message_3 stdout ----

[01:03:10] 	span: Span { lo: BytePos(14), hi: BytePos(19), ctxt: #0 } label: "`a` is a good letter"

[01:03:10] text: Ok("a { b")

[01:03:10] span: Span { lo: BytePos(22), hi: BytePos(27), ctxt: #0 } label: ""

[01:03:10] text: Ok("c } d")

[01:03:10] expected output:

[01:03:10] ------

[01:03:10] error: foo

[01:03:10]  --> test.rs:3:3

[01:03:10]   |

[01:03:10] 3 |   a  bc  d

[01:03:10]   |   ^^^^----

[01:03:10]   |   |

[01:03:10]   |   `a` is a good letter

[01:03:10] 

[01:03:10] ------

[01:03:10] actual output:

[01:03:10] ------

[01:03:10] error: foo

[01:03:10]  --> test.rs:3:3

[01:03:10]   |

[01:03:10] 3 |   a { b { c } d }

[01:03:10]   |   ^^^^^   -----

[01:03:10]   |   |

[01:03:10]   |   `a` is a good letter

[01:03:10] 

[01:03:10] ------

r=me once this is fixed

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 13, 2017
@estebank estebank force-pushed the span-fix branch 2 times, most recently from 802659c to 1494d10 Compare June 15, 2017 15:43
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 15, 2017

📌 Commit 8074a88 has been approved by nikomatsakis

@frewsxcv
Copy link
Member

@bors rollup

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 16, 2017
Position span label correctly when it isn't last

Fix rust-lang#42595.

Before:

```
15 |     map.entry("e").or_insert(0) += 1;
   |     ---------------------------^^^^^ot use `+=` on type `&mut {integer}`
```

After:

```
15 |     map.entry("e").or_insert(0) += 1;
   |     ---------------------------^^^^^
   |     |
   |     cannot use `+=` on type `&mut {integer}`
```
bors added a commit that referenced this pull request Jun 16, 2017
Rollup of 5 pull requests

- Successful merges: #42616, #42651, #42654, #42656, #42685
- Failed merges:
@bors bors merged commit 8074a88 into rust-lang:master Jun 16, 2017
@estebank estebank deleted the span-fix branch November 9, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants