Support for postional only args in dynamic library api

[aaltat]

Fixes #4660

summary:

copilot:summary

[Snooz82]

Walk Through

copilot:walkthrough

[pekkaklarck]

Looks very good but looking for the actual / value should be moved to DynamicArgumentParser from the common base class. I also need to look at tests and docs in a bit more detail later.

[pekkaklarck]

ArgumentSpecParser is a common base class for UserKeywordArgumentParser and DynamicArgumentParser. Checking for actual values, in this case /, should be done in them. I believe it would be best to add new _is_positional_only_separator method and then use it in the for arg in argspec loop. The usage could probably be something like this (untested):

if self._is_positional_only_separator(arg):
    if positional_only_separator_seen:
        self._report_error(...)
    spec.positional_only, spec.positional_or_named = spec.positional_or_named, []
    positional_only_separator_seen = True

The positional_only_separator_seen would need to be initialized in the same place as named_only.

[aaltat]

I agree, this will result cleaner code. I made the changes.

[pekkaklarck]

I understand the idea to make /arg invalid syntax, but this check makes using / anywhere in the argument name invalid. That could be a good idea as well, but it's also to some extend backwards incompatible. Although the risk that this change would actually cause issues is small, it may still not be worth the effort. On the other hand, we could consider limiting argument names also otherwise, but that would require a separate issue. We can discuss all this further in Slack.

[aaltat]

I am happy to going either way, I do not feel strongly on the matter should /arg or a/rg be invalid or not. Lets discuss in Slack.

In latest commit, did leave it as is, but it is easy to change once agreement is reached.

[pekkaklarck]

Looks great! There are few minor things to be changed (for example, this shouldn't require Python 3.8) but I can take care of that myself.