From: Jelte Jansen Date: Tue, 25 Sep 2012 21:14:22 +0000 (+0200) Subject: [2254] refactor tab-completion for identifiers X-Git-Tag: trac2402_base~83^2~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6978e7fbdfb9c036e1f0fedf04828d6edc731d9e;p=thirdparty%2Fkea.git [2254] refactor tab-completion for identifiers removed all word-boundary values except whitespace (so that we no longer have to to magic to replace things around the / character, and other possible characters used in identifiers) Pulled out the identifier-hints collection (and added tests for those) --- diff --git a/src/bin/bindctl/bindcmd.py b/src/bin/bindctl/bindcmd.py index 87aa217ba0..9e7b1e1511 100644 --- a/src/bin/bindctl/bindcmd.py +++ b/src/bin/bindctl/bindcmd.py @@ -48,20 +48,18 @@ 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 whitespace as word boundaries + readline.set_completer_delims(' \t\n') 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 +IDENTIFIER_PARAM = 'identifier' + CSV_FILE_NAME = 'default_user.csv' CONFIG_MODULE_NAME = 'config' CONST_BINDCTL_HELP = """ @@ -463,20 +461,24 @@ 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""" + # First get all items from the given module (up to the first /) + list = self.config_data.get_config_item_list( + id_text.rpartition("/")[0], True) + hints = [val for val in list if val.startswith(id_text[1:])] + 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 + """ + 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) def complete(self, text, state): if 0 == state: @@ -491,17 +493,12 @@ class BindCmdInterpreter(Cmd): 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 - prefix, _, rest = my_text.rpartition("/") - hints = self.remove_prefix(hints, prefix) - # And prevent 'double addition' by also removing final - # part matches - hints = [ h for h in hints if h != rest ] + if self._cmd_has_identifier_param(cmd): + # For tab-completion of identifiers, replace hardcoded + # hints with hints derived from the config data + id_text = self.location + "/" + cur_line.rpartition(" ")[2] + hints = self._get_identifier_startswith(id_text) + except CmdModuleNameFormatError: if not text: hints = self.get_module_names() @@ -522,13 +519,8 @@ class BindCmdInterpreter(Cmd): except BindCtlException: hints = [] - # there are a couple of 'standard' names that are usable, but - # should not be included in direct tab-completion - hints = [ h for h in hints if h not in [ 'argument', 'identifier']] - self.hint = hints - if state < len(self.hint): return self.hint[state] else: diff --git a/src/bin/bindctl/bindctl_main.py.in b/src/bin/bindctl/bindctl_main.py.in index 1685355786..b6a5b18423 100755 --- a/src/bin/bindctl/bindctl_main.py.in +++ b/src/bin/bindctl/bindctl_main.py.in @@ -42,12 +42,12 @@ 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 = 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) + param = ParamInfo(name = IDENTIFIER_PARAM, type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC) cmd.add_param(param) module.add_command(cmd) @@ -60,7 +60,7 @@ 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 = 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.") cmd.add_param(param) @@ -70,21 +70,21 @@ 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", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC) + param = ParamInfo(name = 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) + param = ParamInfo(name = 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.") 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) + param = ParamInfo(name = IDENTIFIER_PARAM, type = "string", optional=False, desc = DEFAULT_IDENTIFIER_DESC) cmd.add_param(param) module.add_command(cmd) @@ -98,7 +98,7 @@ def prepare_config_commands(tool): 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) + param = ParamInfo(name = IDENTIFIER_PARAM, type="string", optional=False, desc = DEFAULT_IDENTIFIER_DESC) cmd.add_param(param) module.add_command(cmd) diff --git a/src/bin/bindctl/tests/bindctl_test.py b/src/bin/bindctl/tests/bindctl_test.py index bcfb6c594d..3364211d3d 100644 --- a/src/bin/bindctl/tests/bindctl_test.py +++ b/src/bin/bindctl/tests/bindctl_test.py @@ -419,6 +419,40 @@ class TestConfigCommands(unittest.TestCase): 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.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 = cmdparse.BindCmdParse("test_module command_with_identifier") + self.assertTrue(self.tool._cmd_has_identifier_param(cmd)) + + cmd = cmdparse.BindCmdParse("test_module command_without_identifier") + self.assertFalse(self.tool._cmd_has_identifier_param(cmd)) + + cmd = cmdparse.BindCmdParse("badmodule command_without_identifier") + self.assertFalse(self.tool._cmd_has_identifier_param(cmd)) + + 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 diff --git a/src/lib/python/isc/config/config_data.py b/src/lib/python/isc/config/config_data.py index e09ce3358d..a09a7c4f7c 100644 --- a/src/lib/python/isc/config/config_data.py +++ b/src/lib/python/isc/config/config_data.py @@ -807,13 +807,13 @@ class MultiConfigData: spec_part['named_set_item_spec']['item_type'] == 'named_set': subslash = "/" values, status = self.get_value(item_name) - if len(values) > 0: + if values is not None and len(values) > 0: return [ item_name + "/" + v + subslash for v in values.keys() ] else: return [ item_name ] elif spec_part_is_list(spec_part): values, status = self.get_value(item_name) - if len(values) > 0: + if values is not None and len(values) > 0: result = [] for i in range(len(values)): name = item_name + '[%d]' % i