[master] Merge branch 'trac2254'

src/bin/bindctl/bindcmd.py

@@ -22,7 +22,7 @@ import sys
 from cmd import Cmd
 from bindctl.exception import *
 from bindctl.moduleinfo import *
-from bindctl.cmdparse import BindCmdParse
+from bindctl.cmdparse import BindCmdParser
 from bindctl import command_sets
 from xml.dom import minidom
 import isc
@@ -48,20 +48,21 @@ except ImportError:
 # if we have readline support, use that, otherwise use normal stdio
 try:
     import readline
-    # This is a fix for the problem described in
-    # http://bind10.isc.org/ticket/1345
-    # If '-' is seen as a word-boundary, the final completion-step
-    # (as handled by the cmd module, and hence outside our reach) can
-    # mistakenly add data twice, resulting in wrong completion results
-    # The solution is to remove it.
-    delims = readline.get_completer_delims()
-    delims = delims.replace('-', '')
-    readline.set_completer_delims(delims)
+    # Only consider spaces as word boundaries; identifiers can contain
+    # '/' and '[]', and configuration item names can in theory use any
+    # printable  character. See the discussion in tickets #1345 and
+    # #2254 for more information.
+    readline.set_completer_delims(' ')
 
     my_readline = readline.get_line_buffer
 except ImportError:
     my_readline = sys.stdin.readline
 
+# 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
+CFGITEM_IDENTIFIER_PARAM = 'identifier'
+
 CSV_FILE_NAME = 'default_user.csv'
 CONFIG_MODULE_NAME = 'config'
 CONST_BINDCTL_HELP = """
@@ -463,41 +464,101 @@ class BindCmdInterpreter(Cmd):
 
         Cmd.onecmd(self, line)
 
-    def remove_prefix(self, list, prefix):
-        """Removes the prefix already entered, and all elements from the
-           list that don't match it"""
-        if prefix.startswith('/'):
-            prefix = prefix[1:]
-
-        new_list = []
-        for val in list:
-            if val.startswith(prefix):
-                new_val = val[len(prefix):]
-                if new_val.startswith("/"):
-                    new_val = new_val[1:]
-                new_list.append(new_val)
-        return new_list
+    def _get_identifier_startswith(self, id_text):
+        """Return the tab-completion hints for identifiers starting with
+           id_text.
+
+           Parameters:
+           id_text (string): the currently entered identifier part, which
+           is to be completed.
+        """
+        # Strip starting "/" from id_text
+        if id_text.startswith('/'):
+            id_text = id_text[1:]
+        # Get all items from the given module (up to the first /)
+        list = self.config_data.get_config_item_list(
+                        id_text.rpartition("/")[0], recurse=True)
+        # filter out all possibilities that don't match currently entered
+        # text part
+        hints = [val for val in list if val.startswith(id_text)]
+        return hints
+
+    def _cmd_has_identifier_param(self, cmd):
+        """
+        Returns True if the given (parsed) command is known and has a
+        parameter which points to a config data identifier
+
+        Parameters:
+        cmd (cmdparse.BindCmdParser): command context, including given params
+
+        """
+        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(CFGITEM_IDENTIFIER_PARAM)
 
     def complete(self, text, state):
-        if 0 == state:
+        """
+        Returns tab-completion hints. See the python documentation of the
+        readline and Cmd modules for more information.
+
+        The first time this is called (within one 'completer' action), it
+        has state 0, and a list of possible completions is made. This list
+        is stored; complete() will then be called with increasing values of
+        state, until it returns None. For each call it returns the state'th
+        element of the hints it collected in the first call.
+
+        The hints list contents depend on which part of the full command
+        line; if no module is given yet, it will list all modules. If a
+        module is given, but no command, it will complete with module
+        commands. If both have been given, it will create the hints based on
+        the command parameters.
+
+        If module and command have already been specified, and the command
+        has a parameter 'identifier', the configuration data is used to
+        create the hints list.
+
+        Parameters:
+        text (string): The text entered so far in the 'current' part of
+                       the command (module, command, parameters)
+        state (int): state used in the readline tab-completion logic;
+                     0 on first call, increasing by one until there are
+                     no (more) hints to return.
+
+        Returns the string value of the hints list with index 'state',
+        or None if no (more) hints are available.
+        """
+        if state == 0:
             self._update_all_modules_info()
             text = text.strip()
             hints = []
             cur_line = my_readline()
             try:
-                cmd = BindCmdParse(cur_line)
+                cmd = BindCmdParser(cur_line)
                 if not cmd.params and text:
                     hints = self._get_command_startswith(cmd.module, text)
+                elif self._cmd_has_identifier_param(cmd):
+                    # 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)
+                    #
+                    # Also, currently, tab-completion does not work
+                    # together with 'config go' (it does not take 'current
+                    # position' into account). But config go currently has
+                    # problems by itself, unrelated to completion.
+                    hints = self._get_identifier_startswith(text)
                 else:
                     hints = self._get_param_startswith(cmd.module, cmd.command,
                                                        text)
-                    if cmd.module == CONFIG_MODULE_NAME:
-                        # grm text has been stripped of slashes...
-                        my_text = self.location + "/" + cur_line.rpartition(" ")[2]
-                        list = self.config_data.get_config_item_list(my_text.rpartition("/")[0], True)
-                        hints.extend([val for val in list if val.startswith(my_text[1:])])
-                        # remove the common prefix from the hints so we don't get it twice
-                        hints = self.remove_prefix(hints, my_text.rpartition("/")[0])
+
             except CmdModuleNameFormatError:
                 if not text:
                     hints = self.get_module_names()
@@ -562,7 +623,7 @@ class BindCmdInterpreter(Cmd):
 
     def _parse_cmd(self, line):
         try:
-            cmd = BindCmdParse(line)
+            cmd = BindCmdParser(line)
             self._validate_cmd(cmd)
             self._handle_cmd(cmd)
         except (IOError, http.client.HTTPException) as err:
@@ -794,7 +855,7 @@ class BindCmdInterpreter(Cmd):
                     else:
                         print("Warning: ignoring unknown directive: " + line)
                 else:
-                    cmd = BindCmdParse(line)
+                    cmd = BindCmdParser(line)
                     self._validate_cmd(cmd)
                     self._handle_cmd(cmd)
         except (isc.config.ModuleCCSessionError,

src/bin/bindctl/bindctl_main.py.in

@@ -42,16 +42,19 @@ 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", 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", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd = CommandInfo(name="show_json",
+                      desc="Show full configuration in JSON format.")
+    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 = "add", desc =
+    cmd = CommandInfo(name="add", desc=
         "Add an entry to configuration list or a named set. "
         "When adding to a list, the command has one optional argument, "
         "a value to add to the list. The value must be in correct JSON "
@@ -60,45 +63,53 @@ 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", 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.")
+    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.")
     cmd.add_param(param)
     module.add_command(cmd)
-    param = ParamInfo(name = "value_for_set", type = "string", optional=True, desc = "Specifies an optional value to add to the named map. It must be in correct JSON format and complete.")
+    param = ParamInfo(name="value_for_set", type="string", optional=True,
+                      desc="Specifies an optional value to add to the named map. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list or named set.")
-    param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd = CommandInfo(name="remove", desc="Remove entry from configuration list or named set.")
+    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", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd = CommandInfo(name="set", desc="Set a configuration value.")
+    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.")
+    param = ParamInfo(name="value", type="string", optional=False,
+                      desc="Specifies a value to set. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     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", type = "string", optional=False, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd = CommandInfo(name="unset", desc="Unset a configuration value (i.e. revert to the default, if any).")
+    param = ParamInfo(name=CFGITEM_IDENTIFIER_PARAM, type="string",
+                      optional=False, desc=DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "diff", desc = "Show all local changes that have not been committed.")
+    cmd = CommandInfo(name="diff", desc="Show all local changes that have not been committed.")
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "revert", desc = "Revert all local changes.")
+    cmd = CommandInfo(name="revert", desc="Revert all local changes.")
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "commit", desc = "Commit all local changes.")
+    cmd = CommandInfo(name="commit", desc="Commit all local changes.")
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "go", desc = "Go to a specific configuration part.")
-    param = ParamInfo(name = "identifier", type="string", optional=False, desc = DEFAULT_IDENTIFIER_DESC)
+    cmd = CommandInfo(name="go", desc="Go to a specific configuration part.")
+    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/cmdparse.py

@@ -25,7 +25,7 @@ except ImportError:
 
 param_name_str = "^\s*(?P<param_name>[\w]+)\s*=\s*"
 
-# The value string can be a sequence without space or comma 
+# The value string can be a sequence without space or comma
 # characters, or a string surroundedby quotation marks(such marks
 # can be part of string in an escaped form)
 #param_value_str  = "(?P<param_value>[\"\'].+?(?<!\\\)[\"\']|[^\'\"][^, ]+)"
@@ -34,8 +34,8 @@ param_value_with_quota_str  = "[\"\'](?P<param_value>.+?)(?<!\\\)[\"\']"
 next_params_str = "(?P<blank>\s*)(?P<comma>,?)(?P<next_params>.*)$"
 
 
-PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str + 
-                                      param_value_with_quota_str + 
+PARAM_WITH_QUOTA_PATTERN = re.compile(param_name_str +
+                                      param_value_with_quota_str +
                                       next_params_str)
 PARAM_PATTERN = re.compile(param_name_str + param_value_str + next_params_str)
 # Used for module and command name
@@ -83,52 +83,52 @@ def _remove_list_and_map_whitespace(text):
                 if map_count == 0:
                     result.append(_remove_unquoted_whitespace(text[cur_start_map_pos:pos + 1]))
                     start_pos = pos + 1
-        
+
 
         pos = pos + 1
     if start_pos <= len(text):
         result.append(text[start_pos:len(text)])
     return "".join(result)
-    
-    
-class BindCmdParse:
+
+
+class BindCmdParser:
     """ This class will parse the command line user input into three parts:
     module name, command, parameters
-    the first two parts are strings and parameter is one hash, 
+    the first two parts are strings and parameter is one hash,
     parameters part is optional
-    
-    Example: zone reload, zone_name=example.com 
+
+    Example: zone reload, zone_name=example.com
     module == zone
     command == reload
     params == [zone_name = 'example.com']
     """
-    
+
     def __init__(self, cmd):
         self.params = OrderedDict()
         self.module = ''
         self.command = ''
         self._parse_cmd(cmd)
 
-    def _parse_cmd(self, text_str):    
+    def _parse_cmd(self, text_str):
         '''Parse command line. '''
         # Get module name
         groups = NAME_PATTERN.match(text_str)
         if not groups:
             raise CmdModuleNameFormatError
-        
+
         self.module = groups.group('name')
         cmd_str = groups.group('others')
         if cmd_str:
             if not groups.group('blank'):
                 raise CmdModuleNameFormatError
-        else:            
+        else:
             raise CmdMissCommandNameFormatError(self.module)
-            
+
         # Get command name
         groups = NAME_PATTERN.match(cmd_str)
         if (not groups):
             raise CmdCommandNameFormatError(self.module)
-        
+
         self.command = groups.group('name')
         param_str = groups.group('others')
         if param_str:
@@ -143,7 +143,7 @@ class BindCmdParse:
     def _parse_params(self, param_text):
         """convert a=b,c=d into one hash """
         param_text = _remove_list_and_map_whitespace(param_text)
-        
+
         # Check parameter name "help"
         param = NAME_PATTERN.match(param_text)
         if param and param.group('name') == "help":
@@ -153,7 +153,7 @@ class BindCmdParse:
         while True:
             if not param_text.strip():
                 break
-                
+
             groups = PARAM_PATTERN.match(param_text) or \
                      PARAM_WITH_QUOTA_PATTERN.match(param_text)
             if not groups:

src/bin/bindctl/tests/bindctl_test.py

@@ -40,14 +40,14 @@ except ImportError:
 class TestCmdLex(unittest.TestCase):
 
     def my_assert_raise(self, exception_type, cmd_line):
-        self.assertRaises(exception_type, cmdparse.BindCmdParse, cmd_line)
+        self.assertRaises(exception_type, cmdparse.BindCmdParser, cmd_line)
 
 
     def testCommandWithoutParameter(self):
-        cmd = cmdparse.BindCmdParse("zone add")
-        assert cmd.module == "zone"
-        assert cmd.command == "add"
-        self.assertEqual(len(cmd.params), 0)
+        cmd_parser = cmdparse.BindCmdParser("zone add")
+        assert cmd_parser.module == "zone"
+        assert cmd_parser.command == "add"
+        self.assertEqual(len(cmd_parser.params), 0)
 
 
     def testCommandWithParameters(self):
@@ -56,45 +56,51 @@ class TestCmdLex(unittest.TestCase):
                  "zone add zone_name = 'cnnic.cn\", file ='cnnic.cn.file' master=1.1.1.1, " }
 
         for cmd_line in lines:
-            cmd = cmdparse.BindCmdParse(cmd_line)
-            assert cmd.module == "zone"
-            assert cmd.command == "add"
-            assert cmd.params["zone_name"] == "cnnic.cn"
-            assert cmd.params["file"] == "cnnic.cn.file"
-            assert cmd.params["master"] == '1.1.1.1'
+            cmd_parser = cmdparse.BindCmdParser(cmd_line)
+            assert cmd_parser.module == "zone"
+            assert cmd_parser.command == "add"
+            assert cmd_parser.params["zone_name"] == "cnnic.cn"
+            assert cmd_parser.params["file"] == "cnnic.cn.file"
+            assert cmd_parser.params["master"] == '1.1.1.1'
 
     def testCommandWithParamters_2(self):
         '''Test whether the parameters in key=value can be parsed properly.'''
-        cmd = cmdparse.BindCmdParse('zone cmd name = 1:34::2')
-        self.assertEqual(cmd.params['name'], '1:34::2')
-
-        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 value=44\"\'\"')
-        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
-        self.assertEqual(cmd.params['value'], '44\"\'\"')
-
-        cmd = cmdparse.BindCmdParse('zone cmd name = 1\"\'34**&2 ,value=  44\"\'\"')
-        self.assertEqual(cmd.params['name'], '1\"\'34**&2')
-        self.assertEqual(cmd.params['value'], '44\"\'\"')
-
-        cmd = cmdparse.BindCmdParse('zone cmd name =  1\'34**&2value=44\"\'\" value = \"==============\'')
-        self.assertEqual(cmd.params['name'], '1\'34**&2value=44\"\'\"')
-        self.assertEqual(cmd.params['value'], '==============')
-
-        cmd = cmdparse.BindCmdParse('zone cmd name =    \"1234, 567890 \" value ==&*/')
-        self.assertEqual(cmd.params['name'], '1234, 567890 ')
-        self.assertEqual(cmd.params['value'], '=&*/')
+        cmd_parser = cmdparse.BindCmdParser('zone cmd name = 1:34::2')
+        self.assertEqual(cmd_parser.params['name'], '1:34::2')
+
+        cmd_parser = cmdparse.BindCmdParser('zone cmd name = 1\"\'34**&2'
+                                            ' value=44\"\'\"')
+        self.assertEqual(cmd_parser.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd_parser.params['value'], '44\"\'\"')
+
+        cmd_parser = cmdparse.BindCmdParser('zone cmd name = 1\"\'34**&2'
+                                            ',value=  44\"\'\"')
+        self.assertEqual(cmd_parser.params['name'], '1\"\'34**&2')
+        self.assertEqual(cmd_parser.params['value'], '44\"\'\"')
+
+        cmd_parser = cmdparse.BindCmdParser('zone cmd name =  1\'34**&2'
+                                            'value=44\"\'\" value = '
+                                            '\"==============\'')
+        self.assertEqual(cmd_parser.params['name'], '1\'34**&2value=44\"\'\"')
+        self.assertEqual(cmd_parser.params['value'], '==============')
+
+        cmd_parser = cmdparse.BindCmdParser('zone cmd name =    \"1234, '
+                                            '567890 \" value ==&*/')
+        self.assertEqual(cmd_parser.params['name'], '1234, 567890 ')
+        self.assertEqual(cmd_parser.params['value'], '=&*/')
 
     def testCommandWithListParam(self):
-        cmd = cmdparse.BindCmdParse("zone set zone_name='cnnic.cn', master='1.1.1.1, 2.2.2.2'")
-        assert cmd.params["master"] == '1.1.1.1, 2.2.2.2'
+        cmd_parser = cmdparse.BindCmdParser("zone set zone_name='cnnic.cn', "
+                                            "master='1.1.1.1, 2.2.2.2'")
+        assert cmd_parser.params["master"] == '1.1.1.1, 2.2.2.2'
 
     def testCommandWithHelpParam(self):
-        cmd = cmdparse.BindCmdParse("zone add help")
-        assert cmd.params["help"] == "help"
+        cmd_parser = cmdparse.BindCmdParser("zone add help")
+        assert cmd_parser.params["help"] == "help"
 
-        cmd = cmdparse.BindCmdParse("zone add help *&)&)*&&$#$^%")
-        assert cmd.params["help"] == "help"
-        self.assertEqual(len(cmd.params), 1)
+        cmd_parser = cmdparse.BindCmdParser("zone add help *&)&)*&&$#$^%")
+        assert cmd_parser.params["help"] == "help"
+        self.assertEqual(len(cmd_parser.params), 1)
 
 
     def testCmdModuleNameFormatError(self):
@@ -130,15 +136,20 @@ class TestCmdSyntax(unittest.TestCase):
         int_spec = { 'item_type' : 'integer',
                        'item_optional' : False,
                        'item_default' : 10}
-        zone_file_param = ParamInfo(name = "zone_file", param_spec = string_spec)
+        zone_file_param = ParamInfo(name = "zone_file",
+                                    param_spec = string_spec)
         zone_name = ParamInfo(name = 'zone_name', param_spec = string_spec)
         load_cmd = CommandInfo(name = "load")
         load_cmd.add_param(zone_file_param)
         load_cmd.add_param(zone_name)
 
-        param_master = ParamInfo(name = "master", optional = True, param_spec = string_spec)
-        param_master = ParamInfo(name = "port", optional = True, param_spec = int_spec)
-        param_allow_update = ParamInfo(name = "allow_update", optional = True, param_spec = string_spec)
+        param_master = ParamInfo(name = "master", optional = True,
+                                 param_spec = string_spec)
+        param_master = ParamInfo(name = "port", optional = True,
+                                 param_spec = int_spec)
+        param_allow_update = ParamInfo(name = "allow_update",
+                                       optional = True,
+                                       param_spec = string_spec)
         set_cmd = CommandInfo(name = "set")
         set_cmd.add_param(param_master)
         set_cmd.add_param(param_allow_update)
@@ -160,13 +171,14 @@ class TestCmdSyntax(unittest.TestCase):
 
 
     def no_assert_raise(self, cmd_line):
-        cmd = cmdparse.BindCmdParse(cmd_line)
-        self.bindcmd._validate_cmd(cmd)
+        cmd_parser = cmdparse.BindCmdParser(cmd_line)
+        self.bindcmd._validate_cmd(cmd_parser)
 
 
     def my_assert_raise(self, exception_type, cmd_line):
-        cmd = cmdparse.BindCmdParse(cmd_line)
-        self.assertRaises(exception_type, self.bindcmd._validate_cmd, cmd)
+        cmd_parser = cmdparse.BindCmdParser(cmd_line)
+        self.assertRaises(exception_type, self.bindcmd._validate_cmd,
+                          cmd_parser)
 
 
     def testValidateSuccess(self):
@@ -177,7 +189,8 @@ class TestCmdSyntax(unittest.TestCase):
         self.no_assert_raise("zone help help='dd' ")
         self.no_assert_raise("zone set allow_update='1.1.1.1' zone_name='cn'")
         self.no_assert_raise("zone set zone_name='cn'")
-        self.my_assert_raise(isc.cc.data.DataTypeError, "zone set zone_name ='cn', port='cn'")
+        self.my_assert_raise(isc.cc.data.DataTypeError,
+                             "zone set zone_name ='cn', port='cn'")
         self.no_assert_raise("zone reload_all")
 
     def testCmdUnknownModuleSyntaxError(self):
@@ -188,15 +201,22 @@ class TestCmdSyntax(unittest.TestCase):
         self.my_assert_raise(CmdUnknownCmdSyntaxError, "zone dd")
 
     def testCmdMissParamSyntaxError(self):
-        self.my_assert_raise(CmdMissParamSyntaxError, "zone load zone_file='cn'")
-        self.my_assert_raise(CmdMissParamSyntaxError, "zone load zone_name='cn'")
-        self.my_assert_raise(CmdMissParamSyntaxError, "zone set allow_update='1.1.1.1'")
-        self.my_assert_raise(CmdMissParamSyntaxError, "zone set ")
+        self.my_assert_raise(CmdMissParamSyntaxError,
+                             "zone load zone_file='cn'")
+        self.my_assert_raise(CmdMissParamSyntaxError,
+                             "zone load zone_name='cn'")
+        self.my_assert_raise(CmdMissParamSyntaxError,
+                             "zone set allow_update='1.1.1.1'")
+        self.my_assert_raise(CmdMissParamSyntaxError,
+                             "zone set ")
 
     def testCmdUnknownParamSyntaxError(self):
-        self.my_assert_raise(CmdUnknownParamSyntaxError, "zone load zone_d='cn'")
-        self.my_assert_raise(CmdUnknownParamSyntaxError, "zone reload_all zone_name = 'cn'")
-        self.my_assert_raise(CmdUnknownParamSyntaxError, "zone help a b c")
+        self.my_assert_raise(CmdUnknownParamSyntaxError,
+                             "zone load zone_d='cn'")
+        self.my_assert_raise(CmdUnknownParamSyntaxError,
+                             "zone reload_all zone_name = 'cn'")
+        self.my_assert_raise(CmdUnknownParamSyntaxError,
+                             "zone help a b c")
 
 class TestModuleInfo(unittest.TestCase):
 
@@ -233,7 +253,8 @@ class TestNameSequence(unittest.TestCase):
             self.tool.add_module_info(ModuleInfo(name = random_str))
 
     def setUp(self):
-        self.random_names = ['1erdfeDDWsd', '3fe', '2009erd', 'Fe231', 'tere142', 'rei8WD']
+        self.random_names = ['1erdfeDDWsd', '3fe', '2009erd',
+                             'Fe231', 'tere142', 'rei8WD']
         self._create_bindcmd()
 
     def testSequence(self):
@@ -321,7 +342,8 @@ class TestConfigCommands(unittest.TestCase):
         def precmd(line):
             self.tool.precmd(line)
         self.tool._update_all_modules_info = update_all_modules_info
-        # If line is equals to 'EOF', _update_all_modules_info() shouldn't be called
+        # If line is equals to 'EOF', _update_all_modules_info()
+        # shouldn't be called
         precmd('EOF')
         self.assertRaises(socket.error, precmd, 'continue')
 
@@ -360,34 +382,41 @@ class TestConfigCommands(unittest.TestCase):
         self.assertEqual((1, MultiConfigData.DEFAULT),
                          self.tool.config_data.get_value("/foo/an_int"))
 
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"5\"")
-        self.tool.apply_config_cmd(cmd)
+        cmd_parser = cmdparse.BindCmdParser('config set identifier='
+                                            '"foo/an_int" value="5"')
+        self.tool.apply_config_cmd(cmd_parser)
         self.assertEqual((5, MultiConfigData.LOCAL),
                          self.tool.config_data.get_value("/foo/an_int"))
 
-        cmd = cmdparse.BindCmdParse("config unset identifier=\"foo/an_int\"")
-        self.tool.apply_config_cmd(cmd)
+        cmd_parser = cmdparse.BindCmdParser('config unset identifier='
+                                            '"foo/an_int"')
+        self.tool.apply_config_cmd(cmd_parser)
 
         self.assertEqual((1, MultiConfigData.DEFAULT),
                          self.tool.config_data.get_value("/foo/an_int"))
 
         # this should raise a NotFoundError
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/bar\" value=\"[]\"")
-        self.assertRaises(isc.cc.data.DataNotFoundError, self.tool.apply_config_cmd, cmd)
+        cmd_parser = cmdparse.BindCmdParser('config set identifier='
+                                            '"foo/bar" value="[]"')
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          self.tool.apply_config_cmd, cmd_parser)
 
-        cmd = cmdparse.BindCmdParse("config unset identifier=\"foo/bar\"")
+        cmd_parser = cmdparse.BindCmdParser('config unset identifier='
+                                            '"foo/bar"')
         self.assertRaises(isc.cc.data.DataNotFoundError,
-                          self.tool.apply_config_cmd, cmd)
+                          self.tool.apply_config_cmd, cmd_parser)
 
         # this should raise a TypeError
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/an_int\" value=\"[]\"")
-        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+        cmd_parser = cmdparse.BindCmdParser('config set identifier='
+                                            '"foo/an_int" value="[]"')
+        self.assertRaises(isc.cc.data.DataTypeError,
+                          self.tool.apply_config_cmd, cmd_parser)
 
     # this is a very specific one for use with a set of list tests
     # to try out the flexibility of the parser (only in the next test)
     def clt(self, full_cmd_string, item_value):
-        cmd = cmdparse.BindCmdParse(full_cmd_string)
-        self.tool.apply_config_cmd(cmd)
+        cmd_parser = cmdparse.BindCmdParser(full_cmd_string)
+        self.tool.apply_config_cmd(cmd_parser)
         self.assertEqual(([item_value], MultiConfigData.LOCAL),
                          self.tool.config_data.get_value("/foo/a_list"))
 
@@ -410,15 +439,56 @@ class TestConfigCommands(unittest.TestCase):
         self.clt("config set identifier = \"foo/a_list\" value=[ \"k\" ]", "k")
 
         # this should raise a TypeError
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=\"a\"")
-        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+        cmd_parser = cmdparse.BindCmdParser('config set identifier='
+                                            '"foo/a_list" value="a"')
+        self.assertRaises(isc.cc.data.DataTypeError,
+                          self.tool.apply_config_cmd, cmd_parser)
 
-        cmd = cmdparse.BindCmdParse("config set identifier=\"foo/a_list\" value=[1]")
-        self.assertRaises(isc.cc.data.DataTypeError, self.tool.apply_config_cmd, cmd)
+        cmd_parser = cmdparse.BindCmdParser('config set identifier='
+                                            '"foo/a_list" value=[1]')
+        self.assertRaises(isc.cc.data.DataTypeError,
+                          self.tool.apply_config_cmd, cmd_parser)
 
     def tearDown(self):
         sys.stdout = self.stdout_backup
 
+    def test_cmd_has_identifier_param(self):
+        module = ModuleInfo(name="test_module")
+
+        cmd = CommandInfo(name="command_with_identifier")
+        param = ParamInfo(name=bindcmd.CFGITEM_IDENTIFIER_PARAM)
+        cmd.add_param(param)
+        module.add_command(cmd)
+
+        cmd = CommandInfo(name="command_without_identifier")
+        param = ParamInfo(name="some_argument")
+        cmd.add_param(param)
+        module.add_command(cmd)
+
+        self.tool.add_module_info(module)
+
+        cmd_parser = cmdparse.BindCmdParser('test_module '
+                                            'command_with_identifier')
+        self.assertTrue(self.tool._cmd_has_identifier_param(cmd_parser))
+
+        cmd_parser = cmdparse.BindCmdParser('test_module '
+                                            'command_without_identifier')
+        self.assertFalse(self.tool._cmd_has_identifier_param(cmd_parser))
+
+        cmd_parser = cmdparse.BindCmdParser('badmodule '
+                                            'command_without_identifier')
+        self.assertFalse(self.tool._cmd_has_identifier_param(cmd_parser))
+
+    def test_get_identifier_startswith(self):
+        hints = self.tool._get_identifier_startswith("/")
+        self.assertEqual(['foo/an_int', 'foo/a_list'], hints)
+
+        hints = self.tool._get_identifier_startswith("/foo/an")
+        self.assertEqual(['foo/an_int'], hints)
+
+        hints = self.tool._get_identifier_startswith("/bar")
+        self.assertEqual([], hints)
+
 class FakeBindCmdInterpreter(bindcmd.BindCmdInterpreter):
     def __init__(self):
         pass

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

@@ -49,6 +49,22 @@
             "item_optional": true
           }
         }
+      },
+      { "item_name": "named_set_item4",
+        "item_type": "named_set",
+        "item_optional": true,
+        "item_default": {},
+        "named_set_item_spec": {
+          "item_name": "named_set_element",
+          "item_type": "named_set",
+          "item_optional": false,
+          "item_default": { "a": 1, "b": 2 },
+          "named_set_item_spec":
+          { "item_name": "named_set_element",
+            "item_type": "integer",
+            "item_optional": true
+          }
+        }
       }
     ]
   }

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/ccsession.py

@@ -575,7 +575,7 @@ class UIModuleCCSession(MultiConfigData):
         # for type any, we determine the 'type' by what value is set
         # (which would be either list or dict)
         cur_value, _ = self.get_value(identifier)
-        type_any = module_spec['item_type'] == 'any'
+        type_any = isc.config.config_data.spec_part_is_any(module_spec)
 
         # the specified element must be a list or a named_set
         if 'list_item_spec' in module_spec or\
@@ -603,7 +603,7 @@ class UIModuleCCSession(MultiConfigData):
             self._add_value_to_named_set(identifier, item_name,
                                          item_value)
         else:
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named set")
+            raise isc.cc.data.DataTypeError(str(identifier) + " is not a list or a named set")
 
     def _remove_value_from_list(self, identifier, value):
         if value is None:
@@ -653,7 +653,7 @@ class UIModuleCCSession(MultiConfigData):
         # for type any, we determine the 'type' by what value is set
         # (which would be either list or dict)
         cur_value, _ = self.get_value(identifier)
-        type_any = module_spec['item_type'] == 'any'
+        type_any = isc.config.config_data.spec_part_is_any(module_spec)
 
         # there's two forms of 'remove from list'; the remove-value-from-list
         # form, and the 'remove-by-index' form. We can recognize the second
@@ -668,7 +668,7 @@ class UIModuleCCSession(MultiConfigData):
            (type_any and type(cur_value) == dict):
             self._remove_value_from_named_set(identifier, value_str)
         else:
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named_set")
+            raise isc.cc.data.DataTypeError(str(identifier) + " is not a list or a named_set")
 
 
 

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

@@ -45,6 +45,13 @@ def spec_part_is_named_set(spec_part):
        named_set specification, and False otherwise."""
     return (type(spec_part) == dict and 'named_set_item_spec' in spec_part)
 
+def spec_part_is_any(spec_part):
+    """Returns true if the given spec_part specifies an element of type
+       any, and False otherwise.
+    """
+    return (type(spec_part) == dict and 'item_type' in spec_part and
+            spec_part['item_type'] == "any")
+
 def check_type(spec_part, value):
     """Does nothing if the value is of the correct type given the
        specification part relevant for the value. Raises an
@@ -237,7 +244,8 @@ def spec_name_list(spec, prefix="", recurse=False):
         elif 'named_set_item_spec' in spec:
             # we added a '/' above, but in this one case we don't want it
             result.append(prefix[:-1])
-        else:
+        # ignore any
+        elif not spec_part_is_any(spec):
             for name in spec:
                 result.append(prefix + name + "/")
                 if recurse:
@@ -392,14 +400,25 @@ class MultiConfigData:
            identifier, or None if not found. The first part of the
            identifier (up to the first /) is interpreted as the module
            name. Returns None if not found, or if identifier is not a
-           string."""
+           string.
+           If an index is given for a List-type element, it returns
+           the specification of the list elements, not of the list itself
+           """
         if type(identifier) != str or identifier == "":
             return None
         if identifier[0] == '/':
             identifier = identifier[1:]
         module, sep, id = identifier.partition("/")
+        if id != "":
+            id, indices = isc.cc.data.split_identifier_list_indices(id)
+        else:
+            indices = None
         try:
-            return find_spec_part(self._specifications[module].get_config_spec(), id)
+            spec_part = find_spec_part(self._specifications[module].get_config_spec(), id)
+            if indices is not None and spec_part_is_list(spec_part):
+                return spec_part['list_item_spec']
+            else:
+                return spec_part
         except isc.cc.data.DataNotFoundError as dnfe:
             return None
         except KeyError as ke:
@@ -780,19 +799,75 @@ class MultiConfigData:
            indices and named_set names to the completion list. If
            the given item_name is for a list or named_set, it'll
            return a list of those (appended to item_name), otherwise
-           the list will only contain the item_name itself."""
+           the list will only contain the item_name itself.
+
+           If the item is a named set, and it's contents are maps
+           or named_sets as well, a / is appended to the result
+           strings.
+
+           If the item is a list, this method is then called recursively
+           for each list entry.
+
+           This behaviour is slightly arbitrary, and currently reflects
+           the most probable way the resulting data should look like;
+           for lists, bindctl would always expect their contents to
+           be added as well. For named_sets, however, we do not
+           do recursion, since the resulting list may be too long.
+           This will probably change in a revision of the way this
+           data is handled; ideally, the result should always recurse,
+           but only up to a limited depth, and the resulting list
+           should probably be paginated clientside.
+
+           Parameters:
+           item_name (string): the (full) identifier for the list or
+                               named_set to enumerate.
+
+           Returns a list of strings with item names
+
+           Examples:
+           _get_list_items("Module/some_item")
+               where item is not a list of a named_set, or where
+               said list or named set is empty, returns
+               ["Module/some_item"]
+           _get_list_items("Module/named_set")
+               where the named_set contains items with names 'a'
+               and 'b', returns
+               [ "Module/named_set/a", "Module/named_set/b" ]
+           _get_list_items("Module/named_set_of_maps")
+               where the named_set contains items with names 'a'
+               and 'b', and those items are maps themselves
+               (or other named_sets), returns
+               [ "Module/named_set/a/", "Module/named_set/b/" ]
+           _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 'item_type' in spec_part and \
-           spec_part['item_type'] == 'named_set':
-            subslash = ""
-            if spec_part['named_set_item_spec']['item_type'] == 'map' or\
-               spec_part['named_set_item_spec']['item_type'] == 'named_set':
-                subslash = "/"
-            values, status = self.get_value(item_name)
-            if len(values) > 0:
+        if spec_part_is_named_set(spec_part):
+            values, _ = self.get_value(item_name)
+            if values is not None and len(values) > 0:
+                subslash = ""
+                if spec_part['named_set_item_spec']['item_type'] == 'map' or\
+                   spec_part['named_set_item_spec']['item_type'] == 'named_set':
+                    subslash = "/"
+                # Don't recurse for named_sets (so as not to return too
+                # much data), but do add a / so the client so that
+                # the user can immediately tab-complete further if needed.
                 return [ item_name + "/" + v + subslash for v in values.keys() ]
             else:
                 return [ item_name ]
+        elif spec_part_is_list(spec_part):
+            values, _ = self.get_value(item_name)
+            if values is not None and len(values) > 0:
+                result = []
+                for i in range(len(values)):
+                    name = item_name + '[%d]' % i
+                    # Recurse for list entries, so that its sub-contents
+                    # are also added to the result
+                    result.extend(self._get_list_items(name))
+                return result
+            else:
+                return [ item_name ]
         else:
             return [ item_name ]
 

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

@@ -993,10 +993,15 @@ class TestUIModuleCCSession(unittest.TestCase):
 
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "no_such_item", "a")
-        self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec2/item1", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "no_such_item", "a")
-        self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "Spec2/item1", "a")
+        # add and remove should raise DataNotFoundError when used with items
+        # that are not a list or named_set (more importantly, they should
+        # not raise TypeError)
+        self.assertRaises(isc.cc.data.DataTypeError, uccs.add_value, "Spec2/item1", "a")
+        self.assertRaises(isc.cc.data.DataTypeError, uccs.remove_value, "Spec2/item1", "a")
+        self.assertRaises(isc.cc.data.DataTypeError, uccs.add_value, "Spec2", "")
+        self.assertRaises(isc.cc.data.DataTypeError, uccs.remove_value, "Spec2", "")
 
         self.assertEqual({}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
@@ -1020,6 +1025,7 @@ class TestUIModuleCCSession(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError,
                           uccs.remove_value, "Spec2/item5", None)
 
+
     # Check that the difference between no default and default = null
     # is recognized
     def test_default_null(self):
@@ -1042,9 +1048,9 @@ class TestUIModuleCCSession(unittest.TestCase):
         items_as_str = [ '1234', 'foo', 'true', 'false' ]
 
         def test_fails():
-            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec40/item1", "foo")
-            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec40/item1", "foo", "bar")
-            self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "Spec40/item1", "foo")
+            self.assertRaises(isc.cc.data.DataTypeError, uccs.add_value, "Spec40/item1", "foo")
+            self.assertRaises(isc.cc.data.DataTypeError, uccs.add_value, "Spec40/item1", "foo", "bar")
+            self.assertRaises(isc.cc.data.DataTypeError, uccs.remove_value, "Spec40/item1", "foo")
             self.assertRaises(isc.cc.data.DataTypeError, uccs.remove_value, "Spec40/item1[0]", None)
 
         # A few helper functions to perform a number of tests

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

@@ -252,6 +252,18 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(ConfigDataError, spec_name_list, 1)
         self.assertRaises(ConfigDataError, spec_name_list, [ 'a' ])
 
+        # Test one with type any as well
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec40.spec")
+        spec_part = module_spec.get_config_spec()
+        name_list = spec_name_list(module_spec.get_config_spec())
+        self.assertEqual(['item1', 'item2', 'item3'], name_list)
+
+        # item3 itself is 'empty'
+        spec_part = find_spec_part(spec_part, 'item3')
+        name_list = spec_name_list(spec_part)
+        self.assertEqual([], name_list)
+
+
     def test_init(self):
         self.assertRaises(ConfigDataError, ConfigData, "asdf")
 
@@ -377,12 +389,37 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(None, spec_part)
         spec_part = self.mcd.find_spec_part("/Spec2/item1")
         self.assertEqual(None, spec_part)
-        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
+        module_spec = isc.config.module_spec_from_file(self.data_path +
+                                                       os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
         spec_part = self.mcd.find_spec_part("Spec2/item1")
-        self.assertEqual({'item_name': 'item1', 'item_type': 'integer', 'item_optional': False, 'item_default': 1, }, spec_part)
+        self.assertEqual({'item_name': 'item1', 'item_type': 'integer',
+                          'item_optional': False, 'item_default': 1, },
+                         spec_part)
+
+        # For lists, either the spec of the list itself, or the
+        # spec for the list contents should be returned (the
+        # latter when an index is given in the identifier)
+        spec_part = self.mcd.find_spec_part("Spec2/item5")
+        self.assertEqual({'item_default': ['a', 'b'],
+                          'item_name': 'item5',
+                          'item_optional': False,
+                          'item_type': 'list',
+                          'list_item_spec': {'item_default': '',
+                                             'item_name': 'list_element',
+                                             'item_optional': False,
+                                             'item_type': 'string'}},
+                         spec_part)
+        spec_part = self.mcd.find_spec_part("Spec2/item5[0]")
+        self.assertEqual({'item_default': '',
+                          'item_name': 'list_element',
+                          'item_optional': False,
+                          'item_type': 'string'},
+                         spec_part)
+
 
     def test_find_spec_part_nested(self):
+        # Check that find_spec_part works for nested lists
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec30.spec")
         self.mcd.set_specification(module_spec)
         spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/final_element")
@@ -391,6 +428,7 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(None, spec_part)
 
     def test_find_spec_part_nested2(self):
+        # Check that find_spec_part works for nested lists and maps
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec31.spec")
         self.mcd.set_specification(module_spec)
         spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/map_element/list1[1]/list2[2]")
@@ -638,7 +676,7 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(expected, maps)
 
         # A slash at the end should not produce different output with
-        # indices too
+        # indices either
         expected2 = [{'default': True,
                       'type': 'integer',
                       'name': 'Spec22/value5[1]',
@@ -720,6 +758,8 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataNotFoundError, self.mcd.unset, "Spec2/doesnotexist")
 
     def test_get_config_item_list(self):
+        # Test get_config_item_list(), which returns a list of the config
+        # items in a specification.
         config_items = self.mcd.get_config_item_list()
         self.assertEqual([], config_items)
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
@@ -729,15 +769,42 @@ class TestMultiConfigData(unittest.TestCase):
         config_items = self.mcd.get_config_item_list(None, False)
         self.assertEqual(['Spec2'], config_items)
         config_items = self.mcd.get_config_item_list(None, True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1',
+                          'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          'Spec2/item4', 'Spec2/item5[0]', 'Spec2/item5[1]',
+                          'Spec2/item6/value1', 'Spec2/item6/value2'],
+                          config_items)
         config_items = self.mcd.get_config_item_list("Spec2")
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          'Spec2/item4', 'Spec2/item5[0]', 'Spec2/item5[1]',
+                          'Spec2/item6'], config_items)
         config_items = self.mcd.get_config_item_list("/Spec2")
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          'Spec2/item4', 'Spec2/item5[0]', 'Spec2/item5[1]',
+                          'Spec2/item6'], config_items)
+        config_items = self.mcd.get_config_item_list("Spec2", True)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          'Spec2/item4', 'Spec2/item5[0]', 'Spec2/item5[1]',
+                          'Spec2/item6/value1', 'Spec2/item6/value2'],
+                          config_items)
+
+        # When lists are empty, it should only show the name
+        self.mcd.set_value('Spec2/item5', [])
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3',
+                          '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")
@@ -756,7 +823,8 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(['Spec32'], config_items)
         config_items = self.mcd.get_config_item_list(None, True)
         self.assertEqual(['Spec32/named_set_item', 'Spec32/named_set_item2',
-                          'Spec32/named_set_item3'], config_items)
+                          'Spec32/named_set_item3', 'Spec32/named_set_item4'],
+                         config_items)
         self.mcd.set_value('Spec32/named_set_item', { "aaaa": 4, "aabb": 5,
                                                       "bbbb": 6})
         config_items = self.mcd.get_config_item_list("/Spec32/named_set_item",
@@ -766,6 +834,19 @@ class TestMultiConfigData(unittest.TestCase):
                           'Spec32/named_set_item/bbbb',
                          ], config_items)
 
+        self.mcd.set_value('Spec32/named_set_item', {})
+        config_items = self.mcd.get_config_item_list("/Spec32/named_set_item",
+                                                     True)
+        self.assertEqual(['Spec32/named_set_item'], config_items)
+
+        self.mcd.set_value('Spec32/named_set_item4', { "a": { "aa": 4 } } )
+        config_items = self.mcd.get_config_item_list("/Spec32/named_set_item4",
+                                                     True)
+        self.assertEqual(['Spec32/named_set_item4/a/'], config_items)
+        config_items = self.mcd.get_config_item_list("/Spec32/named_set_item4/a",
+                                                     True)
+        self.assertEqual(['Spec32/named_set_item4/a/aa'], config_items)
+
     def test_set_named_set_nonlocal(self):
         # Test whether a default named set is copied to local if a subitem
         # is changed, and that other items in the set do not get lost