From d5f02d61cb274646e8235e80c941a633eef312e7 Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Wed, 12 Dec 2018 16:09:40 -0800 Subject: [PATCH] Fix wrong and misleading "using default: None" in --verbose mode When something is not found in git config, the --verbose option logs the wrong value "None", example: 2018-12-12 Running: git config --get gitreview.branch 2018-12-12 using default: None 2018-12-12 Running: git config --get gitreview.scheme 2018-12-12 using default: None 2018-12-12 Running: git config --get remote.gerrit.pushurl 2018-12-12 using default: None 2018-12-12 Running: git config --get remote.gerrit.url 2018-12-12 result: ssh://marc@review.openstack.org:29418/openstack-infra/git-review From a --verbose user perspective this is plain wrong for (at least) all the options defined in the DEFAULTS list. Change --verbose message to look like this instead: 2018-12-12 Running: git config --get gitreview.branch 2018-12-12 Config['branch'] = master 2018-12-12 Running: git config --get gitreview.scheme 2018-12-12 Config['scheme'] = ssh 2018-12-12 Running: git config --get remote.gerrit.pushurl 2018-12-12 Running: git config --get remote.gerrit.url 2018-12-12 ... remote.gerrit.url = ssh://marc@review.openstack.org:29418/openstack-infra/git-review If git_config_get_value('new_option',... ) is ever invoked in the future with a not None, default="fubar" parameter then --verbose will print these lines: 2018-12-19 Running: git config --get gitreview.new_option 2018-12-19 ... nothing in git config, returning func parameter: fubar This logging issue is especially misleading considering the many levels of defaults and fallbacks: git config x3; .gitreview; DEFAULTS list, git_config_get_value(default=...) parameter, options parser logic, etc. Change-Id: I6cee46e88b90b8f11689be3875d64ec5e577f12f --- git_review/cmd.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/git_review/cmd.py b/git_review/cmd.py index f36eec25..b147c77a 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -261,12 +261,15 @@ def git_config_get_value(section, option, default=None, as_bool=False): try: result = run_command_exc(GitConfigException, *cmd).strip() if VERBOSE: - print(datetime.datetime.now(), "result:", result) + print(datetime.datetime.now(), "... %s.%s = %s" + % (section, option, result)) return result except GitConfigException as exc: if exc.rc == 1: - if VERBOSE: - print(datetime.datetime.now(), "using default:", default) + if VERBOSE and default is not None: + print(datetime.datetime.now(), + "... nothing in git config, returning func parameter:", + default) return default raise @@ -289,9 +292,16 @@ class Config(object): self.config.update(load_config_file(filename)) def __getitem__(self, key): + """Let 'git config --get' override every Config['key'] access""" value = git_config_get_value('gitreview', key) if value is None: value = self.config[key] + # "--verbose" doesn't trace *early* invocations; for that you + # must change the value at the top of this file (*and* pass + # --verbose) + if VERBOSE: + print(datetime.datetime.now(), + "Config['%s'] = %s " % (key, value)) return value