Reorder span suggestions to appear below main labels #43251

gaurikholkar-zz · 2017-07-15T12:49:29Z

A fix to #41698

oli-obk

Just a nit. Lgtm otherwise

oli-obk · 2017-07-16T08:59:54Z

src/librustc_errors/emitter.rs

@@ -347,8 +347,7 @@ impl EmitterWriter {

        // Sort the annotations by (start, end col)
        let mut annotations = line.annotations.clone();
-        annotations.sort();
-        annotations.reverse();
+        annotations.sort_by(|a,b| b.start_col.cmp(&a.start_col));


Please add a comment here about what and why this is done, the reversing is pretty subtle.

I agree that a comment would be good. I personally have no idea why this yields the correct result. =) But the change looks good!

If you have a list of annotations (A1, A2, C1, C2, B1, B2) where the letter signifies the span, since we are only sorting by the span, the order of the elements with the same span will never change. Since we are reversing the ordering (|a, b| but b.cmp(a)), you get (C1, C2, B1, B2, A1, A2). All the elements with the same span are still ordered first to last, but all the elements with different spans are ordered by their spans in last to first order. Last to first order is important, because the jiggly lines and | are on the left, so the rightmost span needs to be rendered first, otherwise the lines would end up needing to go over a message.

If the above is comprehensible, I think it can be copy pasted as the requested comment ;)

since we are only sorting by the span, the order of the elements with the same span will never change

This is because our sort is stable?

@nikomatsakis the documentation says that sort_by is stable though. @oli-obk @evolarium

This seems like a pretty decent description. =)

oli-obk · 2017-07-16T09:41:59Z

hmm, bors does't like review comments. I'm not a reviewer, so:

r? @nikomatsakis

nikomatsakis

A comment explaining why the sort produces the desired effect would be good! But otherwise looks nice.

bors · 2017-07-17T13:34:43Z

☔ The latest upstream changes (presumably #42033) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2017-07-17T14:13:05Z

@bors r+

bors · 2017-07-17T14:13:06Z

📌 Commit 66f3c07 has been approved by nikomatsakis

oli-obk · 2017-07-17T14:59:23Z

You have some trailing whitespace in emitter.rs

[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:351: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:352: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:354: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:355: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:356: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:357: trailing whitespace
[00:03:00] tidy error: /checkout/src/librustc_errors/emitter.rs:358: trailing whitespace

gaurikholkar-zz · 2017-07-17T15:12:28Z

@oli-obk fixed the tidy errors.

nikomatsakis · 2017-07-17T15:14:22Z

@bors r+

bors · 2017-07-17T15:14:23Z

📌 Commit 601a7a9 has been approved by nikomatsakis

Mark-Simulacrum · 2017-07-17T18:36:08Z

Could you squash the commits down?

nikomatsakis · 2017-07-17T19:16:23Z

@bors r+

bors · 2017-07-17T19:16:23Z

📌 Commit 26a8357 has been approved by nikomatsakis

bors · 2017-07-18T03:50:22Z

⌛ Testing commit 26a8357 with merge 67cdb87d51c55b3a1b2e70f37992f70f8fcda9e9...

bors · 2017-07-18T03:56:11Z

💔 Test failed - status-travis

est31 · 2017-07-18T06:51:30Z

@bors retry

bors · 2017-07-19T08:58:00Z

⌛ Testing commit 26a8357 with merge f868a5ba86c3e204cfde67a7ec9405bd6433bbee...

bors · 2017-07-19T09:02:08Z

💔 Test failed - status-appveyor

oli-obk · 2017-07-19T09:08:16Z

spurious?

 Downloading toml v0.1.30
error: unable to get packages from source
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: Unknown error (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.)
failed to run: C:\projects\rust\build\x86_64-pc-windows-msvc\stage0\bin/cargo.exe build --manifest-path C:\projects\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:01:05
make: *** [Makefile:77: prepare] Error 1
Command failed. Attempt 2/5:
 Downloading libc v0.2.26
error: unable to get packages from source
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: Unknown error (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.)
failed to run: C:\projects\rust\build\x86_64-pc-windows-msvc\stage0\bin/cargo.exe build --manifest-path C:\projects\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:00
make: *** [Makefile:77: prepare] Error 1
Command failed. Attempt 3/5:
 Downloading cmake v0.1.24
error: unable to get packages from source
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: Unknown error (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.)
failed to run: C:\projects\rust\build\x86_64-pc-windows-msvc\stage0\bin/cargo.exe build --manifest-path C:\projects\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:00
make: *** [Makefile:77: prepare] Error 1
Command failed. Attempt 4/5:
 Downloading libc v0.2.26
error: unable to get packages from source
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: Unknown error (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.)
failed to run: C:\projects\rust\build\x86_64-pc-windows-msvc\stage0\bin/cargo.exe build --manifest-path C:\projects\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:00
make: *** [Makefile:77: prepare] Error 1
Command failed. Attempt 5/5:
 Downloading filetime v0.1.10
error: unable to get packages from source
Caused by:
  [35] SSL connect error (schannel: next InitializeSecurityContext failed: Unknown error (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline.)
failed to run: C:\projects\rust\build\x86_64-pc-windows-msvc\stage0\bin/cargo.exe build --manifest-path C:\projects\rust\src/bootstrap/Cargo.toml --locked
Build completed unsuccessfully in 0:00:00
make: *** [Makefile:77: prepare] Error 1
The command has failed after 5 attempts.
Command exited with code 1

gaurikholkar-zz · 2017-07-19T10:08:17Z

Network connectivity issues?

est31 · 2017-07-19T10:34:50Z

@gaurikholkar #43178 (comment)

nagbot-rs · 2017-07-19T14:13:04Z

@bors: retry

…

On Wed, Jul 19, 2017 at 4:02 AM, bors ***@***.***> wrote: 💔 Test failed - status-appveyor <https://ci.appveyor.com/project/rust-lang/rust/build/1.0.3972> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#43251 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AaPN0C1dUzhMugaxuquAokDAsrfW3imXks5sPcYkgaJpZM4OZAep> . -- You received this message because you are subscribed to the Google Groups "rust-ops" group. To unsubscribe from this group and stop receiving emails from it, send an email to ***@***.*** To post to this group, send email to ***@***.*** To view this discussion on the web visit https://groups.google.com/d/ msgid/rust-ops/rust-lang/rust/pull/43251/c316320506%40github.com <https://groups.google.com/d/msgid/rust-ops/rust-lang/rust/pull/43251/c316320506%40github.com?utm_medium=email&utm_source=footer> . For more options, visit https://groups.google.com/d/optout.

bors · 2017-07-19T14:13:05Z

@nagbot-rs: 🔑 Insufficient privileges: and not in try users

alexcrichton · 2017-07-19T14:14:24Z

@bors: retry

bors · 2017-07-20T03:36:20Z

⌛ Testing commit 26a8357 with merge 1beaea2...

@nikomatsakis

Reorder span suggestions to appear below main labels A fix to #41698 r? @nikomatsakis

bors · 2017-07-20T06:38:37Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 1beaea2 to master...

oli-obk requested changes Jul 16, 2017

View reviewed changes

Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2017

Mark-Simulacrum assigned nikomatsakis Jul 17, 2017

nikomatsakis requested changes Jul 17, 2017

View reviewed changes

gaurikholkar-zz force-pushed the span_reorder branch from e38877f to 66f3c07 Compare July 17, 2017 13:50

reorder span labels

26a8357

gaurikholkar-zz force-pushed the span_reorder branch from db9b62b to 26a8357 Compare July 17, 2017 18:38

arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2017

bors added a commit that referenced this pull request Jul 20, 2017

Auto merge of #43251 - gaurikholkar:span_reorder, r=nikomatsakis

1beaea2

Reorder span suggestions to appear below main labels A fix to #41698 r? @nikomatsakis

bors merged commit 26a8357 into rust-lang:master Jul 20, 2017

estebank mentioned this pull request Sep 1, 2017

Reorder span_suggestions to appear below main labels #40851 #41698

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorder span suggestions to appear below main labels #43251

Reorder span suggestions to appear below main labels #43251

gaurikholkar-zz commented Jul 15, 2017 •

edited

Loading

oli-obk left a comment

oli-obk Jul 16, 2017

nikomatsakis Jul 17, 2017

oli-obk Jul 17, 2017

nikomatsakis Jul 17, 2017

gaurikholkar-zz Jul 17, 2017 •

edited

Loading

nikomatsakis Jul 17, 2017

oli-obk commented Jul 16, 2017

nikomatsakis left a comment

bors commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

oli-obk commented Jul 17, 2017

gaurikholkar-zz commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

Mark-Simulacrum commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

bors commented Jul 18, 2017

bors commented Jul 18, 2017

est31 commented Jul 18, 2017

bors commented Jul 19, 2017

bors commented Jul 19, 2017

oli-obk commented Jul 19, 2017

gaurikholkar-zz commented Jul 19, 2017

est31 commented Jul 19, 2017

nagbot-rs commented Jul 19, 2017 via email

bors commented Jul 19, 2017

alexcrichton commented Jul 19, 2017

bors commented Jul 20, 2017

bors commented Jul 20, 2017

Reorder span suggestions to appear below main labels #43251

Reorder span suggestions to appear below main labels #43251

Conversation

gaurikholkar-zz commented Jul 15, 2017 • edited Loading

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk Jul 16, 2017

Choose a reason for hiding this comment

nikomatsakis Jul 17, 2017

Choose a reason for hiding this comment

oli-obk Jul 17, 2017

Choose a reason for hiding this comment

nikomatsakis Jul 17, 2017

Choose a reason for hiding this comment

gaurikholkar-zz Jul 17, 2017 • edited Loading

Choose a reason for hiding this comment

nikomatsakis Jul 17, 2017

Choose a reason for hiding this comment

oli-obk commented Jul 16, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

bors commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

oli-obk commented Jul 17, 2017

gaurikholkar-zz commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

Mark-Simulacrum commented Jul 17, 2017

nikomatsakis commented Jul 17, 2017

bors commented Jul 17, 2017

bors commented Jul 18, 2017

bors commented Jul 18, 2017

est31 commented Jul 18, 2017

bors commented Jul 19, 2017

bors commented Jul 19, 2017

oli-obk commented Jul 19, 2017

gaurikholkar-zz commented Jul 19, 2017

est31 commented Jul 19, 2017

nagbot-rs commented Jul 19, 2017 via email

bors commented Jul 19, 2017

alexcrichton commented Jul 19, 2017

bors commented Jul 20, 2017

bors commented Jul 20, 2017

gaurikholkar-zz commented Jul 15, 2017 •

edited

Loading

gaurikholkar-zz Jul 17, 2017 •

edited

Loading