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 termination char to every segment, even the last one. #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

micred
Copy link

@micred micred commented Jul 21, 2016

HL7 Inspector notifies:

Warning: Last segment have no segment termination char 0x0d !

The to_hl7 - and then to_mllp - methods does not terminate the last segment.

HL7 specification says that every segment should be terminated, even the last one.
With this simple PR every segment is terminated.

@mogox
Copy link
Member

mogox commented Jul 26, 2016

Hey @micred, this breaks a lot of test. Would you mind taking a look at that? My guess is that we would be needing to update the test if we decide to go with this change.

Maybe we need to setup a different delimiter for test?

@micred
Copy link
Author

micred commented Jul 27, 2016

Test are comparing to the wrong terminating segments:
https://travis-ci.org/ruby-hl7/ruby-hl7/jobs/146315226

The is also a \n that should not be present, do you agree?
If you'd like, I can provide a fix for newline and correct the termination in the tests.

@mitch-lindsay
Copy link
Collaborator

This will generate HL7 that the gem won't be able to parse. #3 The parser has trouble with a trailing newline on the last segment, treating it like a new, empty segment and throwing a HL7::EmptySegmentNotAllowed error. Not saying you shouldn't do this but if you're going to address one, it would be great to address the other.

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