logging: Make it possible to add cli colors to custom log levels #8804
Conversation
Zac-HD
added
plugin: logging
|
Thanks @terjr for the contribution! Sorry for the delay here, seems most of the contributors are busy (including me). This might be a bit controversial because usually we don't recommend users to explicitly configure internal plugins like that, but I'm totally fine with this approach if we mark it as experimental. Other than that it would be nice to update to update logging.rst with a section demonstrating how to customize logging colors. 👍 |
|
@nicoddemus Thanks for having a look! I just wrote some documentation and marked the feature as experimental. |
src/_pytest/logging.py
Outdated
| for level, color_opts in self.LOGLEVEL_COLOROPTS.items(): | ||
| self.add_color_level(level, *color_opts) | ||
|
|
||
| def add_color_level(self, level, *color_opts): |
There was a problem hiding this comment.
can we do anything about the style color options here
every time we add a terminalwriter style into the api we lock this broken api i want to remove more into pytest
how about migrating from pylib color options to colorama options?
There was a problem hiding this comment.
What do you mean? The TerminalWriter color codes are just ANSI escape codes that shouldn't change too often...
@final
class TerminalWriter:
_esctable = dict(
black=30,
red=31,
green=32,
yellow=33,
blue=34,
purple=35,
cyan=36,
white=37,
Black=40,
Red=41,
Green=42,
Yellow=43,
Blue=44,
Purple=45,
Cyan=46,
White=47,
bold=1,
light=2,
blink=5,
invert=7,
)
There was a problem hiding this comment.
i want to eventually remove the terminalwriter from pytest
which is why id like to avoid putting its apis into other places when possible,
unfortunately we have no alternative ready right now
There was a problem hiding this comment.
Ok, any timeline on when an alternative will be ready?
There was a problem hiding this comment.
Ok, any timeline on when an alternative will be ready?
Unfortunately we don't have a timeline.
I understand @RonnyPfannschmidt's feelings here, the terminal writer markup codes are something we want to move away from, but there are already other APIs which use them, and we don't have a clear clear definition of what we want to use instead.
Regardless, even if in the future we get around to replace the markup codes with something else, the same deprecation/upgrade path that we will need to use in other parts of pytest that use markup codes, would be applied here as well.
So I don't think this should be a blocker for the PR.
how about migrating from pylib color options to colorama options?
That's not a good idea either, because we are then introducing an alternative way to declare markup options which is already deprecated from the start, because I'm sure we don't want to lock the markup definition to colorama either.
There was a problem hiding this comment.
i will have to agree, as i im unable commit to a timeline for anything alternative
its not right to stop this pr from going in just cause its using the api that is actually available
There was a problem hiding this comment.
Thanks @terjr!
I left some minor requests for changes related to documentation mostly.
src/_pytest/logging.py
Outdated
| for level, color_opts in self.LOGLEVEL_COLOROPTS.items(): | ||
| self.add_color_level(level, *color_opts) | ||
|
|
||
| def add_color_level(self, level, *color_opts): |
There was a problem hiding this comment.
Ok, any timeline on when an alternative will be ready?
Unfortunately we don't have a timeline.
I understand @RonnyPfannschmidt's feelings here, the terminal writer markup codes are something we want to move away from, but there are already other APIs which use them, and we don't have a clear clear definition of what we want to use instead.
Regardless, even if in the future we get around to replace the markup codes with something else, the same deprecation/upgrade path that we will need to use in other parts of pytest that use markup codes, would be applied here as well.
So I don't think this should be a blocker for the PR.
how about migrating from pylib color options to colorama options?
That's not a good idea either, because we are then introducing an alternative way to declare markup options which is already deprecated from the start, because I'm sure we don't want to lock the markup definition to colorama either.
| def add_color_level(self, level, *color_opts): | ||
| """Add or update color opts for a log level. | ||
|
|
||
| .. warning:: |
There was a problem hiding this comment.
I think it would be nice to describe color_opts, and even provide an example.
Co-authored-by: Bruno Oliveira <[email protected]>
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Thanks @terjr, this looks great!
Let's wait for @RonnyPfannschmidt to give it another look before merging. 👍
nicoddemus
changed the title
Closes pytest-dev#8803 PR pytest-dev#8804 Co-authored-by: Bruno Oliveira <[email protected]> Co-authored-by: Terje Runde <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This PR adds the possibility to color custom log levels on the CLI:
Not sure how this can be unit tested or where to add documentation. Please comment.
Closes #8803.