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

Indexed tuple validation #310

Merged
merged 2 commits into from
Sep 3, 2019
Merged

Conversation

phlptp
Copy link
Collaborator

@phlptp phlptp commented Aug 17, 2019

If merged this PR will remove the restrictions on tuple size, and add an index to validators to allow them to apply to specific components of a tuple type or vector type.

it should address #296

@codecov
Copy link

codecov bot commented Aug 18, 2019

Codecov Report

Merging #310 into master will decrease coverage by 0.03%.
The diff coverage is 97.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #310      +/-   ##
==========================================
- Coverage     100%   99.96%   -0.04%     
==========================================
  Files          12       12              
  Lines        3002     2995       -7     
==========================================
- Hits         3002     2994       -8     
- Misses          0        1       +1
Impacted Files Coverage Δ
include/CLI/App.hpp 100% <100%> (ø) ⬆️
include/CLI/TypeTools.hpp 100% <100%> (ø) ⬆️
include/CLI/Validators.hpp 100% <100%> (ø) ⬆️
include/CLI/Option.hpp 99.76% <92.85%> (-0.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 127f538...9c663e0. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 18, 2019

Codecov Report

Merging #310 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #310   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        3017   3028   +11     
=====================================
+ Hits         3017   3028   +11
Impacted Files Coverage Δ
include/CLI/Option.hpp 100% <100%> (ø) ⬆️
include/CLI/App.hpp 100% <100%> (ø) ⬆️
include/CLI/TypeTools.hpp 100% <100%> (ø) ⬆️
include/CLI/Validators.hpp 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06ab2d0...aa85fa2. Read the comment docs.

remove restrictions on tuple size, and add some additional tests and modified documentation

fix some issues with the negative number check

add some test for indexed validation on tuple

allow specific validators for specific elements in a type with multiple values, or to just apply to the last given argument
@phlptp phlptp force-pushed the indexed_tuple_validation branch from f44a065 to b7b2bb7 Compare August 19, 2019 00:20
if(detail::split_short(current, dummy1, dummy2)) {
if(dummy1[0] >= '0' && dummy1[0] <= '9') {
if(get_option_no_throw(std::string{'-', dummy1[0]}) == nullptr) {
return detail::Classifier::NONE;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a bug mentioned earlier that -N can match when it should be a number, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it should fix the bug mentioned here

include/CLI/Validators.hpp Outdated Show resolved Hide resolved
@@ -113,7 +115,13 @@ class Validator {
non_modifying_ = no_modify;
return *this;
}

/// Specify the application index of a validator
Validator &application_index(int app_index) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could a const copying version work? This currently cannot be applied inline without changing the validator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the same is true for any of the validator modification functions. active, name,description,non_modifying. I think we would want them all to have the same paradigm.

It would add a number of overloads but might make the calling a little easier for many of the const objects, which require an extra CLI::Validator( ) around them. I will make that change tomorrow morning.

@phlptp
Copy link
Collaborator Author

phlptp commented Aug 22, 2019

not sure why codacy isn't analyzing things?

@henryiii henryiii merged commit fc6e1c7 into CLIUtils:master Sep 3, 2019
@henryiii henryiii deleted the indexed_tuple_validation branch September 3, 2019 00:58
@henryiii henryiii added this to the v1.9 milestone Dec 31, 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.

2 participants