pass the namespace to completion function

[teto]

I would like to set a choices_function that depends on the previous arguments, i.e., I believe cmd2 should pass the current parser namespace when calling the choices_function Like I have this first argument that loads a csv file, and I would like the 2nd argument to choose between availables ids in the previously loaded csv file. Right now thechoices_function looks a bit useless and can only return static results.

[anselor]

@kmvanbrunt is working on an update to the autocompleter that will give the completer function that will give a dict containing all prior arguments.

[teto]

oh cool, seems like it's https://github.com/python-cmd2/cmd2/commits/completion_state . I will try it locally if the API is stable ?

[tleonhardt]

@kmvanbrunt Can you comment as to the likely API stability of the code on the completion_state branch?

[anselor]

@teto I think it'd probably be worthwhile for you to try it and give some feedback. From my conversation with @kmvanbrunt about it I don't think it's going to change very much.

Also, I think it's actually giving a namespace of string tokens, not a dict.

[kmvanbrunt]

@teto choices_function and choices_method serve two purposes

1. argparse's choices parameter only allows Iterables (e.g. lists, sets, tuples). If a choices list needs to be generated from a dynamic source like a database or other application state data, then choices_function and choices_method allow for this.

2. If you have a lot of choices and don't want help text that looks like the following, then providing this list with one of our parameters solves the problem.

   Usage: fake [-h]
           {choice_01, choice_02, choice_03, choice_04, choice_05, choice_06,
           choice_07, choice_08, choice_09, choice_10, choice_11, choice_12,
           choice_13, choice_14, choice_15, choice_16, choice_17}

The documentation in argparse_custom.py says this about these parameters:

Pass a function that returns choices. This is good in cases where the choice list is dynamically generated when the user hits tab.

[kmvanbrunt]

@teto @anselor @tleonhardt The completion_state branch is mostly done, but I'd like feedback on my design choices. Once the design is finalized, then I need to update documentation and unit tests.

I've added a parameter called pass_parsed_args to:

• add_argument()
• set_choices_function()
• set_choices_method()
• set_completer_function()
• set_completer_method()

If this parameter is True, then choices/completer functions is passed an argparse.Namespace by AutoCompleter. The namespace contains tokens prior to the one being tab completed organized under the argparse argument they belong to.

Ex:

parser = Cmd2ArgumentParser()
parser.add_argument('arg1')
parser.add_argument('arg2')
parser.add_argument('arg3')

If you are completing arg3, then the Namespace will have arg1 and arg2 members. All members of the Namespace are lists of strings that contain the tokens which belong to that argument.

If arg3 was changed to:

parser.add_argument('arg3', nargs=3)

If you were tab completing the third token of arg3, then the Namespace will now have an arg3 member whose value is a list of the first two tokens of arg3.

So I'd like opinions on these design choices

1. Is pass_parsed_args a good name?
2. All members of the Namespace are lists, even if a particular argument expects only 1 token
3. Since AutoCompleter is for tab completion, it does not convert the tokens to their actual argument types or validate their values. All tokens are stored in the Namespace as the raw strings provided on the command line. It is up to the developer to determine if the user entered the correct argument type (e.g. int) and validate their values.

[teto]

I wonder if pass_parsed_args makes sense ? Isn't it simpler to always pass arguments ? it breaks backwards compatibility but the feature is very recent (~a month ?) and anyway recent cmd2 versions haven't been scared of breaking backwards compatibility.

the functions set_choices_function/set_choices_method have been introduced following one of my issues I think, yet they don't seem to do much, they seem more like a burden to the devs so might be best to get rid of them ?

I wonder if having a cmd2.Action that derives from argparse.Action could not help with implementation ? just a random thought I am not familiar with the code enough to make that call.

  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/cmd2.py", line 1569, in complete
    self._completion_for_command(text, line, begidx, endidx, shortcut_to_restore)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/cmd2.py", line 1454, in _completion_for_command
    self.completion_matches = self._redirect_complete(text, line, begidx, endidx, compfunc)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/cmd2.py", line 1249, in _redirect_complete
    return compfunc(text, line, begidx, endidx)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/cmd2.py", line 1607, in _autocomplete_default
    return completer.complete_command(tokens_to_parse, text, line, begidx, endidx)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/argparse_completer.py", line 255, in complete_command
    return completer.complete_command(tokens[token_index:], text, line, begidx, endidx)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/argparse_completer.py", line 324, in complete_command
    begidx, endidx, consumed_arg_values)
  File "/nix/store/k85cwpsdamsznlk2yk1rzjbym3pjhwb3-python3.7-cmd2-0.9.17/lib/python3.7/site-packages/cmd2/argparse_completer.py", line 473, in _complete_for_arg
    arg_choices = arg_choices.to_call(*args)
TypeError: 'ChoicesCallable' object is not callable

in https://github.com/teto/mptcpanalyzer/commit/370fb5222cb992ceaf29a1038439667604e6ee85#diff-32b8d0cac9da815b67726256f094d4aeR555

[kmvanbrunt]

@teto pass_parsed_args is needed because completer functions aren't required to take a Namespace. For instance, path_complete doesn't take one. This is the same reason we have the choice_function/choice_method and completer_function/completer_method distinctions. They basically tell the AutoCompleter whether or not the functions expect self to be passed.

Not all cmd2 software uses the argparse capabilities, but they still can use the tab completion functions without worrying about passing a Namespace.

The set_choices_*/set_completer_* functions I added are for custom action types which enable tab completion in their initializer. These functions do exactly what add_argument() would do without the developer needing to know the underlying details of ChoicesCallable.

I'm not sure how having a cmd2.Action would make life easier. Passing parameters to add_argument already accounts for actions of any type. What did you have in mind with this suggestion?

Assuming the params dictionary defines parameters passed to add_argument(), your code needs to be changed to:

params = {
            'action': "store",
            'help': proto_str + '.stream wireshark id',
            'choices_function': show_range,
            'pass_parsed_args': True
        }

[teto]

pass_parsed_args is needed because completer functions aren't required to take a Namespace. For instance, path_complete doesn't take one

I think it's best to always pass on the namespace (better more info than none + it reduces complexitiy since we can remove pass_parsed_args), even if it means ignoring it in the completer. Btw with your change, I can make it work. Very cool thanks.

[teto]

I would like to run functions that may take some time to complete (like loading a csv on the fly). zsh/bash can interrupt autocompletion via Ctrl-C, would something like that be doable with cmd2 ? Like interrupt the autocompletion function via Ctrl-C.

[kmvanbrunt]

cmd2 already has a Ctrl-C handler called sigint_handler() that will interrupt your function. You won't have to make any changes.

[teto]

ok I will try. I have another question: my understanding of cmd2 is that the Namespace is filled by each Action when the action is called. Nevertheless, your branch manages to pass a Namespace that is filled even though actions have not been called yet. Do you do some magic first ? or do I get the argparse working wrong.

One of my issue is that I would like to trigger the Action of the previous argument upon calling completion. I wonder if it wouldn't be worth to pass the parser along with the namespace.

[kmvanbrunt]

cmd2 uses a custom class called AutoCompleter to perform the tab completion functionality on argparse-based commands. While AutoCompleter has a decent understanding of argparse, it only uses this knowledge to figure out which action each command-line token belongs to and identify which action is currently being completed. Argument validation and triggering of actions does not occur until you press Enter and we call parse_args(). This is why the Namespace I'm passing you only contains lists of strings organized by each argument (action). These are still the raw tokens from the command line.

For your situation, it does sound like you will also need the parser. It won't be hard to pass that to you, but performing argument validation during tab completion introduces other problems, What will you do if an action you trigger raises an argparse.ArgumentError or argparse.ArgumentTypeError? One option is that AutoCompleter could catch these exception types and print their errors and redraw the prompt.

[kmvanbrunt]

@teto I added a member to the namespace I pass you called __parser__. See if this helps accomplish what you're trying to do.

[kmvanbrunt]

@teto @tleonhardt I've given more thought to the design suggested by @teto where he said:

I think it's best to always pass on the namespace (better more info than none + it reduces complexitiy since we can remove pass_parsed_args), even if it means ignoring it in the completer.

I agree with this approach because it does reduce the complexity of adding tab completion to argparse commands. To support this, I will require all choices and completion functions called by AutoCompleter to take a positional argument for the Namespace.

choices functions will be passed the Namespace for their first argument completer functions will have these positionals (text, line, begidx, endidx, namespace)

For those functions that don't need the Namespace, they can use *args as a placeholder in their signature.

Do you see any issues with this? Would it better to make the Namespace a keyword argument?

[tleonhardt]

@kmvanbrunt If you go with that approach, I think it would be better to make the Namespace a keyword-only argument

[teto]

Couldn't the choice between choices_function, choices_method be automated via inspect.ismethod ?

[kmvanbrunt]

inspect.ismethod() only returns True for bound methods, which means it must be bound to an object instance. No CLI object instances exist when creating the argparse parsers.

It is possible to inspect the function's signature and see if first parameter is called self. That technique also works on functools.partial objects. I support it's not an unreasonable condition to place on a developer that they don't rename self to something else.

@tleonhardt thoughts?

[tleonhardt]

I'd rather have separate options for functions and methods and force developers to call the correct one than make an assumption that developers didn't call self something else.

[kmvanbrunt]

I ended up using the inspect.signature() approach to determine when to pass parsed_args to a choices/completer function. It now only passes it if the function has an argument called parsed_args.

[kmvanbrunt]

@tleonhardt @anselor @teto As part of the Namespace change, I want to create a CompletionError class. It's purpose is for when a user wants to print an error instead of completion results.

This could handle cases like

• An error occurred retrieving completion results (e.g. database error)
• A prior token which determines the completion results is invalid

Do you think CompletionError should be an exception that is raised with the message to print? Or should it just be something the choices/completer function returns instead of results? Sort of how CompletionItem is used.

[teto]

I just noticed the change on parsed_args (was wondering why my partial function calls would fail xD). I liked the idea of just passing the namespace better, even if it means ignoring it. Now I need to remember the name parsed_args.

I wonder if it's an error in my code or in cmd2's but the pcap argument has type str (it is added like this

    def add_pcap(self, name, **kwargs):
        params = {
            'action': LoadSinglePcap,
            'help': 'Pcap file',
            'completer_method': cmd2.Cmd.path_complete
        }
        params.update(**kwargs)
        load_pcap = self.add_argument(name, type=str, **params)
        return load_pcap

) yet in the parsed_args namespace pcap is a list (pcap=['examples/client_2_filtered.pcapng']) instead of a string: parsed_args Namespace(__parser__=MpTcpAnalyzerParser(prog='plot tcp_attr', usage=None, description=None, formatter_class=<class 'cmd2.argparse_custom.Cmd2HelpFormatter'>, conflict_handler='error', add_help=False), pcap=['examples/client_2_filtered.pcapng']).

Might be best to return, Exception force a specific pattern. No strong opinion though.

[teto]

I htink the problem comes from cmd2 since when the parser parses the whole command, it generates a correct namespace with pcap of type string

[anselor]

@kmvanbrunt @teto @tleonhardt

I like the latest change that now doesn't require changing all existing completer functions to take kwargs. It seems to me that the need to process prior arguments when completing a current argument, while important, is more of a specialized case. It doesn't make sense to force that on all completion functions because some of them need this information. It seems reasonable to me to only pass the additional arguments if the completer function declares it accepts **kwargs or passed_args.

Also, I like the idea of raising a CompletionError when something goes wrong. Returning a special type of CompletionItem that's treated like an error feels wrong to me.

[kmvanbrunt]

Will passing parsed_args to a function that accepts **kwargs but does not explicitly have an argument called parsed_args lead to any unforeseen problems? I worry about a case in which the function in question raises an unexpected keyword argument because it examines the list of kwargs passed to it.

[kmvanbrunt]

@teto Refer to items 2 and 3 of my earlier post where I asked for opinions on the design

So I'd like opinions on these design choices

2. All members of the Namespace are lists, even if a particular argument expects only 1 token
3. Since AutoCompleter is for tab completion, it does not convert the tokens to their actual argument types or validate their values. All tokens are stored in the Namespace as the raw strings provided on the command line. It is up to the developer to determine if the user entered the correct argument type (e.g. int) and validate their values.

[anselor]

@kmvanbrunt **kwargs should accept all keyword arguments. The implementer can then search for the keywords they want. I think it's understood with that kind of interface that you'll be dealing with variable inputs and write your code accordingly.

[teto]

@kmvanbrunt sorry I missed that. I understand that we want to pass a Namespace in case the user passed a custom namespace (which I do) but then changing the meaning of the passed arguments looks not very clean (though already useful).

I wonder if we couldn't instead call parse_args() iteratively as we request for completion (or call the actions directly). Then the namespace would get correctly filled on the fly. I can imagine some edge cases that would require either user attention (like for args that can be repeated several times).

Another advantage I see about this is that you don't have to wait for the full command line to notify the user of errors in his argument. It could be an experimental cmd2.argparse_custom.Cmd2ArgumentParser derivative parser until we are comfortable enough to make it a default ?

[kmvanbrunt]

@teto Perhaps I should rename parsed_args to arg_tokens to make the intention more clear. The purpose of this feature is to organize the command-line tokens according to the argparse action they belong to. It is then up to the developer to determine the validity of a command-line token in their choices/completer function, possibly using parsed_args.__parser__ for advanced cases. Making arg_tokens a dictionary instead of a Namespace might also make its intention more clear.

AutoCompleter is the cmd2 class that performs tab-completion for argparse-based commands. It is custom code and not derived from any argparse class, so it is completely distinct from Cmd2ArgumentParser. The purpose of AutoCompleter is only to assist in tab-completing tokens while a user types a command. It has a basic knowledge of argparse parsing rules and uses this to figure out what argparse action is being typed by the user. That's as far as AutoCompleter goes in terms of parsing.

Writing a version of parse_args that can be called iteratively is way beyond the scope of AutoCompleter. Calling actions directly is best left to the developer who has knowledge of their intended function and possible need for state information.

You have a fairly advanced use case and are going to need to write some custom code to process the data we pass you.

[teto]

sry I didn't mean to nag you, thanks for the impressive work on autocompletion/parsing. I was thinking about how to make the UI consistant and it occured to me that a natural consequence would be for actions to be executed upon completion. I am already contributing to way too many projects but I hope the idea resonates with someone.

[teto]

I came up with something that worked on my example. I am not sure if I can generalize it yet. I modified cmd2 to pass the full line in parser.line, then I monkey patch the parser to be able to retrieve the namespace upon failure (something that might be worth requesting to argparse folks: please return the namespace upon error or in the exception) In my function responsible for completion, I run the parser with the command line written so far. I catch errors to retrieve a valid (but incomplete) namespace.

...
try:
        import types

        def new_error(self, msg):
            raise Exception("Matt: %s" % msg)
        parser.error = types.MethodType(new_error, parser)

        # copy of the argparse code https://github.com/python/cpython/blob/1f21eaa15e8a0d2b0f78d0e3f2b9e5b458eb0a70/Lib/argparse.py#L1796
        namespace, args = parser._parse_known_args(temp, namespace)
        if hasattr(namespace, _UNRECOGNIZED_ARGS_ATTR):
            args.extend(getattr(namespace, _UNRECOGNIZED_ARGS_ATTR))
            delattr(namespace, _UNRECOGNIZED_ARGS_ATTR)
    except argparse.ArgumentError as e:
        import sys
        err = sys.exc_info()[1]
        print(str(err))
        print("PARSING FAILED\nNamespace", namespace)
        print("Exception %s" % e)

...

thus even when the parser fail, I can still retreive the namespace with the loaded actions. I modified my actions so that they become idempotent: also if they detect the pcap is alreadly loaded in the namespace, they won't load it again, thus keeping the completion snappy. Once I get sthg a tad cleaner, I will share but it already works.

[kmvanbrunt]

@tleonhardt @anselor @teto I realize this is lighter weight than @teto wants, but how does everyone feel about the current state of this code? I feel it give developers enough information to work with. I still need to add the CompletionError exception, but I want consensus before moving on.

Consider these points:

1. All members of the Namespace are lists, even if a particular argument expects only 1 token
2. Since AutoCompleter is for tab completion, it does not convert the tokens to their actual argument types or validate their values. All tokens are stored in the Namespace as the raw strings provided on the command line. It is up to the developer to determine if the user entered the correct argument type (e.g. int) and validate their values.
3. Since the tokens are not validated or run through their respective action, should I rename parsed_args to arg_tokens to make the intention more clear?

[anselor]

Yeah, I think this is an acceptable path. I think renaming it to reduce confusion sounds good as well.

[tleonhardt]

Everything you mention there above @kmvanbrunt sounds good to me

[teto]

and maybe you could pass the full command line as well, in a line for instance, or maybe even in place of the tokens, if it's easy enough to build the tokens from the command (and the other direction. i.e., rebuilding the command line from the parsed_args looks much harder). In the end I want to run the parser with the command line so I don't even need the parsed_args .

[kmvanbrunt]

@teto I think this Namespace feature isn't intended for your use case. What you want can already be done using a completer function that takes a parser.

A completer function has the following signature: completer(text, line, begidx, endidx)

• text: the string prefix we are attempting to match (all matches must begin with it)
• line: the current input line with leading whitespace removed
• begidx: the beginning index of the prefix text
• endidx: the ending index of the prefix text

You can add an additional argument called parser so you completer would be:

from cmd2 import utils

def my_completer(self, text: str, line: str, begidx: int, endidx: int, parser: Cmd2ArgumentParser):
    # Get command line tokens through the one being completed
    tokens, _ = self.tokens_for_completion(line, begidx, endidx)

    # Parse the command line tokens
    args = parser.parse_args(tokens)

    # Do logic to determine what data set we are tab completing against
    match_against = ...

    # Do the tab completion
    return utils.basic_complete(text, line, begidx, endidx, match_against)

Then define your argument this way

my_parser.add_argument('my_action',
                       completer_method=functools.partial(my_completer, parser=my_parser)