Skip to content

Support VerseRefs with segments across versifications#415

Open
Enkidu93 wants to merge 1 commit into
masterfrom
verse_segments_across_versifications
Open

Support VerseRefs with segments across versifications#415
Enkidu93 wants to merge 1 commit into
masterfrom
verse_segments_across_versifications

Conversation

@Enkidu93
Copy link
Copy Markdown
Collaborator

@Enkidu93 Enkidu93 commented May 11, 2026

Includes:

  • Add test to cover alignment of verses across versifications with segments (e.g., a,b,c...).
  • Strip segments when converting/comparing refs with verse segments across versifications because VerseRef does not support it properly

This 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 extension VerseRef.CompareTo()) exhibit very odd behavior when working with verse segments. Basically,

var vrS = new VerseRef("NUM 17:1a", ScrVers.Original);
var vr = new VerseRef("NUM 17:1", ScrVers.Original);

vrS.ChangeVersification(ScrVers.English);
vr.ChangeVersification(ScrVers.English);

// Now
// vr == new VerseRef("NUM 16:36", ScrVers.English) :heavy_check_mark: 
// but 
// vrS == new VerseRef("NUM 17:1a", ScrVers.English) :exploding_head: 

This change is Reviewable

…gments across versifications because VerseRef does not support it properly
@Enkidu93 Enkidu93 requested a review from ddaspit May 11, 2026 21:17
Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Enkidu93
Copy link
Copy Markdown
Collaborator Author

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 SIL.Scripture to see what's really going on. From what I could tell, the look-ups for mappings between verses across versifications were exact such that if a verse had a verse segment, it didn't have a mapping to the other versification even if the verse without a verse segment did. I don't think there's any other explicit handling of verses with segments. My strategy here is to just strip the segments before converting since there isn't a guaranteed way to map those to another versification anyways.

Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Enkidu93
Copy link
Copy Markdown
Collaborator Author

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.

Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants