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 RRuleSet::set_from_string to support loading rules without DTSTART #105

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

sciyoshi
Copy link
Contributor

This PR adds a new method to RRuleSet called set_from_string which is similar to the FromStr implementation but allows passing rules without a DTSTART line. This is useful in particular when working with Google's API, which provides the rrule without the DTSTART:

image

As it stands today, because the entire parser module is private, you would need to format a DTSTART line and add it to the rrule (only for it to be parsed again).

I pulled out the common logic in from_str to a private set_from_content_lines method to avoid duplication.

@fmeringdal fmeringdal self-requested a review March 26, 2024 09:58
@fmeringdal
Copy link
Owner

The way this will be used is like this?

RRuleSet::new(date_time)
     .set_from_string(google_rrule_string)

@sciyoshi
Copy link
Contributor Author

sciyoshi commented Apr 1, 2024

@fmeringdal correct. The name set_from_string could maybe be improved but that's how we plan to use it.

@sciyoshi sciyoshi force-pushed the rruleset-set-from-string branch from ecf5076 to e039297 Compare April 1, 2024 17:42
Copy link
Owner

@fmeringdal fmeringdal left a comment

Choose a reason for hiding this comment

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

Can you also add an entry to the changelog for this new function and a line about the motivating use-case for it?

rrule/src/core/rruleset.rs Outdated Show resolved Hide resolved
@fmeringdal
Copy link
Owner

@fmeringdal correct. The name set_from_string could maybe be improved but that's how we plan to use it.

It is good enough, naming is hard :)

@sciyoshi sciyoshi force-pushed the rruleset-set-from-string branch from d65f491 to 1c506ab Compare April 1, 2024 17:52
@sciyoshi
Copy link
Contributor Author

sciyoshi commented Apr 1, 2024

Can you also add an entry to the changelog for this new function and a line about the motivating use-case for it?

Done 👍

@fmeringdal
Copy link
Owner

Thanks again. I am aiming for a release later this week that will include these changes.

@fmeringdal fmeringdal merged commit 307931c into fmeringdal:main Apr 1, 2024
8 checks passed
tomquist added a commit to tomquist/rrule-rust that referenced this pull request May 3, 2024
This allows creating an RRuleSet without a DTSTART
(see fmeringdal/rust-rrule#105)
lsndr pushed a commit to lsndr/rrule-rust that referenced this pull request May 4, 2024
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