-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
Use color to highlight error locations #112730
Comments
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
This looks really good, thanks! Perhaps something for a followup, can we customise the colouring a bit? PR #112732 right now: GitHub / MagicPythonGitHub markdown (with Traceback (most recent call last):
File "/Users/hugo/github/python/cpython/main/1.py", line 1, in <module>
1/0
~^~
ZeroDivisionError: division by zero (Uses https://github.com/MagicStack/MagicPython for Python tracebacks: https://github.com/github-linguist/linguist/blob/master/vendor/README.md) Sphinx / pygmentsSphinx with (Uses Pygments' Native style) |
Yup, see #112732 (comment). I want to explore customization options AFTER we land the basic version first. EDIT Or are you referring to customizing the coloring as "adding more colors"? On the other hand, are you suggesting to also colorize the name of the exception, the file, the number and the Traceback text? I have been told that yellow and cian are really bad for light mode, so we should do something else there. |
@hugovk What about like this? |
I wasn't thinking about allowing the user to customise them (although that's something to consider) but about having different colours for the different elements (exception name, file, etc). Yes, we should check the contrast is good for both light and dark modes. The GitHub colours (ignoring the plain black/white text) look the same for each mode and display well for both: edit: and the blues are different, but you get the idea :) |
I am quite colorblind myself 😅 Do you mind telling me what colors are supposed to be every piece? As in "filename is ..., line number is ...., function name is ..., source code is ..., error location is ...) |
Signed-off-by: Pablo Galindo <pablogsal@gmail.com> Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Where are we on allowing user-specified colors? On my system the REPL background is black, and the exception text is a dark purple which is extremely hard to read. |
Currently the only option is to use |
@ethanfurman Please could you post a screenshot? What OS/terminal/etc do you use? |
Note that this still needs support for |
Also I don't see NO_COLORS and FORCE_COLORS listed in |
|
But then shouldn’t the argument be that Python shouldn’t be sensitive to env vars that don’t start with PYTHON? |
I personally think it's very useful for Python to use the same env vars that are being increasingly widely adopted elsewhere. So I suppose I lean towards changing |
The "community standards" like https://no-color.org and https://force-color.org are trying to solve the problem that every tool has their own env vars, which makes it hard to configure them all individually. And having our own |
@hugovk mentioned "GitHub Actions is another good place to use FORCE_COLOR. It's not a tty so autodetect doesn't work, but in most cases colour really helps you find errors in long logs." in chat... If so, resolving the test failure issue by disabling them during a |
Yeah I think we can just skip these particular tests if the environment variables are set (or check if we can make them work regardless of what is set) while keeping the whole test suite working with whatever env is set to benefit from the colours in the logs. |
Maybe we could just add a support decorator that just does the appropriate monkey patching in the colorise function to avoid being affected by the force colours variable. To avoid regressions, setting FORCE_COLORS in GH actions sounds like a good idea. |
I think if we don't do this it's going to be a full pain for us and a lot of users that have the variable set in CI and are running subprocesses in tests |
…ronment variables
#117672 broke several stable buildbots, see e.g. here. As Jakub noted they seem to be Do you have time to look into this? |
I will look into this today. We can revert in the meanwhile if you wish |
…h no-colorize fixes
I think this will be fixed by #118288 |
#118288 has been merged, closing. |
This has several advantages:
Control features:
PY_COLORS=1
activates the feature (used by pytest: https://github.com/pytest-dev/pytest/blob/022f1b4de546c8b3529e071965555888ecf01cb4/src/_pytest/_io/terminalwriter.py#L28)PY_COLORS=0
deactivates the featureNO_COLOR=1
deactivates the featureFORCE_COLOR=1
activates the featureTERM
is set todumb
.Linked PRs
The text was updated successfully, but these errors were encountered: