From 9f866b35a16e01cea06deb73006fa2e55f30ea51 Mon Sep 17 00:00:00 2001
From: Doug Hellmann <doug@doughellmann.com>
Date: Wed, 10 Oct 2018 12:48:37 -0400
Subject: [PATCH] only override config values from command line if they are
 actually set

If we set defaults for the query configuration options and then pass
the namespace created by parsing the command line options to the
config object it cannot tell the difference between values set on the
command line and defaults. That means we may override settings in the
configuration file with defaults from the command line parser.

This change sets all of the defaults for command line options to None
as a sentinel value and then updates the Config class to ignore
command line option values of None. It also adds tests to verify that
we get the expected override behavior with boolean and string options.

Change-Id: I1c9ce668b5e5c372d1c861bcae6e6de05a8ebc0c
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
---
 ...-cli-option-handling-a13652d14507f2d7.yaml |  7 +++++
 reno/config.py                                |  6 +++--
 reno/main.py                                  |  6 ++---
 reno/tests/test_config.py                     | 26 ++++++++++++++++++-
 4 files changed, 39 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/fix-cli-option-handling-a13652d14507f2d7.yaml

diff --git a/releasenotes/notes/fix-cli-option-handling-a13652d14507f2d7.yaml b/releasenotes/notes/fix-cli-option-handling-a13652d14507f2d7.yaml
new file mode 100644
index 0000000..8c84efe
--- /dev/null
+++ b/releasenotes/notes/fix-cli-option-handling-a13652d14507f2d7.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fix an issue with the way command line options and configuration
+    settings interact so that the settings in the configuration file
+    are used properly when command line arguments for those options
+    are not provided.
diff --git a/reno/config.py b/reno/config.py
index 3aa6583..5990e25 100644
--- a/reno/config.py
+++ b/reno/config.py
@@ -275,9 +275,11 @@ class Config(object):
         arg_values = {
             o.name: getattr(parsed_args, o.name)
             for o in _OPTIONS
-            if hasattr(parsed_args, o.name)
+            if getattr(parsed_args, o.name, None) is not None
         }
-        self.override(**arg_values)
+        if arg_values:
+            LOG.info('[config] updating from command line options')
+            self.override(**arg_values)
 
     @property
     def reporoot(self):
diff --git a/reno/main.py b/reno/main.py
index b9837d7..cb27712 100644
--- a/reno/main.py
+++ b/reno/main.py
@@ -32,7 +32,7 @@ _query_args = [
           help='the branch to scan, defaults to the current')),
     (('--collapse-pre-releases',),
      dict(action='store_true',
-          default=config.Config.get_default('collapse_pre_releases'),
+          default=None,
           help='combine pre-releases with their final release')),
     (('--no-collapse-pre-releases',),
      dict(action='store_false',
@@ -42,12 +42,12 @@ _query_args = [
      dict(default=None,
           help='stop when this version is reached in the history')),
     (('--ignore-cache',),
-     dict(default=False,
+     dict(default=None,
           action='store_true',
           help='if there is a cache file present, do not use it')),
     (('--stop-at-branch-base',),
      dict(action='store_true',
-          default=True,
+          default=None,
           dest='stop_at_branch_base',
           help='stop scanning when the branch meets master')),
     (('--no-stop-at-branch-base',),
diff --git a/reno/tests/test_config.py b/reno/tests/test_config.py
index 6251aa7..38e9635 100644
--- a/reno/tests/test_config.py
+++ b/reno/tests/test_config.py
@@ -126,7 +126,7 @@ collapse_pre_releases: false
         }
         self.assertEqual(expected, actual)
 
-    def test_override_from_parsed_args(self):
+    def test_override_from_parsed_args_boolean_false(self):
         c = self._run_override_from_parsed_args([
             '--no-collapse-pre-releases',
         ])
@@ -138,6 +138,30 @@ collapse_pre_releases: false
         expected['collapse_pre_releases'] = False
         self.assertEqual(expected, actual)
 
+    def test_override_from_parsed_args_boolean_true(self):
+        c = self._run_override_from_parsed_args([
+            '--collapse-pre-releases',
+        ])
+        actual = c.options
+        expected = {
+            o.name: o.default
+            for o in config._OPTIONS
+        }
+        expected['collapse_pre_releases'] = True
+        self.assertEqual(expected, actual)
+
+    def test_override_from_parsed_args_string(self):
+        c = self._run_override_from_parsed_args([
+            '--earliest-version', '1.2.3',
+        ])
+        actual = c.options
+        expected = {
+            o.name: o.default
+            for o in config._OPTIONS
+        }
+        expected['earliest_version'] = '1.2.3'
+        self.assertEqual(expected, actual)
+
     def test_override_from_parsed_args_ignore_non_options(self):
         parser = argparse.ArgumentParser()
         main._build_query_arg_group(parser)