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 parsing an rrule string without frequency #339

Merged

Conversation

martinjlowm
Copy link
Contributor

@martinjlowm martinjlowm commented Apr 28, 2019

Prior to this PR, rrulestr fails to parse a simple RRule of "DTSTART:X".

  • As an RRule, rrulevals becomes empty and groomRruleOptions spreads an undefined value.
  • As an RRuleSet, dtstart is parsed and only ever used if the parsed RRule string contains a frequency component ("RRULE:Y").

This PR stores the parsed DTSTART as a field on an RRuleSet which may be changed using the new dtstart-setter. The DTSTART component is only printed if RRuleSet._dtstart is set and RRuleSet._rrule is an empty array to avoid conflicting outputs.

  • Merged in or rebased on the latest master commit
  • Linked to an existing bug or issue describing the bug or feature you're
    addressing (- I hope the description here is enough)
  • Written one or more tests showing that your change works as advertised
  • Run yarn build to rebuild the dist/ files

edit:

FYI, with the changes in e1146a3, RRuleSet.tzid() and RRuleSet.dtstart() lose their type signatures. These may be recovered by upgrading TypeScript to 3.2+ with the strictBindCallApply-option.

@codecov-io
Copy link

codecov-io commented Apr 28, 2019

Codecov Report

Merging #339 into master will increase coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage   89.92%   90.01%   +0.09%     
==========================================
  Files          28       28              
  Lines        1935     1953      +18     
  Branches      581      583       +2     
==========================================
+ Hits         1740     1758      +18     
  Misses        195      195
Impacted Files Coverage Δ
src/rruleset.ts 100% <100%> (ø) ⬆️
src/rrulestr.ts 92.56% <100%> (+0.06%) ⬆️
src/iter/poslist.ts 100% <0%> (ø) ⬆️
src/masks.ts 100% <0%> (ø) ⬆️

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 211d215...e1146a3. Read the comment docs.

@davidgoli
Copy link
Collaborator

Hi! What's the use case for this that isn't supported by RDATE?

@martinjlowm
Copy link
Contributor Author

Hi :)

To support something as simple as rrulestr('DTSTART:19970902T090000Z') and to recover the original string as follows

rrulestr('DTSTART:19970902T090000Z', { forceset: true }).toString() === 'DTSTART:19970902T090000Z'

Copy link
Collaborator

@davidgoli davidgoli left a comment

Choose a reason for hiding this comment

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

This seems to work OK without altering existing behavior. Thanks for the submission!

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