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

Diagnostics' ^~~~ is not aligned properly when error contains 日本語 characters #21492

Closed
ghost opened this issue Jan 22, 2015 · 7 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@ghost
Copy link

ghost commented Jan 22, 2015

Note the misaligned ^~~~~~~ below due to the 日本語 characters(their length is not computed properly?):

$ rustc -v main.rs 
main.rs:3:38: 3:43 error: unresolved name `hello`
main.rs:3     println!("Hello in English: {}", hello());
                                               ^~~~~
note: in expansion of format_args!
<std macros>:2:42: 2:75 note: expansion site
<std macros>:1:1: 2:77 note: in expansion of println!
main.rs:3:5: 3:47 note: expansion site
main.rs:4:46: 4:53 error: unresolved name `goodbye`
main.rs:4     println!("Goodbye in Japanese(日本語): {}", goodbye());
                                                       ^~~~~~~
note: in expansion of format_args!
<std macros>:2:42: 2:75 note: expansion site
<std macros>:1:1: 2:77 note: in expansion of println!
main.rs:4:5: 4:57 note: expansion site
error: aborting due to 2 previous errors

main.rs is this(compiled with rustc -v main.rs):

fn main() {
    println!("Hello in English: {}", hello());
    println!("Goodbye in Japanese(日本語): {}", goodbye());
}
$ rustc --version --verbose
rustc 1.0.0-dev (3bf41dafc 2015-01-20 06:45:02 +0000)
binary: rustc
commit-hash: 3bf41dafcfb6c979efb4e2438e047e1a54045eec
commit-date: 2015-01-20 06:45:02 +0000
host: x86_64-unknown-linux-gnu
release: 1.0.0-dev

tested in urxvt and xfce4-terminal (TERM is rxvt-unicode-256color respectively xterm)
Linux 3.19.0-rc5-gec6f34e

Thank you.

@ftxqxd
Copy link
Contributor

ftxqxd commented Jan 22, 2015

Dupe of #8706, I think, although that issue doesn’t directly mention multi-column characters, only zero-column characters.

@kmcallister kmcallister added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 22, 2015
@mandel59
Copy link

Unicode Standard Annex #11 East Asian Width
http://unicode.org/reports/tr11/
There is a hard problem that the width of East Asian Ambiguous characters depends on environments.

@Florob
Copy link
Contributor

Florob commented Jan 24, 2015

I think this is simply an artefact of not handling East Asian Width at all. As such this is not a duplicate of #8706, one of these CJK codepoints would still be one grapheme, but their glyph occupies the room of two "regular" glyphs. This specific case also seems unrelated to East Asian Ambiguous characters, as all characters in the example are East Asian Wide. In general we need a decision on how and how thoroughly to deal with East Asian Width though.

@ftxqxd
Copy link
Contributor

ftxqxd commented Jan 24, 2015

My PR #21499 fixes this issue. I believe this is a dupe of #8706 because the issue does in fact (although I didn’t notice it at first) mention multi-column characters:

The proper fix for this requires grapheme handling (#7043), e.g. some graphemes are double width.

@ghost
Copy link
Author

ghost commented Jan 24, 2015

tl;dr: confirmed that PR fixes this particular issue(/example)

main.rs:2:36: 2:41 error: unresolved name `hello`
main.rs:2   println!("Hello in English: {}", hello());
                                             ^~~~~
note: in expansion of format_args!
<std macros>:2:42: 2:75 note: expansion site
<std macros>:1:1: 2:77 note: in expansion of println!
main.rs:2:3: 2:45 note: expansion site
main.rs:3:44: 3:51 error: unresolved name `goodbye`
main.rs:3   println!("Goodbye in Japanese(日本語): {}", goodbye());
                                                        ^~~~~~~
note: in expansion of format_args!
<std macros>:2:42: 2:75 note: expansion site
<std macros>:1:1: 2:77 note: in expansion of println!
main.rs:3:3: 3:55 note: expansion site
error: aborting due to 2 previous errors

(here it looks unaligned by one char due to github markup?, in edit mode(Write) looks fine, and on console screen too!, on Preview looks one char off)

--------- rant about long compile times:
I've been trying to test exactly that PR, since yesterday, but the amount of time it takes to compile Rust is demoralizing to be honest. Took this long because I stumbled on some other issues which required recompiling a few times.

I compiled master now (took ~90mins for rust and ~29 mins more for tests) and now I applied your PR and I'm currently waiting (ok it's done now after 118mins ) for the recompile.
I used time ( time make -j4 all NO_REBUILD=1 TIME_PASSES=1 TIME_LLVM_PASSES=1 && time make check ) to compile.

Someone please tell me where do I donate(to rust devs) to reduce this compile time significantly ? (incremental compilation? or something)

The long compile time just, breaks my heart...

@Florob
Copy link
Contributor

Florob commented Jan 24, 2015

@P1start My point was that counting grapheme clusters instead of "characters" is not sufficient. The width() method used in your patch works by summing each characters width, and doesn't actually deal with grapheme clusters at all. So while the title and description of #8706 seems misleading I agree that this is a dupe of sorts, if PR #21499 is the going to be the fix.

@ghost
Copy link
Author

ghost commented Feb 4, 2015

Shouldn't this be closed due to #21499 ? I mean, it fixes my original issue, unless you guys think that not all cases are handled or something. Reopen if that's the case. (or maybe a new issue should be created for those cases instead)
Thanks

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

4 participants