Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for repeated flags/options #21 #44

Conversation

goofinator
Copy link
Contributor

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
Copy link
Contributor Author

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

@akamensky
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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

@akamensky akamensky merged commit ae96b42 into akamensky:master Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants