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

Conversion support to use RecurrenceRule with native code or export it (JSON) #30

Closed
wants to merge 2 commits into from
Closed

Conversion support to use RecurrenceRule with native code or export it (JSON) #30

wants to merge 2 commits into from

Conversation

GoldenSoju
Copy link

Reason

To be able to use RecurrenceRules with native code (e.g. Method Channel) or to export it (e.g. to server), minimal support for to and from JSON conversion is needed.

Changes

  • Added fromJson/toJson functions for RecurrenceRule and ByWeekDayEntry
  • Added tests for fromJson/toJson (json_compatibility_test.dart)
  • Added util functions intToFrequency/frequencyToInt
  • Make frequencyFromString public
  • Updated dependencies in pubspec.yaml
    • meta: ^1.3.0 -> ^1.7.0
    • test: ^1.16.8 -> ^1.20.1

Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

@JonasWanke
Copy link
Owner

JonasWanke commented Jan 7, 2022

I'm not yet sure whether the proposed solution is the best approach because the JSON format does not seem to be standardized. If you want to pass a RecurrenceRule to native code or a server, you can already do that using the standardized string format: RecurrenceRuleStringCodec and the convenience functions RecurrenceRule.fromString(…) and recurrenceRule.toString(…) convert from/to iCalendar/RFC 5545-compliant strings. Maybe this already covers your use cases?

If conversion from/to JSON is really necessary, I'd prefer to support a standardized format like jCal (#3). It's similar to the JSON format in this PR, but, e.g., encodes frequencies and ByWeekDayEntrys as strings.

@GoldenSoju
Copy link
Author

The RecurrenceRule String support is great, but it leaves you with the hassle to encode/decode it on native side to use it.
jCal support looks promising, but that would mean that decoding/encoding for ByWeekDayEntries would be necessary.
Instead of having to chose one, how about implementing support for both for a wider range of use cases?
An 'unstandardized' JSON/Map format, and a standardized jCAL format.

@JonasWanke
Copy link
Owner

I don't think that supporting both a standardized and a non-standardized JSON format would benefit users of this package, but rather create confusion and more code to maintain. If there are no native libraries for en-/decoding RRULE strings or jCal, I suggest doing either of the following:

  • Create a native library supporting a standardized format. En-/decoding ByWeekDayEntrys doesn't require much code:
    String convert(ByWeekDayEntry input) =>
    '${input.occurrence ?? ''}${_weekDayToString(input.day)}';
    ByWeekDayEntry convert(String input) {
    final match =
    RegExp('(?:(\\+|-)?([0-9]{1,2}))?([A-Za-z]{2})\$').matchAsPrefix(input);
    if (match == null) {
    throw FormatException('Cannot parse $input');
    }
    int? occurrence;
    final numberMatch = match.group(2);
    if (numberMatch != null) {
    occurrence = int.parse(numberMatch);
    if (1 > occurrence || occurrence > 53) {
    throw FormatException('Value must be in range ±1–53');
    }
    if (match.group(1) == '-') occurrence = -occurrence;
    }
    final dayMatch = match.group(3);
    final weekDay = _weekDayFromString(dayMatch!);
    if (weekDay == null) {
    throw FormatException(
    'Invalid day of week: "$dayMatch"; allowed values are '
    '${recurWeekDayValues.keys.join(',')}.',
    );
    }
    return ByWeekDayEntry(weekDay, occurrence);
    }
  • Implement a non-standardized format in your project, tailored to its use-cases (simple de-/serialization, etc.)

@GoldenSoju
Copy link
Author

Thanks for you input. I am sure I can work with the jCAL format.
I am wondering if you have plans to implement it in the near future? I saw the related issue dates back to 2020.

@JonasWanke
Copy link
Owner

I created it back when I created this package to note some future ideas but didn't need it yet. It's now implemented and published as v0.2.6.

@GoldenSoju
Copy link
Author

Wow great! Thanks a lot for your awesome and fast support!

@GoldenSoju GoldenSoju closed this Jan 19, 2022
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