Uh oh!
There was an error while loading. Please reload this page.
Use Unicode line drawing characters for spans - #28902
Conversation
With some other cleanup at the use sites.
…utput I bet we can find other uses for these sorts of characters in clarifying type errors, etc. This is marked as a "stable" flag for consistency with --color. However we should reconsider both of them as part of rust-lang#19051. For these features to be conveniently available with Cargo, we may want to use environment variables or a config file.
rust-highfive
commented
Oct 8, 2015
(rust_highfive has picked a reviewer for you, use r? to override) |
Manishearth
commented
Oct 8, 2015
(Rather busy right now, might not get time to review, sorry. @alexcrichton, could you have a look through this?) |
alexcrichton
commented
Oct 9, 2015
Thanks for picking this up @LeoTestard! I think there's definitely an interesting question of defaults here, so it's something I wouldn't mind exploring a bit. I would be a little wary to turn this on by default as I haven't seen much precedent for CLI tools using unicode symbols by default, but on the other hand I feel like if it's turned off by default it may just end up being dead code in the compiler. For example Cargo wouldn't have an easy method of passing this down to the compiler so it'd probably end up happening that the vast majority of compiler invocations don't pass this option. So along those lines, this seems cool, but I'm a little wary about landing this for now. cc @rust-lang/tools, though, other opinions? |
wthrowe
commented
Oct 9, 2015
gcc uses unicode (smart quotes) by default in a UTF-8 locale since at least version 4.6.1 (earliest I have handy to test): |
LeoTestard
commented
Oct 10, 2015
@alexcrichton I agree. We have several options here. As @wthrowe and other people said, GCC does use Unicode output by default when the locale includes UTF-8. An other option would be an environment variable, as suggested in the previous thread. This way the user can specify that it wants the Unicode output to be enabled without Cargo having to know anything about it. A third possibility is a global configuration file for Cargo in which it is possible to specify this kind of things. Both those options allow not to set Unicode output as the default in the compiler. |
alexcrichton
commented
Oct 12, 2015
Aha fascinating! That seems like a plausible vector do discover whether this should be turned on by default, and then an environment variable to force it off seems plausible. |
retep998
commented
Oct 12, 2015
Note that for the Windows console, the console encoding doesn't matter since we use the wide functions anyway. What really matters is the current console font, which we can get the name of via |
nrc
commented
Oct 12, 2015
Opinion: I'd like to see this, but I also worry about the defaults. I'd prefer to be implicit if we can, rather than use command line options or a config file. If we can be sure the user can handle the unicode lines, we should do it. But we should never do it if we can't. Can we ensure that? Or am I dreaming? In the future, errors for tools will be serialised data structures, rather than text, so there is no issue in that context. |
sanxiyn
commented
Oct 15, 2015
Travis failure is legit (failed tidy). |
bors
commented
Nov 3, 2015
☔ The latest upstream changes (presumably #29285) made this pull request unmergeable. Please resolve the merge conflicts. |
sanxiyn
commented
Nov 25, 2015
Do you mind if I take this over? |
alexcrichton
commented
Dec 2, 2015
Closing due to inactivity, but feel free to resubmit with a rebase! |
LeoTestard
commented
Dec 4, 2015
@sanxiyn I'd still like to work on this PR, but I've got the impression that there is no general consensus about what to do for the options and the defaults. |
Manishearth
commented
Feb 29, 2016
@nrc now that we're in the future with proper json errors, should we push this again? |
nrc
commented
Mar 14, 2016
Yeah, I guess we can. I think what to do by default is the big question here, but it sounds like checking the locale is an acceptable way forward. So, if that works, let's do it! |
See #24387.
This is a port of the first PR (#21406) to the current master branch. I also added a commit to change the characters to use WGL4 instead, as suggested in the comment thread of the original PR, since those are more widely supported.
The proposal that was more or less accepted was to remove the
--colorand--drawingoptions and replace them with environment variables and a single--output-plain-asciioption. The problem is that--colorstill exists, and a stable option. I left--drawingfor now, for consistency with--colorbut there are maybe better options.This PR is not yet ready to be merged but I wanted to post it to have feedback on this issue. The proposal also included auto-enabling Unicode output when writing to a terminal and LC_CTYPE ends with
.UTF-8. I'll implement this if there is still agreement that this should be done.cc @Manishearth.
(PS: I did not change the author of the commits since I did not implement the Unicode outputting itself, but the commits from the original PR needed a lot of fixes to build on the current master branch, maybe I should have. Let me know if I should. Doing a separate commit was also possible but generated a huge diff of duplicate modifications.)