Add repetition handling

[diazruy]

Breaking change.

When a field (other than MSH.2, encoding characters) contains the repetition delimiter (~), the field accessor now returns an Array with each value in the repetition. E.g.

pid = HL7::Message::Segment::PID.new('PID|||123~456')
pid.patient_id_list # => ['123', '456']

When an Array is assigned to a field, the values are concatenated with the repetition delimiter when converting back to String/HL7

Values assigned to fields are no longer immediately cast into Strings, allowing you to append more values to a repetition. E.g.

pid = HL7::Message::Segment::PID.new
pid.patient_id_list = ['123', '456']
pid.patient_id_list << '789'
pid.to_s # => 'PID|||123~456~789'

[Stratus3D]

Hi @diazruy, thanks for the PR! I've not be actively using this project for many years and only occasionally check up on it. I'll wait and see if a more active maintainer is available to review your changes.

@ruby-hl7/maintainers @mogox @mitch-lindsay can someone review this PR?

[mitch-lindsay]

Sorry for the delay, didn't see this PR till now. I like the spirit of this change and the implementation looks clean. My only concern is the breaking change. Likely any consumers of this gem don't have specs to cover repetition so unless they are paying close attention to the changeset, the breaking change will likely hit them in production.
I have a proposed solution though. I put together a PR to add configurability to the gem. #122. If this looks good and reasonable, we can config drive this change so users that want it can enable it but default it off.

[diazruy]

Does the gem not follow Semantic Versioning? A change in the major version should signal to the consumers of the gem that this is a breaking change, would it not? Better than adding conditionals all over the place whose interactions could be hard to predict (similar to an explosion in feature flags). I guess one possibility would be to introduce it as a feature flagged thing (essentially what the configuration is doing) which will be enforced starting at v2.x or something like that.

[mitch-lindsay]

Ideally we would follow semantic versioning but we have only had one major version for the very long lifecycle of this project. There's just not enough active development or large refactors going on to warrant major version changes. I propose the configuration approach only as a means of rolling this out in a less disruptive way; very much like a feature flag. But ultimately this isn't a hill that's worth dieing on. If the consensus is that we release this as a major version update with a breaking change, I'm ok with that too.

[diazruy]

No, I get it... feature flagging it seems like a reasonable intermediate step.

[mitch-lindsay]

Configuration changes have been integration into master. Feel free to integrate that change in to feature flag your change so we can release now. Thanks!

[diazruy]

@mitch-lindsay sorry it took this long, but I finally got around to integrating the feature flag. I actually ended up choosing to go with two flags since it seemed counterintuitive to have an enable_repetitions flag toggling whether methods called .to_s or not. I thought adding a separate feature flag for preserve_data_types whose presence is enforced when enabling the enable_repetitions flag was a better route. Let me know your thoughts.