akamensky / argparse

Argparse for golang. Just because `flag` sucks
MIT License
604 stars 62 forks source link

Add support for repeated flags/options #21 #44

Closed goofinator closed 4 years ago

goofinator commented 4 years ago

Good day! First of all, thanks for the library. Periodically I use repeating flags to set the level of logging, so the problem of issue #21 is close to me.

The first commit adds this functionality. To do this, defined "type" Counter for the argument. In some places, bool type checking is replaced by checking the size of the argument, because this parameter is common for flag and counter. This allows you to use them together when combining arguments into one. func (o *arg) check(argument string) int - now returns integer, for counting of combined arguments. It's used by parse function to calculate the result for counter.

Second commit adds tests for counter.

The third commit performs testing and modifies the tests to check the arguments, combined into one for uniqueness, if necessary (also based on the results of the check function).

goofinator commented 4 years ago

It's look like some test configuration problem: $ sonar-scanner sonar-scanner: command not found The command "sonar-scanner" exited with 127.

akamensky commented 4 years ago

Hi, thanks for the PR. It fails because I configured SonarQube for code analysis, but seems something is not right with configuration. I will take a look at this soon.

Meantime, I agree this would be a good functionality for this library, the implementation looks good. However I think name of the method should be more descriptive (Counter might not be as explicit as it should be). Perhaps something like NumArgs or even FlagCounter (since it copies behavior of flags and just counts them instead) or something else that could be more descriptive. I think ideally users of this library don't need to use godoc to understand the point of given method.

goofinator commented 4 years ago

I agree. In python argparser, this kind of parameter is set through the action = "count". I called it Counter, because I hoped for memory, but I was mistaken. But both of these names are not entirely obvious. I also think that appropriate in the context of the verb (action=) "count" is not appropriate in the context of the name of the entity (function, object, etc.). I like your proposed FlagCounter. Let's use it. I will prepare the changes.

akamensky commented 4 years ago

I've fixed the travis-ci integration, could you please rebase to unblock this PR <3