pygments themes are customizable #8752

azmeuk · 2021-06-11T15:21:06Z

This patchs allows users to set two environment variables (PYTEST_THEME and PYTEST_THEME_MODE) to customize the pygments theme to use. It fixes #7132

I know some people were interested in rich and suggested to wait and see, but this solution is so easy and unintrusive I think it might be worth right now.

What do you think?

RonnyPfannschmidt

i like this as even tho its a workaround ,it will give people with conditions like color blindness options to work with

i believe we should have some sort of test + a UsageError on case bg/style are unknown

do you want to add such a test/error handler?

azmeuk · 2021-06-11T15:46:07Z

I can. What do you expect? Catching the pygments exception and raise a custom exception with a nicer error message when the environment variables have invalid values?

nicoddemus

Thanks a lot @azmeuk!

Please also add the new variables to the reference.rst file, in the "Environment Variables" section.

doc/en/reference/customize.rst

changelog/7132.feature.rst

azmeuk · 2021-06-11T17:00:59Z

I added some tests and applied your suggestions.

azmeuk · 2021-06-11T17:01:32Z

src/_pytest/_io/terminalwriter.py

@@ -195,16 +195,34 @@ def _write_source(self, lines: Sequence[str], indents: Sequence[str] = ()) -> No

    def _highlight(self, source: str) -> str:
        """Highlight the given source code if we have markup support."""
+        from _pytest.config.exceptions import UsageError


I let this import inside the function to avoid a circular import error.

nicoddemus

Thanks @azmeuk! Looks great!

Left two other suggestions, otherwise LGTM. 👍

src/_pytest/_io/terminalwriter.py

azmeuk · 2021-06-11T17:29:50Z

Done :)

The-Compiler

I don't want to bikeshed this, but I wonder why it should be an environment variable, when (almost) all other things are either ini settings or commandline arguments?

We already have --code-highlight from #6934, so why not --code-highlight-theme and --code-highlight-theme-mode or so?

nicoddemus · 2021-06-11T18:21:04Z

I'm 👍 on configuring this as env vars... my thinking is that this is something you might want change just for yourself because of preference/color blindness issues, and this is not easily done with config vars (you can of course use command-line vars indirectly with PYTEST_ADDOPTS).

azmeuk · 2021-06-11T19:18:09Z

I suggested an environment variable because of this comment, and because each contributor of a given project may have its own terminal configuration, and need its own color customization.

I am open to add/replace a command line argument though.

RonnyPfannschmidt · 2021-06-11T19:25:20Z

unless pytest gets someething nice to unify config by env var, config file and cli argument im of the oppinion that this particular one is best placed in a env var, accompanied with some google search firendly docs for people that need it

azmeuk · 2021-06-14T08:37:07Z

It looks like the failing tests are unrelated with this patch.

nicoddemus · 2021-06-14T12:36:03Z

@The-Compiler do you agree with the reasoning for the environment variables? If so could you merge it then? Thanks! 🙇

The-Compiler · 2021-06-14T13:02:40Z

Does this really work? I don't see a change in output when I set PYTEST_THEME, and the TerminalFormatter we use doesn't seem to have a style option at all.

nicoddemus · 2021-06-14T13:21:32Z

@azmeuk did you test this yourself? Can you post some screenshots please?

azmeuk · 2021-06-14T15:00:07Z

For some reasons, just changing the theme but keeping the default dark mode, does not change anything. Here are two screenshots:

before

after

env PYTEST_THEME=solarized-dark PYTEST_THEME_MODE=light pytest path/to/test.py

azmeuk · 2021-06-14T15:07:50Z

Does this really work? I don't see a change in output when I set PYTEST_THEME, and the TerminalFormatter we use doesn't seem to have a style option at all.

I think you are right, I did not tested with other pygments themes. This piece of documentation indeed tells that TerminalFormatter does not use the style parameter. I will checks this out.

RonnyPfannschmidt · 2021-06-14T15:58:59Z

https://github.com/pygments/pygments/blob/faf69c028f0efa7798a938163e8432d838acfda2/pygments/formatters/terminal256.py#L100 will use the color style,

https://github.com/pygments/pygments/blob/faf69c028f0efa7798a938163e8432d838acfda2/pygments/formatters/terminal.py#L25-L58 will use a color scheme argument

i believe its very safe to usually pick the 256 formatter, lets figure how to correctly pick

RonnyPfannschmidt · 2021-06-14T16:00:24Z

pygments itself does a explicit choice - https://github.com/pygments/pygments/blob/faf69c028f0efa7798a938163e8432d838acfda2/pygments/cmdline.py#L390-L395

i wonder if the color sceme can be worked into the details

azmeuk · 2021-06-14T16:02:03Z

i wonder if the color sceme can be worked into the details

What do you mean? Use the same trick as pygments to guess if a TerminalFormatter or a Terminal256Formatter should be used?

RonnyPfannschmidt · 2021-06-14T16:03:40Z

@azmeuk no, i meant trying to figure of color schemes could be sensibly mapped to the low color terminal (or alternatively at least having color scemes for the visually imparied

azmeuk · 2021-06-14T16:05:11Z

Now rereading my PR I guess it does not really have sense anymore unless we indeed use Terminal256Formatter instead of TerminalFormatter. Is it ok for you if I do the move?

RonnyPfannschmidt · 2021-06-14T16:07:49Z

i'm going to be happy with formatter change

i believe for full compat we should do the same check pygments does (and print a warning if a color style is passed but not supported due to lack of 256 color terminals)

a followup then could be to provide a color sceme for the color-bind either by pygments directly or by addding one and upstreaming later

The-Compiler · 2021-06-14T17:16:03Z

We're not really in "easy and unintrusive" terrain at that point anymore, IMHO... Even pygmentize gets this wrong, checking if 256 is in $TERM is by no means sufficient to check if the terminal supports 256 colors.

Case in point, here's the pygments default output with my terminal and TerminalFormatter:

and here when it switches to 256 color mode:

So this would really make things less readable (and uniform) for many setups, rather than more so.

FWIW I still think the right solution to this is the user configuring the colors in their terminal (and sticking with the current 16-color formatter), rather than adding 256-color support just so that we can use pygment themes. Right now, the output looks like other colorized output (say, ls and such) in a terminal, while with hand-picked/hardcoded color themes, there's a much bigger chance for conflicts with how the rest of the terminal output looks.

azmeuk · 2021-06-14T17:51:40Z

I have a custom set of terminal colors (solarized), every terminal utility displays well except pytest.
I think we could meet in the middle:

If PYTEST_THEME is defined, use Terminal256Color or TerminalTrueColorFormatter, with the passed theme,
Else, by default, keep to TerminalFormatter.

What do you think @The-Compiler?

The-Compiler · 2021-06-14T18:01:44Z

I guess that'd work indeed!

azmeuk · 2021-06-14T21:49:00Z

Ok here it is.
Here are some tests with the solarized, emacs, and tango themes.

testing/test_terminal.py

hugovk

Just a little typo.

doc/en/reference/customize.rst

doc/en/reference/reference.rst

testing/test_terminal.py

src/_pytest/_io/terminalwriter.py

azmeuk · 2021-06-15T21:43:30Z

@RonnyPfannschmidt @hugovk Thank you for your feedbacks, they are fixed.

azmeuk · 2021-06-17T07:18:11Z

@The-Compiler Is this PR acceptable to you now?

The-Compiler

Thanks for the update! Seems fine now, other than the linting issues. I added a couple of comments for the mypy ones.

testing/conftest.py

azmeuk · 2021-06-17T09:30:39Z

@The-Compiler Here it is!

azmeuk · 2021-06-20T17:55:12Z

@The-Compiler Ping! Would you consider approving this PR now?

The-Compiler · 2021-06-20T19:04:44Z

While test failures are unrelated, the linters in the CI are still failing.

azmeuk · 2021-06-20T19:22:01Z

Ok. This should be fixed now.

The-Compiler

Thanks! Looks good now, but I'm still confused about the test failures. I thought they were only something unrelated being flaky, but I can see them consistently happen in your branch, but apparently not on the main branch. However, I'm unable to reproduce locally... anyone has an idea what's going on there?

azmeuk · 2021-06-20T21:44:04Z

I rebased against the main branch, just in case.

azmeuk · 2021-06-21T10:28:11Z

Different tests are failing each time, but the common thing is

pytest.PytestUnraisableExceptionWarning: Exception ignored in: <_io.FileIO [closed]>
...
ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/null' mode='w' encoding='UTF-8'>

Not sure where it comes from.

The-Compiler · 2021-06-22T12:08:38Z

I've been trying to reproduce with Python 3.9 in a Ubuntu 20.04 Docker container, but still no luck, unfortunately... Maybe someone else has an idea what could be going on.

azmeuk · 2021-07-01T21:02:19Z

@nicoddemus @RonnyPfannschmidt do you maybe have any clue?

azmeuk · 2021-07-25T16:40:03Z

I just tried -but failed- to reproduce the errors with act.

nicoddemus · 2021-07-26T14:06:57Z

@nicoddemus @RonnyPfannschmidt do you maybe have any clue?

Sorry no clue :(

Have you tried rebasing on the latest main?

azmeuk · 2021-08-13T14:27:21Z

Have you tried rebasing on the latest main?

Yep.

I found that the errors are raised randomly on different other tests due to test_code_highlight_invalid_theme. I suppose trying to access a non existing theme in pygments has side effects that make some other tests fail, depending on the order the are executed.

Here I am looking for unclosed file descriptors and warnings...

nicoddemus · 2021-08-13T16:58:57Z

I found that the errors are raised randomly on different other tests due to test_code_highlight_invalid_theme. I suppose trying to access a non existing theme in pygments has side effects that make some other tests fail, depending on the order the are executed.

If this is happening in a few tests, we can use pytester.runpytest_subprocess(), which will ensure no state will leak.

azmeuk · 2021-08-13T18:26:40Z

If this is happening in a few tests, we can use pytester.runpytest_subprocess(), which will ensure no state will leak.

❤️ ❤️ ❤️

Tests are green now, this patch should be mergeable!

nicoddemus · 2021-08-13T18:59:35Z

Woot! Thanks for the patience and following through with the PR! ❤️

azmeuk · 2021-08-15T10:20:16Z

@nicoddemus @RonnyPfannschmidt @The-Compiler anyone interested in pressing the merge button?

nicoddemus · 2021-08-15T12:06:57Z

Thanks!

RonnyPfannschmidt reviewed Jun 11, 2021

View reviewed changes

nicoddemus requested changes Jun 11, 2021

View reviewed changes

doc/en/reference/customize.rst Outdated Show resolved Hide resolved

changelog/7132.feature.rst Outdated Show resolved Hide resolved

azmeuk commented Jun 11, 2021

View reviewed changes

nicoddemus approved these changes Jun 11, 2021

View reviewed changes

src/_pytest/_io/terminalwriter.py Outdated Show resolved Hide resolved

src/_pytest/_io/terminalwriter.py Outdated Show resolved Hide resolved

The-Compiler requested changes Jun 11, 2021

View reviewed changes

azmeuk requested a review from The-Compiler June 12, 2021 13:08

RonnyPfannschmidt approved these changes Jun 14, 2021

View reviewed changes

RonnyPfannschmidt reviewed Jun 15, 2021

View reviewed changes

testing/test_terminal.py Show resolved Hide resolved

hugovk reviewed Jun 15, 2021

View reviewed changes

doc/en/reference/customize.rst Outdated Show resolved Hide resolved

doc/en/reference/reference.rst Show resolved Hide resolved

testing/test_terminal.py Show resolved Hide resolved

src/_pytest/_io/terminalwriter.py Show resolved Hide resolved

The-Compiler requested changes Jun 17, 2021

View reviewed changes

testing/conftest.py Outdated Show resolved Hide resolved

testing/conftest.py Outdated Show resolved Hide resolved

The-Compiler approved these changes Jun 20, 2021

View reviewed changes

pygments themes are customizable

7770dac

nicoddemus merged commit 0f79fc6 into pytest-dev:main Aug 15, 2021

Pierre-Sassoulas mentioned this pull request Apr 10, 2025

Dark-red coloured crash stack dump is very hard to read on a dark-themed terminal #6984

Closed

pygments themes are customizable #8752

pygments themes are customizable #8752

Conversation

azmeuk commented Jun 11, 2021

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 11, 2021

nicoddemus left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 11, 2021

azmeuk Jun 11, 2021

Choose a reason for hiding this comment

nicoddemus left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 11, 2021

The-Compiler left a comment • edited Loading

Choose a reason for hiding this comment

nicoddemus commented Jun 11, 2021

azmeuk commented Jun 11, 2021

RonnyPfannschmidt commented Jun 11, 2021

azmeuk commented Jun 14, 2021

nicoddemus commented Jun 14, 2021

The-Compiler commented Jun 14, 2021

nicoddemus commented Jun 14, 2021

azmeuk commented Jun 14, 2021 • edited Loading

before

after

azmeuk commented Jun 14, 2021

RonnyPfannschmidt commented Jun 14, 2021

RonnyPfannschmidt commented Jun 14, 2021

azmeuk commented Jun 14, 2021

RonnyPfannschmidt commented Jun 14, 2021

azmeuk commented Jun 14, 2021

RonnyPfannschmidt commented Jun 14, 2021

The-Compiler commented Jun 14, 2021

azmeuk commented Jun 14, 2021 • edited Loading

The-Compiler commented Jun 14, 2021

azmeuk commented Jun 14, 2021

hugovk left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 15, 2021

azmeuk commented Jun 17, 2021

The-Compiler left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 17, 2021 • edited Loading

azmeuk commented Jun 20, 2021 • edited Loading

The-Compiler commented Jun 20, 2021 • edited Loading

azmeuk commented Jun 20, 2021

The-Compiler left a comment

Choose a reason for hiding this comment

azmeuk commented Jun 20, 2021

azmeuk commented Jun 21, 2021 • edited Loading

The-Compiler commented Jun 22, 2021

azmeuk commented Jul 1, 2021

azmeuk commented Jul 25, 2021

nicoddemus commented Jul 26, 2021

azmeuk commented Aug 13, 2021 • edited Loading

nicoddemus commented Aug 13, 2021

azmeuk commented Aug 13, 2021

nicoddemus commented Aug 13, 2021

azmeuk commented Aug 15, 2021

nicoddemus commented Aug 15, 2021

The-Compiler left a comment •

edited

Loading

azmeuk commented Jun 14, 2021 •

edited

Loading

azmeuk commented Jun 14, 2021 •

edited

Loading

azmeuk commented Jun 17, 2021 •

edited

Loading

azmeuk commented Jun 20, 2021 •

edited

Loading

The-Compiler commented Jun 20, 2021 •

edited

Loading

azmeuk commented Jun 21, 2021 •

edited

Loading

azmeuk commented Aug 13, 2021 •

edited

Loading