Add a global option to disable colors by oz123 · Pull Request #4739 · pypa/pip

oz123 · 2017-09-27T08:39:33Z

This is a fix for issue #2449

All it does is simply add a global option --no-color

Internally it switches ColorizedStreamHandler to StreamHandler if
this flag is detected.

This is a fix for issue pypa#2449 All it does is simply add a global option --no-color Internally it switches ColorizedStreamHandler to StreamHandler if this flag is detected.

oz123 · 2017-09-27T08:47:07Z

should have an auto mode which disables coloring when not a tty but also allow for manual override.

As a note for my PR, the above desired behaviour for detecting whether we can output color or not, is an issue of colorama.

This is a simpler fix for cases like:

Same thing when building docker images with PyCharm. The ANSI sequences and deflated output make it hard to spot issues quickly.

Which is almost exactly what the title of the issue says:

pip needs a way to force all colours off

The only difference is the my PR is switching off colors 😄

ghost · 2017-09-28T18:51:27Z

BTW you need to fix the lint errors before the PR will be reviewed.

* not sure it makes the code more readable though ...

oz123 · 2017-09-28T21:21:15Z

@xoviat some tests failed now, but I don't understand why, the exact same tests worked before I linted my code. Can you or some other contributor restart the build with out me pushing code again?

This seems like a temporary name resolution error:

warning: connecting to bitbucket.org using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info

abort: HTTP Error 404: Not Found

pfmoore · 2017-09-28T21:32:55Z

I've restarted the failed ones.

oz123 · 2017-09-29T03:08:00Z

@pfmoore Thanks! now all tests pass! I hope that this PR can be soon reviewed and merged.

src/pip/_internal/basecommand.py

pradyunsg

Looks good barring one minor issue. :)

pradyunsg · 2017-10-07T12:20:52Z

Ping @oz123! :)

pradyunsg · 2017-10-07T12:21:14Z

docs/reference/pip.rst


 pip offers :ref:`-v, --verbose <--verbose>` and :ref:`-q, --quiet <--quiet>`
-to control the console log level.
+to control the console log level. By default, some messages (error and warnnings)


typo: warnings

* As requested per review * Make the code shorter and easier to follow

oz123 · 2017-10-07T22:05:17Z

@pradyunsg, the requested changes have been implemented. Thanks for reminding me ...

src/pip/_internal/cmdoptions.py

oz123 · 2017-10-20T20:02:05Z

@pradyunsg @xavfernandez , can you please review my changes? I would be happy if this is merged.

pradyunsg · 2017-10-21T05:52:32Z

Thanks for the reminder. I'll try to do it in the evening today.

pradyunsg

A test would be nice though I'm not sure how that'd work. I'm fine with the implementation and documentation.

I'm also fine if you just add a comment above no-color saying it's not tested by the test suite.

oz123 · 2017-10-21T12:26:37Z

@pradyunsg ,

I could add a test that is based on the cache feature. Like I showed in the screenshot.
What do you think about that?

pradyunsg · 2017-10-21T17:06:11Z

I think just uninstall based one would be fine too.

pip uninstall not_installed_package would print it with \033 and pip uninstall --no-color not_installed_package would not. That should be a good test.

oz123 · 2017-10-26T20:55:47Z

@pradyunsg, I finally had some time to add a functional test. I hope you would approve it.
Unfortunately, I could not use the existing testing facilities (e.g. test.lib.TestPipResult), but I was able to add a test case using sp.Popen directly.

* The version found in Travis-CI does not respect this flag It added a prefix line and suffix line found if one does not add the flag --quiet (script from util-linux 2.26.2). As such the out put is: Script started on Fri 27 Oct 2017 07:17:30 AM CEST \x1b[31mCannot uninstall requirement noSuchPackage, not installed\x1b[0m\n Script done on Fri 27 Oct 2017 07:17:31 AM CEST With this change, the test should pass, and is hopefully more stable.

oz123 · 2017-11-04T07:53:50Z

@pradyunsg, ping. Any more questions or suggestions for this PR? Can it be merged?

pradyunsg

A quick skim looks good.

I'll look at this again in a little bit. :)

tests/functional/test_no_color.py

pradyunsg · 2017-11-04T09:22:07Z

tests/functional/test_no_color.py

+import pytest
+
+
+@pytest.mark.skipif(sys.platform == 'win32',


A cross-platform test would be nice to have.

@pradyunsg , I would love that too, but it is unfair to ask me to implement this. I have no access to
a windows machine.

No issues then. :)

tests/functional/test_no_color.py

pradyunsg · 2017-11-04T14:38:56Z

That should retrigger CI.

I'll wait for another @pypa/pip-committers to review this. :)

xavfernandez

Even if I'm not a fan of the test with subprocess but I don't see another solution.

pradyunsg · 2017-11-05T09:54:18Z

Thanks for this @oz123! :)

* Add a global option to disable colors This is a fix for issue pypa#2449 All it does is simply add a global option --no-color Internally it switches ColorizedStreamHandler to StreamHandler if this flag is detected. * Fix lint errors * not sure it makes the code more readable though ... * Fix typo * Choose logging class before assigning * As requested per review * Make the code shorter and easier to follow * Be polite to followers, add commas * Add functional test for the --no-color output * Better detection of windows * Fix fragile tests - can't trust script --quiet * The version found in Travis-CI does not respect this flag It added a prefix line and suffix line found if one does not add the flag --quiet (script from util-linux 2.26.2). As such the out put is: Script started on Fri 27 Oct 2017 07:17:30 AM CEST \x1b[31mCannot uninstall requirement noSuchPackage, not installed\x1b[0m\n Script done on Fri 27 Oct 2017 07:17:31 AM CEST With this change, the test should pass, and is hopefully more stable. * Simplify testing for color or no-color

lock · 2019-06-02T16:06:05Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Add a global option to disable colors

5d880cc

This is a fix for issue pypa#2449 All it does is simply add a global option --no-color Internally it switches ColorizedStreamHandler to StreamHandler if this flag is detected.

Fix lint errors

eca7abd

* not sure it makes the code more readable though ...

pradyunsg reviewed Sep 29, 2017

View reviewed changes

src/pip/_internal/basecommand.py Outdated Show resolved Hide resolved

pradyunsg reviewed Sep 29, 2017

View reviewed changes

pradyunsg added type: enhancement Improvements to functionality C: logging Information Logging labels Sep 29, 2017

pradyunsg requested changes Oct 7, 2017

View reviewed changes

Fix typo

bd48245

oz123 force-pushed the no_color branch from 6f77273 to bd48245 Compare October 7, 2017 21:58

Choose logging class before assigning

92c7da2

* As requested per review * Make the code shorter and easier to follow

xavfernandez reviewed Oct 7, 2017

View reviewed changes

src/pip/_internal/cmdoptions.py Outdated Show resolved Hide resolved

src/pip/_internal/cmdoptions.py Outdated Show resolved Hide resolved

Be polite to followers, add commas

0e1f4e1

pradyunsg reviewed Oct 21, 2017

View reviewed changes

pradyunsg added type: feature request Request for a new feature and removed type: enhancement Improvements to functionality labels Oct 24, 2017

Add functional test for the --no-color output

445b612

oz123 added 2 commits October 27, 2017 06:59

Better detection of windows

e550fc7

oz123 force-pushed the no_color branch from 7a59818 to 9cf8db4 Compare October 27, 2017 05:32

pradyunsg reviewed Nov 4, 2017

View reviewed changes

pradyunsg requested changes Nov 4, 2017

View reviewed changes

Simplify testing for color or no-color

bf5933d

oz123 force-pushed the no_color branch from b0466b9 to bf5933d Compare November 4, 2017 10:13

pradyunsg approved these changes Nov 4, 2017

View reviewed changes

pradyunsg closed this Nov 4, 2017

pradyunsg reopened this Nov 4, 2017

xavfernandez approved these changes Nov 4, 2017

View reviewed changes

xavfernandez added this to the 10.0 milestone Nov 4, 2017

pradyunsg merged commit 4370a5a into pypa:master Nov 5, 2017

This was referenced Nov 5, 2017

pip needs a way to force all colours off #2449

Closed

tests: don't check the actual sequence in test #4841

Merged

Add handling of multiple deps and different extras #3878

Closed

pradyunsg mentioned this pull request Nov 16, 2017

Improve --no-color and test for it #4866

Merged

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019

		import pytest


		@pytest.mark.skipif(sys.platform == 'win32',

Conversation

oz123 commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz123 commented Sep 27, 2017

Uh oh!

ghost commented Sep 28, 2017

Uh oh!

oz123 commented Sep 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfmoore commented Sep 28, 2017

Uh oh!

oz123 commented Sep 29, 2017

Uh oh!

Uh oh!

pradyunsg left a comment

Choose a reason for hiding this comment

Uh oh!

pradyunsg commented Oct 7, 2017

Uh oh!

pradyunsg Oct 7, 2017

Choose a reason for hiding this comment

Uh oh!

oz123 commented Oct 7, 2017

Uh oh!

Uh oh!

Uh oh!

oz123 commented Oct 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pradyunsg commented Oct 21, 2017

Uh oh!

pradyunsg left a comment

Choose a reason for hiding this comment

Uh oh!

oz123 commented Oct 21, 2017

Uh oh!

pradyunsg commented Oct 21, 2017

Uh oh!

oz123 commented Oct 26, 2017

Uh oh!

oz123 commented Nov 4, 2017

Uh oh!

pradyunsg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pradyunsg Nov 4, 2017

Choose a reason for hiding this comment

Uh oh!

oz123 Nov 4, 2017

Choose a reason for hiding this comment

Uh oh!

pradyunsg Nov 4, 2017

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pradyunsg commented Nov 4, 2017

Uh oh!

xavfernandez left a comment

Choose a reason for hiding this comment

Uh oh!

pradyunsg commented Nov 5, 2017

Uh oh!

lock bot commented Jun 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oz123 commented Sep 27, 2017 •

edited

Loading

oz123 commented Sep 28, 2017 •

edited

Loading

oz123 commented Oct 20, 2017 •

edited

Loading