Glad you found the issue with it, and many thanks for reporting and offering to contribute.

I'd probably name it SEGMENT_ENDING_CHARACTER that accepts any character including '' (which is equivalent to false in the current case).

Since we'll be accepting only a single-character string, we need to add a validation for the same.

Regarding the segmentToString change, while I agree moving the ending character logic to it, this might be a breaking change for those who're already calling the method? May be we should do it in the next major release.

In the PR please update Readme, and mention SEGMENT_ENDING_BAR as deprecated, and that it'll be removed in the next major release.

SEGMENT_ENDING_BAR replacement

I don't think setting a separate character is the correct route because it has to be the same as the field separator to work with HL7 spec. Is this wrong?

Maybe a shorter version would be TRAILING_FIELD_SEPARATOR if that's a concern.

segmentToString() change

I agree with this possibly being a breaking change.

Agreed that using a separate character than field-separators violates hl7 standard. Also, we already have a SEGMENT_SEPARATOR global.

I'd use the word "segment" and avoid "field" as we're essentially separating the segments with this. May be WITH_SEGMENT_ENDING_BAR? That should make it clear that it's meant for segment-endings, and accepts true/false value?

I think we need to stick with the 'field' term to relate it to the correct separator character. Also, WITH_SEGMENT_ENDING_BAR goes back to the original problem of defining using bar which is incorrect. It is the field separator.

I did check the spec to see if there was already existing terminology that we could use. But I just found that it actually violates the definition for the field separator.

Separates two adjacent data fields within a segment. It also separates the segment ID from the first data field in each segment.

The the spec doesn't allow for it, although it sounds like quite a few companies use it, so we need to support it. I'm not sure if this is something left over from v1 but it shouldn't have been used by anyone implementing v2. 🫤

After all of that, I think TRAILING_FIELD_SEPARATOR=t/f is the most descriptive of what is actually happening; we are allowing the use of a trailing field separator.

That makes sense, but since we're talking about the presence of the separator at the end of a segment, wouldn't TRAILING_FIELD_SEPARATOR be a little confusing? As in, it doesn't say "what" it trails. May be SEGMENT_ENDING_FIELD_SEPARATOR as you had suggested earlier?

Also, since this field does not actually contain the separator but rather true/false, probably WITH_SEGMENT_ENDING_FIELD_SEPARATOR will be a better name? A little on the bigger side, but at least it conveys the intent.