Merge pull request #8578 from McSinyx/parse-count

Allow verbose/quiet level to be specified via config file and env var

docs/html/user_guide.rst

@@ -458,6 +458,15 @@ and ``--no-cache-dir``, falsy values have to be used:
     no-compile = no
     no-warn-script-location = false
 
+For options which can be repeated like ``--verbose`` and ``--quiet``,
+a non-negative integer can be used to represent the level to be specified:
+
+.. code-block:: ini
+
+    [global]
+    quiet = 0
+    verbose = 2
+
 It is possible to append values to a section within a configuration file such as the pip.ini file.
 This is applicable to appending options like ``--find-links`` or ``--trusted-host``,
 which can be written on multiple lines:
@@ -504,6 +513,15 @@ is the same as calling::
 
     pip install --find-links=http://mirror1.example.com --find-links=http://mirror2.example.com
 
+Options that do not take a value, but can be repeated (such as ``--verbose``)
+can be specified using the number of repetitions, so::
+
+    export PIP_VERBOSE=3
+
+is the same as calling::
+
+    pip install -vvv
+
 .. note::
 
    Environment variables set to be empty string will not be treated as false.

news/8578.bugfix

@@ -0,0 +1,4 @@
+Allow specifying verbosity and quiet level via configuration files
+and environment variables. Previously these options were treated as
+boolean values when read from there while through CLI the level can be
+specified.

src/pip/_internal/cli/parser.py

@@ -11,6 +11,7 @@
 import textwrap
 from distutils.util import strtobool
 
+from pip._vendor.contextlib2 import suppress
 from pip._vendor.six import string_types
 
 from pip._internal.cli.status_codes import UNKNOWN_ERROR
@@ -197,15 +198,27 @@ def _update_defaults(self, defaults):
             if option is None:
                 continue
 
-            if option.action in ('store_true', 'store_false', 'count'):
+            if option.action in ('store_true', 'store_false'):
                 try:
                     val = strtobool(val)
                 except ValueError:
-                    error_msg = invalid_config_error_message(
-                        option.action, key, val
+                    self.error(
+                        '{} is not a valid value for {} option, '  # noqa
+                        'please specify a boolean value like yes/no, '
+                        'true/false or 1/0 instead.'.format(val, key)
+                    )
+            elif option.action == 'count':
+                with suppress(ValueError):
+                    val = strtobool(val)
+                with suppress(ValueError):
+                    val = int(val)
+                if not isinstance(val, int) or val < 0:
+                    self.error(
+                        '{} is not a valid value for {} option, '  # noqa
+                        'please instead specify either a non-negative integer '
+                        'or a boolean value like yes/no or false/true '
+                        'which is equivalent to 1/0.'.format(val, key)
                     )
-                    self.error(error_msg)
-
             elif option.action == 'append':
                 val = val.split()
                 val = [self.check_default(option, key, v) for v in val]
@@ -251,16 +264,3 @@ def get_default_values(self):
     def error(self, msg):
         self.print_usage(sys.stderr)
         self.exit(UNKNOWN_ERROR, "{}\n".format(msg))
-
-
-def invalid_config_error_message(action, key, val):
-    """Returns a better error message when invalid configuration option
-    is provided."""
-    if action in ('store_true', 'store_false'):
-        return ("{0} is not a valid value for {1} option, "
-                "please specify a boolean value like yes/no, "
-                "true/false or 1/0 instead.").format(val, key)
-
-    return ("{0} is not a valid value for {1} option, "
-            "please specify a numerical value like 1/0 "
-            "instead.").format(val, key)

tests/unit/test_options.py

@@ -1,5 +1,6 @@
 import os
 from contextlib import contextmanager
+from tempfile import NamedTemporaryFile
 
 import pytest
 
@@ -286,6 +287,107 @@ def test_subcommand_option_before_subcommand_fails(self):
             main(['--find-links', 'F1', 'fake'])
 
 
+@contextmanager
+def tmpconfig(option, value, section='global'):
+    with NamedTemporaryFile(mode='w', delete=False) as f:
+        f.write('[{}]\n{}={}\n'.format(section, option, value))
+        name = f.name
+    try:
+        yield name
+    finally:
+        os.unlink(name)
+
+
+class TestCountOptions(AddFakeCommandMixin):
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(4))
+    def test_cli_long(self, option, value):
+        flags = ['--{}'.format(option)] * value
+        opt1, args1 = main(flags+['fake'])
+        opt2, args2 = main(['fake']+flags)
+        assert getattr(opt1, option) == getattr(opt2, option) == value
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(1, 4))
+    def test_cli_short(self, option, value):
+        flag = '-' + option[0]*value
+        opt1, args1 = main([flag, 'fake'])
+        opt2, args2 = main(['fake', flag])
+        assert getattr(opt1, option) == getattr(opt2, option) == value
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(4))
+    def test_env_var(self, option, value, monkeypatch):
+        monkeypatch.setenv('PIP_'+option.upper(), str(value))
+        assert getattr(main(['fake'])[0], option) == value
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(3))
+    def test_env_var_integrate_cli(self, option, value, monkeypatch):
+        monkeypatch.setenv('PIP_'+option.upper(), str(value))
+        assert getattr(main(['fake', '--'+option])[0], option) == value + 1
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', (-1, 'foobar'))
+    def test_env_var_invalid(self, option, value, monkeypatch, capsys):
+        monkeypatch.setenv('PIP_'+option.upper(), str(value))
+        with assert_option_error(capsys, expected='a non-negative integer'):
+            main(['fake'])
+
+    # Undocumented, support for backward compatibility
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', ('no', 'false'))
+    def test_env_var_false(self, option, value, monkeypatch):
+        monkeypatch.setenv('PIP_'+option.upper(), str(value))
+        assert getattr(main(['fake'])[0], option) == 0
+
+    # Undocumented, support for backward compatibility
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', ('yes', 'true'))
+    def test_env_var_true(self, option, value, monkeypatch):
+        monkeypatch.setenv('PIP_'+option.upper(), str(value))
+        assert getattr(main(['fake'])[0], option) == 1
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(4))
+    def test_config_file(self, option, value, monkeypatch):
+        with tmpconfig(option, value) as name:
+            monkeypatch.setenv('PIP_CONFIG_FILE', name)
+            assert getattr(main(['fake'])[0], option) == value
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', range(3))
+    def test_config_file_integrate_cli(self, option, value, monkeypatch):
+        with tmpconfig(option, value) as name:
+            monkeypatch.setenv('PIP_CONFIG_FILE', name)
+            assert getattr(main(['fake', '--'+option])[0], option) == value + 1
+
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', (-1, 'foobar'))
+    def test_config_file_invalid(self, option, value, monkeypatch, capsys):
+        with tmpconfig(option, value) as name:
+            monkeypatch.setenv('PIP_CONFIG_FILE', name)
+            with assert_option_error(capsys, expected='non-negative integer'):
+                main(['fake'])
+
+    # Undocumented, support for backward compatibility
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', ('no', 'false'))
+    def test_config_file_false(self, option, value, monkeypatch):
+        with tmpconfig(option, value) as name:
+            monkeypatch.setenv('PIP_CONFIG_FILE', name)
+            assert getattr(main(['fake'])[0], option) == 0
+
+    # Undocumented, support for backward compatibility
+    @pytest.mark.parametrize('option', ('verbose', 'quiet'))
+    @pytest.mark.parametrize('value', ('yes', 'true'))
+    def test_config_file_true(self, option, value, monkeypatch):
+        with tmpconfig(option, value) as name:
+            monkeypatch.setenv('PIP_CONFIG_FILE', name)
+            assert getattr(main(['fake'])[0], option) == 1
+
+
 class TestGeneralOptions(AddFakeCommandMixin):
 
     # the reason to specifically test general options is due to the
@@ -310,24 +412,6 @@ def test_require_virtualenv(self):
         assert options1.require_venv
         assert options2.require_venv
 
-    def test_verbose(self):
-        options1, args1 = main(['--verbose', 'fake'])
-        options2, args2 = main(['fake', '--verbose'])
-        assert options1.verbose == options2.verbose == 1
-
-    def test_quiet(self):
-        options1, args1 = main(['--quiet', 'fake'])
-        options2, args2 = main(['fake', '--quiet'])
-        assert options1.quiet == options2.quiet == 1
-
-        options3, args3 = main(['--quiet', '--quiet', 'fake'])
-        options4, args4 = main(['fake', '--quiet', '--quiet'])
-        assert options3.quiet == options4.quiet == 2
-
-        options5, args5 = main(['--quiet', '--quiet', '--quiet', 'fake'])
-        options6, args6 = main(['fake', '--quiet', '--quiet', '--quiet'])
-        assert options5.quiet == options6.quiet == 3
-
     def test_log(self):
         options1, args1 = main(['--log', 'path', 'fake'])
         options2, args2 = main(['fake', '--log', 'path'])