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

Fix bug with option key ordering. #527

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

DarryQueen
Copy link
Contributor

The logic for isFullyConvertible is sensitive to the key ordering of origOptions because it short-circuits once it sees one of the ignored keys. I imagine this is a bug; the ignored keys should just be skipped over in the loop.

@davidgoli
Copy link
Collaborator

@DarryQueen I've merged your other PR [#528] but now this one has conflicts. Please resolve them and I'll merge.

@DarryQueen
Copy link
Contributor Author

Done, merge conflicts resolved.

import { optionsToString } from '../src/optionstostring'
import { DateFormatter } from '../src/nlp/totext'
import { datetime } from './lib/utils'

const texts = [
['Every day', 'RRULE:FREQ=DAILY'],
['Every day at 10, 12 and 17', 'RRULE:FREQ=DAILY;BYHOUR=10,12,17'],
[
'Every week on Sunday at 10, 12 and 17',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this and moved it down in with "~ approximate" because it is technically not fully supported; byhour is not fully convertible for weekly.

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.

3 participants