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

Fixes #240. Edge case when dealing with multiple ranges for one parameter of the cron input #241

Merged
merged 3 commits into from Mar 10, 2021

Conversation

muhammadtalhas
Copy link
Contributor

I discussed the issue in #240

Essentially, if the g flag is removed from the replace function, we make sure to only replace the first instance of the expression to be replaced. This is okay since the while loop will run again if there's other ranges to be converted.

Since the original regex finds the first instance of the range, this replace will only replace the first instance.

Hence, fixes the edge case of replacing an incorrect sub-string, and also will not change the functionality.

@coveralls
Copy link

coveralls commented Aug 6, 2020

Pull Request Test Coverage Report for Build 362

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 361: 0.0%
Covered Lines: 274
Relevant Lines: 274

💛 - Coveralls

@merencia
Copy link
Member

@muhammadtalhas thanks for your PR and sorry about the delay!

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

3 participants