Support VerseRefs with segments across versifications#415
Conversation
…gments across versifications because VerseRef does not support it properly
ddaspit
left a comment
There was a problem hiding this comment.
Did you look into SIL.Scripture to see if segments are being handled in any specific way by the ChangeVersification method?
@ddaspit reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.
I did take a look - although it's difficult sometimes to dig through all the layers in |
ddaspit
left a comment
There was a problem hiding this comment.
I don't love entirely stripping off the segments. Maybe we could add them back in after changing the versification?
@ddaspit made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.
I did consider that. Is there a reliable way to do so? I don't think we're guaranteed that the segmentation of a verse in one versification is the same in another, are we? We're definitely not guaranteed that the segment naming is the same (i.e., in some versions, it's not a,b,c - I've seen other characters used, I'm pretty sure), but I'm not even sure we're guaranteed that they really refer to the same chunk of text. |
ddaspit
left a comment
There was a problem hiding this comment.
There probably isn't a 100% reliable way to transfer the segments, but I feel like we have to do it anyway. In most cases, it would be more correct to transfer the segment, rather than strip it. Also, there are cases where the versification has mappings specifically for verse segments. We don't want to break those mappings, so we need to detect that case and use the normal change versification logic.
@ddaspit made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved.
Includes:
VerseRefdoes not support it properlyThis fixes an issue Michael recently brought up on slack: https://sil-language-software.slack.com/archives/C06A8JFBU5S/p1778328868601589. (I can create a retroactive issue if you would like). I decided to prioritize this because depending on the problem, it could be causing catastrophic misalignment - especially for silnlp - and it was in this case.
The root of this problem is that
ChangeVersification()(and by extensionVerseRef.CompareTo()) exhibit very odd behavior when working with verse segments. Basically,This change is