Skip to content

tidy: Make errors be printed in red and underlined #18826

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

Closed
wants to merge 1 commit into from

Conversation

scialex
Copy link
Contributor

@scialex scialex commented Nov 10, 2014

This is what it looks like with this patch
Image of an error

@alexcrichton
Copy link
Member

Sadly I think that this is a unix-only-ism and may not work on windows, do you know if python translates this under the hood for windows?

@scialex
Copy link
Contributor Author

scialex commented Nov 10, 2014

@alexcrichton No. I bet there is some package on PyPI that does it but I don't know of one off the top of my head. OTOH this is just a Lint tool, even if it does not work all that will change is that something like "?[4;31m" will be printed before the error and "[0m" will be printed after it, which doesn't seem like all that bad an outcome considering it will help most people...

@ghost
Copy link

ghost commented Nov 10, 2014

@scialex I think https://pypi.python.org/pypi/colorama is what you have in mind.

But for now we should be able to detect if we can print colours with https://docs.python.org/3/library/curses.html#curses.has_colors. r=me with that change.

@scialex
Copy link
Contributor Author

scialex commented Nov 11, 2014

@jakub- that won't work. Starting up curses clears the screen, which would somewhat defeat the purpose. It would also totally kill things like the travis, which rely on pipes working intelligently.

@bstrie
Copy link
Contributor

bstrie commented Nov 20, 2014

We print colors elsewhere, so surely this shouldn't be blocked on that. If the problem is underlining, then I'd be fine removing the underlining and keeping the color. Even without the underline it stands out quite nicely.

Or am I misunderstanding how we handle color?

@alexcrichton
Copy link
Member

I believe that all of our color printing from the compiler at least will perform detection of support at runtime. Not all shells work by printing these escapes (notably windows ones don't), and the compiler detects the best way to do so. If there was an easy way to guard this for only shells that support this form of colorization, then that would be great!

@nodakai
Copy link
Contributor

nodakai commented Nov 20, 2014

Using the curses module should be preferred over hard-coding ANSI escape sequences. Cf. http://en.wikipedia.org/wiki/Tput#Usage

I'm not sure if Windows console is supported, but we can probably check with the curses.has_colors() method.

@scialex
Copy link
Contributor Author

scialex commented Nov 21, 2014

@nodakai No what we really need is something like https://pypi.python.org/pypi/colorama as @jakub- suggested. The problem with using curses is that is screws up piping if we aren't very careful, which would kill travis.

@nodakai
Copy link
Contributor

nodakai commented Nov 21, 2014

Ah, I overlooked @jakub- already mentioned curses.has_colors()...

So we have two issues:

  1. Windows(NT) doesn't support ANSI escape sequence at all. colorama addresses this issue by wrapping stdout and translating them to corresponding Win32 API. That's a nice addition. But...
  2. Some tools like Travis which parses redirected stdout might get confused by ANSI escape sequences. colorama doesn't help here, so something like os.isatty() would be necessary after all.
(foo)$ cat a.py    
import colorama
print(colorama.Fore.RED + "Hello")
(foo)$ python a.py | xxd
0000000: 1b5b 3331 6d48 656c 6c6f 0a              .[31mHello.

Because of 2, we need to programatically switch on/off terminal coloring depending on the result of os.isatty(). Then hard-coding ANSI escape sequence is no good and we need a clever use of either curses.tigetstr() etc. or colorama.Fore.*.

In my previous comment, I almost suggested to disable terminal coloring on Windows by simply checking curses.has_colors() (because I don't use Windows at all 😁)

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with the windows issues addressed!

lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 7, 2025
fix: Clear flycheck diagnostics per package properly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants