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

💥 SequenceSet input validation for Set, Array, and enumerables #319

Merged
merged 3 commits into from
Sep 13, 2024

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented Sep 13, 2024

Rather than allow any object with #each as a potential input, this narrows down the API to only allow Set and Array objects:

  • Nested arrays will still be deeply traversed, the same as before. This is primarily for backward compatibility with MessageSet.
  • Sets may only contain integers, without nesting. This behaves more consistently with #to_set. With this change, the round trip of SequenceSet.new(set).to_set == set should be true.
  • No other enumerable types will be converted, unless they implement #to_sequence_set. This way, we won't accidentally attempt to convert hashes with string keys, etc.

Although I believe the impact will be negligible, this is a breaking change. SequenceSet is currently documented as "experimental" except for two methods, and this change affects one of those methods: SequenceSet[*inputs]. So I'd like this change to be in the v0.5.0 release.

This can simplify some calling code.
Each member of the set should be an integer, a range, or a sequence-set
formatted string.  SequenceSet should not allow Set inputs to be nested.
Other enumerable types are no longer valid inputs.
@nevans nevans added this to the v0.5 milestone Sep 13, 2024
@nevans nevans merged commit 5907cd9 into master Sep 13, 2024
18 checks passed
@nevans nevans deleted the sequence-set-input-validation branch September 13, 2024 15:04
nevans added a commit to nevans/net-imap that referenced this pull request Sep 26, 2024
nevans added a commit that referenced this pull request Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant