Skip to content
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

Change stderr color to grey instead of red #2944

Closed
wants to merge 1 commit into from

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Mar 14, 2023

As stderr is more use for logging than display errors, we better avoid using red and go for a neutral gray color.

This should avoid visual strain on users or confusions about presence of red colour on logs, which is almost always a sign of a important, usually fatal, error.

Thanks for contribution

Please, make sure you address all the checklists (for details on how see
development documentation)!

  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

Before

After

@ssbarnea ssbarnea added the bug:minor does not affect many people or has no big impact label Mar 14, 2023
Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I tend to disagree here 🤷 so I'll wait other maintainers opinion. cc @masenf @jugmac00

@ssbarnea
Copy link
Member Author

ssbarnea commented Mar 14, 2023

@gaborbernat I can open a poll about the subject, maybe we get other ideas about how we could about the extensive red output on screen, maybe other default color to use than the one I proposed and a lot more feedback from the users. WDYT?

PS. At least we do agree on the desire to distinguish between stdout and stderr, as you would not have implemented the red features if you would not.

@gaborbernat
Copy link
Member

I personally prefer the status quo but will listen to other maintainers opinion. The red does not bother me personally, and feels appropiate.

As stderr is more use for logging than display errors, we better
avoid using red and go for a neutral gray color.
@masenf
Copy link
Collaborator

masenf commented Mar 14, 2023

I don't have strong opinions on the matter, but I agree it is nice to visually distinguish what is stdout and stderr. In my experience with test systems, I've mostly seen stderr colored red, or red background (in web views).

I'll try to broaden my perspective a bit and ask around my network see if anyone does have strong opinions and why.

@jugmac00
Copy link
Member

Thanks to the now red color for stderr of tox 4, I was made aware of many little issues, warnings, and so on, which were overlooked for years, but could lead to issues, like e.g.
Screenshot from 2023-03-15 14-46-37

I am +1 for keeping the red color, and I am avers to change, especially after the recent-ish tox 4 release. We had years of alpha releases for people to give feedback. Now I am happy that things have settled.

@ssbarnea I think you are aware of mkdocs -q?

Instead of changing the color for all, would it make sense to make the color configurable? Not that I am a fan of too much customization, but that would be a reasonable compromise.

@hashar
Copy link
Contributor

hashar commented Jul 11, 2023

I have the use case of a command emitting its output to STDOUT and logging information to STDERR. So I can do:

tox -e mycommand > output.txt

And still receive in progress messages to STDERR. Due to tox any standard error output ends up colored red when the command emits debug/info etc messages.

The command uses logging so I guess I can pass it a logging configuration to format log entries with a leading color reset ansi code (\033[0m), but that is a bit painful to have to do that for each application having the same behavior.

It would be great to have an optional to disable the forced coloring (and potentially a way to set the color).

@jugmac00
Copy link
Member

jugmac00 commented Jul 11, 2023

@hashar It is usually a good idea to create a new issue or a new discussion to get more feedback - not many read new comments at closed issues.

About disabling coloring - this is already implemented.

NO_COLOR=1 tox -e py311

Also see https://tox.wiki/en/latest/upgrading.html#output-changes

@hashar
Copy link
Contributor

hashar commented Sep 12, 2023

@hashar It is usually a good idea to create a new issue or a new discussion to get more feedback - not many read new comments at closed issues.

I will, and probably come up directly with a pull request when I get the inspiration to write the code :)

About disabling coloring - this is already implemented.

NO_COLOR=1 tox -e py311

This is slightly different since it turns off all tox coloring. I have the use case of not having stderr colored at all (cause some other code is coloring it differently or the output is expected).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:minor does not affect many people or has no big impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants