[2254] Expand _complete comment, add one more testcase

also renamed IDENTIFIER_PARAM to CFGITEM_IDENTIFIER_PARAM

src/bin/bindctl/bindcmd.py

@@ -61,7 +61,7 @@ except ImportError:
 # Used for tab-completion of 'identifiers' (i.e. config values)
 # If a command parameter has this name, the tab completion hints
 # are derived from config data
-IDENTIFIER_PARAM = 'identifier'
+CFGITEM_IDENTIFIER_PARAM = 'identifier'
 
 CSV_FILE_NAME = 'default_user.csv'
 CONFIG_MODULE_NAME = 'config'
@@ -495,7 +495,7 @@ class BindCmdInterpreter(Cmd):
         if cmd.module not in self.modules:
             return False
         command = self.modules[cmd.module].get_command_with_name(cmd.command)
-        return command.has_param_with_name(IDENTIFIER_PARAM)
+        return command.has_param_with_name(CFGITEM_IDENTIFIER_PARAM)
 
     def complete(self, text, state):
         """
@@ -538,8 +538,17 @@ class BindCmdInterpreter(Cmd):
                 if not cmd.params and text:
                     hints = self._get_command_startswith(cmd.module, text)
                 elif self._cmd_has_identifier_param(cmd):
-                    # For tab-completion of identifiers, replace hardcoded
-                    # hints with hints derived from the config data
+                    # If the command has an argument that is a configuration
+                    # identifier (currently, this is only a subset of
+                    # the config commands), then don't tab-complete with
+                    # hints derived from command parameters, but from
+                    # possible configuration identifiers.
+                    #
+                    # This solves the issue reported in #2254, where
+                    # there were hints such as 'argument' and 'identifier'.
+                    #
+                    # Since they are replaced, the tab-completion no longer
+                    # adds 'help' as an option (but it still works)
                     hints = self._get_identifier_startswith(text)
                 else:
                     hints = self._get_param_startswith(cmd.module, cmd.command,

src/bin/bindctl/bindctl_main.py.in

@@ -42,15 +42,15 @@ def prepare_config_commands(tool):
     cmd = CommandInfo(name = "show", desc = "Show configuration.")
     param = ParamInfo(name = "argument", type = "string", optional=True, desc = "If you specify the argument 'all' (before the identifier), recursively show all child elements for the given identifier.")
     cmd.add_param(param)
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=True,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=True, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     module.add_command(cmd)
 
     cmd = CommandInfo(name="show_json",
                       desc="Show full configuration in JSON format.")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=True,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=True, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     module.add_command(cmd)
 
@@ -63,8 +63,8 @@ def prepare_config_commands(tool):
         "parameter value, similar to when adding to a list. "
         "In either case, when no value is given, an entry will be "
         "constructed with default values.")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=True,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=True, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     param = ParamInfo(name="value_or_name", type="string", optional=True,
                       desc="Specifies a value to add to the list, or the name when adding to a named set. It must be in correct JSON format and complete.")
@@ -76,16 +76,16 @@ def prepare_config_commands(tool):
     module.add_command(cmd)
 
     cmd = CommandInfo(name="remove", desc="Remove entry from configuration list or named set.")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=True,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=True, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     param = ParamInfo(name = "value", type = "string", optional=True, desc = "When identifier is a list, specifies a value to remove from the list. It must be in correct JSON format and complete. When it is a named set, specifies the name to remove.")
     cmd.add_param(param)
     module.add_command(cmd)
 
     cmd = CommandInfo(name="set", desc="Set a configuration value.")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=True,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=True, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     param = ParamInfo(name="value", type="string", optional=False,
                       desc="Specifies a value to set. It must be in correct JSON format and complete.")
@@ -93,8 +93,8 @@ def prepare_config_commands(tool):
     module.add_command(cmd)
 
     cmd = CommandInfo(name="unset", desc="Unset a configuration value (i.e. revert to the default, if any).")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=False,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=False, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     module.add_command(cmd)
 
@@ -108,8 +108,8 @@ def prepare_config_commands(tool):
     module.add_command(cmd)
 
     cmd = CommandInfo(name="go", desc="Go to a specific configuration part.")
-    param = ParamInfo(name=IDENTIFIER_PARAM, type="string", optional=False,
-                      desc=DEFAULT_IDENTIFIER_DESC)
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=False, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     module.add_command(cmd)
 

src/bin/bindctl/tests/bindctl_test.py

@@ -456,7 +456,7 @@ class TestConfigCommands(unittest.TestCase):
         module = ModuleInfo(name="test_module")
 
         cmd = CommandInfo(name="command_with_identifier")
-        param = ParamInfo(name=bindcmd.IDENTIFIER_PARAM)
+        param = ParamInfo(name=bindcmd.CFGITEM_IDENTIFIER_PARAM)
         cmd.add_param(param)
         module.add_command(cmd)
 

src/lib/config/tests/testdata/spec42.spec

@@ -0,0 +1,17 @@
+{
+  "module_spec": {
+    "module_name": "Spec42",
+    "config_data": [
+      { "item_name": "list_item",
+        "item_type": "list",
+        "item_optional": true,
+        "list_item_spec": {
+          "item_name": "list_element",
+          "item_type": "string",
+          "item_optional": false,
+          "item_default": ""
+        }
+      }
+    ]
+  }
+}

src/lib/python/isc/config/config_data.py

@@ -841,9 +841,6 @@ class MultiConfigData:
            _get_list_items("Module/list")
                where the list contains 2 elements, returns
                [ "Module/list[0]", "Module/list[1]" ]
-           _get_list_items("Module/list")
-               where the list contains 2 elements, returns
-               [ "Module/list[0]", "Module/list[1]" ]
         """
         spec_part = self.find_spec_part(item_name)
         if spec_part_is_named_set(spec_part):

src/lib/python/isc/config/tests/config_data_test.py

@@ -798,6 +798,14 @@ class TestMultiConfigData(unittest.TestCase):
                           'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1',
                           'Spec2/item6/value2'], config_items)
 
+        # Also if the list is None (optional value and no default)
+        module_spec = isc.config.module_spec_from_file(self.data_path
+                                                       + os.sep
+                                                       + "spec42.spec")
+        self.mcd.set_specification(module_spec)
+        config_items = self.mcd.get_config_item_list("Spec42", True)
+        self.assertEqual(['Spec42/list_item'], config_items)
+
     def test_is_named_set(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         self.mcd.set_specification(module_spec)