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

Allow empty default values - solves #19 #20

Merged
merged 1 commit into from
Jan 26, 2016

Conversation

vladimir-mencl-eresearch
Copy link
Contributor

Allow empty default values:

  • Change regexp part for parsing default values to allow zero-or-more (so far
    was one-or-more); keep the prefer-less flag
  • Make the regexp part for default value syntax a capturing group: (\:\-)
  • Add an additional "separator" parameter to the capture function to see
    whether the default value separator was used
  • Add a corresponding constant DefaultValueSyntax with the separator value (":-")
  • Change the logic for testing whether a default value was provided to check
    whether the separator is non-empty.
  • TEST: adjust TestCapture accordingly to also handle separator values
  • TEST: test the newly supported empty-default-value syntax in TestFullParseDefaults function

Allow empty default values:

* Change regexp part for parsing default values to allow zero-or-more (so far
  was one-or-more); keep the prefer-less flag
* Make the regexp part for default value syntax a capturing group: (\:\-)
* Add an additional "separator" parameter to the capture function to see
  whether the default value separator was used
* Add a corresponding constant DefaultValueSyntax with the separator value (":-")
* Change the logic for testing whether a default value was provided to check
  whether the separator is non-empty.
* TEST: adjust TestCapture accordingly to also handle separator values
* TEST: test the newly supported empty-default-value syntax in TestFullParseDefaults function
@yawn
Copy link
Contributor

yawn commented Jan 25, 2016

Looks pretty good - please allow me a couple of days before merging. A new release should be ready until the end of the week!

@yawn
Copy link
Contributor

yawn commented Jan 26, 2016

LGTM!

yawn added a commit that referenced this pull request Jan 26, 2016
@yawn yawn merged commit e58fe8a into kreuzwerker:master Jan 26, 2016
@vladimir-mencl-eresearch
Copy link
Contributor Author

Hi,

Thanks for merging my pull request.

Could you please also make a release to fully close this off?

Many thanks in advance!

Cheers,
Vlad

@yawn
Copy link
Contributor

yawn commented Feb 2, 2016

Yeah, I was doing some refactoring to finally close #4 but did not manage to complete it. That's basically the holdup. If I can't finish this today I'll do an intermediate, including the work in #21.

@vladimir-mencl-eresearch
Copy link
Contributor Author

Hi, I didn't mean to rush you! It's not urgent for me to have the release now - I was just probing if it's not stalled. All fine if you need some more time - can wait without doing an intermediate release.

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