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

Fix mispositioned span in suggestions with wide characters #47407

Merged
merged 6 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,8 +1187,8 @@ impl EmitterWriter {
let sub_len = parts[0].snippet.trim().chars().fold(0, |acc, ch| {
acc + unicode_width::UnicodeWidthChar::width(ch).unwrap_or(0)
});
let underline_start = span_start_pos.col.0 + start;
let underline_end = span_start_pos.col.0 + start + sub_len;
let underline_start = span_start_pos.col_display + start;
let underline_end = span_start_pos.col_display + start + sub_len;
for p in underline_start..underline_end {
buffer.putc(row_num,
max_line_num_len + 3 + p,
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/issue-47377.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
fn main(){
let b = "hello";
Copy link
Member

Choose a reason for hiding this comment

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

This file doesn't use tabs. You need to change the indentation of these two lines to use tabs and then add a line // ignore-tidy-tab to get tidy passing, like done in src/test/ui/codemap_tests/tab_2.rs.

Copy link
Author

Choose a reason for hiding this comment

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

@est31 I changed it to ~^ ERROR E0369 }. Do I still need //ignore-tidy-tab?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think ignore-tidy-tab is so that the tidy check for the project doesn't block the merge of the PR, while keeping the tabs in the code so that we avoid regressions in the handling of them. I believe your editor might be "helpfully" replacing tabs with spaces.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah it was. Can you have a look now?

let _a = b + ", World!";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still using spaces instead of tabs.

//~^ ERROR E0369
}
12 changes: 12 additions & 0 deletions src/test/ui/issue-47377.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0369]: binary operation `+` cannot be applied to type `&str`
--> $DIR/issue-47377.rs:12:14
|
12 | let _a = b + ", World!";
| ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
|
12 | let _a = b.to_owned() + ", World!";
| ^^^^^^^^^^^^

error: aborting due to previous error

15 changes: 15 additions & 0 deletions src/test/ui/issue-47380.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-tidy-tab
Copy link
Member

Choose a reason for hiding this comment

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

This is not the file that should get this line, the other one should get it with the tabs. Here we are testing for emojis.

fn main(){
let b = "hello";
println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
//~^ ERROR E0369
}
12 changes: 12 additions & 0 deletions src/test/ui/issue-47380.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0369]: binary operation `+` cannot be applied to type `&str`
--> $DIR/blah.rs:13:33
|
13 | println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
| ^^^^^^^^^^^^^^ `+` can't be used to concatenate two `&str` strings
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
|
13 | println!("🦀🦀🦀🦀🦀"); let _a = b.to_owned() + ", World!";
Copy link
Contributor

Choose a reason for hiding this comment

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

Note for reviewers: this looks misaligned on the browser, but not in terminals that handle emojis correctly.

Copy link
Author

Choose a reason for hiding this comment

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

@estebank the output is misaligned in iTerm though

| ^^^^^^^^^^^^

error: aborting due to previous error