From 7e580d30bfeacbb171f99ea0e85e6aab5e697dcd Mon Sep 17 00:00:00 2001
From: "ChangBo Guo(gcb)" <eric.guo@easystack.cn>
Date: Thu, 26 Oct 2017 18:23:46 +0800
Subject: [PATCH] Make help message include choices information for StrOpt

Config option `StrOpt` accept choices parameter to indicate
valid values and include comments "# Allowed values:" when generating
sample config file, so it's unnecessary to add allowed values in hep
string of the config option. But it doesn't include choices information
when printing help for the config option. This commit makes consistency
for both of sample config file and help output of command line.

Closes-Bug:1727683

Change-Id: I962e49c81bdf44043a6e1233408a6ee5b883acde
---
 oslo_config/cfg.py            | 21 +++++++++++++++++++++
 oslo_config/tests/test_cfg.py | 26 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py
index 8f97af20..1c003ea5 100644
--- a/oslo_config/cfg.py
+++ b/oslo_config/cfg.py
@@ -1261,6 +1261,27 @@ class StrOpt(Opt):
                                          max_length=max_length),
                                      **kwargs)
 
+    def _get_choice_text(self, choice):
+        if choice is None:
+            return '<None>'
+        elif choice == '':
+            return "''"
+        return six.text_type(choice)
+
+    def _get_argparse_kwargs(self, group, **kwargs):
+        """Extends the base argparse keyword dict for the config dir option."""
+        kwargs = super(StrOpt, self)._get_argparse_kwargs(group)
+
+        if getattr(self.type, 'choices', None):
+            choices_text = ', '.join([self._get_choice_text(choice)
+                                      for choice in self.type.choices])
+            if kwargs['help'] is not None:
+                kwargs['help'] += (' Allowed values: %s\n' % choices_text)
+            else:
+                kwargs['help'] = (' Allowed values: %s\n' % choices_text)
+
+        return kwargs
+
 
 class BoolOpt(Opt):
 
diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
index 4e236d04..a23e6b3e 100644
--- a/oslo_config/tests/test_cfg.py
+++ b/oslo_config/tests/test_cfg.py
@@ -167,6 +167,32 @@ class HelpTestCase(BaseTestCase):
         self.assertIn('optional', f.getvalue())
         self.assertIn('-h, --help', f.getvalue())
 
+    def test_print_strOpt_with_choices_help(self):
+        f = moves.StringIO()
+        cli_opts = [
+            cfg.StrOpt('aa', short='a', default='xx',
+                       choices=['xx', 'yy', 'zz'],
+                       help='StrOpt with choices.'),
+            cfg.StrOpt('bb', short='b', default='yy',
+                       choices=[None, 'yy', 'zz'],
+                       help='StrOpt with choices.'),
+            cfg.StrOpt('cc', short='c', default='zz',
+                       choices=['', 'yy', 'zz'],
+                       help='StrOpt with choices.'),
+        ]
+        self.conf.register_cli_opts(cli_opts)
+        self.conf([])
+        self.conf.print_help(file=f)
+        self.assertIn('usage: test FOO BAR', f.getvalue())
+        self.assertIn('optional', f.getvalue())
+        self.assertIn('-h, --help', f.getvalue())
+        self.assertIn('StrOpt with choices. Allowed values: xx, yy, zz',
+                      f.getvalue())
+        self.assertIn('StrOpt with choices. Allowed values: <None>, yy, zz',
+                      f.getvalue())
+        self.assertIn("StrOpt with choices. Allowed values: '', yy, zz",
+                      f.getvalue())
+
     def test_print_sorted_help(self):
         f = moves.StringIO()
         self.conf.register_cli_opt(cfg.StrOpt('abc'))