Skip to content

Fix double mappings handling and expand trailing paragraph marker definition #521

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.

Sign up for GitHub

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

Merged
Enkidu93 merged 3 commits into from
Sep 28, 2026
Merged

Fix double mappings handling and expand trailing paragraph marker definition #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3086059
Fix double mappings handling and expand trailing paragraph marker def…
Enkidu93 Sep 24, 2026
cfc73f3
Limit duplicate verse suppression to the already-written verse
ddaspit Sep 25, 2026
52463f3
Drop paragraph markers that introduce a dropped verse
ddaspit Sep 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 27 additions & 13 deletions src/SIL.Machine/Corpora/ConvertUsfmVersificationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ string pubNumber
}

bool addedVerseText = false;
bool duplicateVerse = false;

string start = null;
for (int i = 0; i < verseRefs.Count; i++)
Expand All @@ -150,7 +151,8 @@ string pubNumber
)
{
AddTrailingTokens();
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
if (!duplicateVerse)
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
if (!addedVerseText && state.Index + 1 < state.Tokens.Count)
{
UsfmToken nextToken = state.Tokens[state.Index + 1];
Expand All @@ -164,12 +166,14 @@ string pubNumber
_tokens.Add(new UsfmToken(UsfmTokenType.Chapter, "c", "", "", verseRefs[i].Chapter));
_tokens.Add(new UsfmToken(UsfmTokenType.Paragraph, "nb", "", "", ""));
start = verseRefs[i].Verse;
duplicateVerse = false;
_prevVerseRef = verseRefs[i];
}
else if (_prevVerseRef.VerseNum + 1 != verseRefs[i].VerseNum)
{
AddTrailingTokens();
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
if (!duplicateVerse)
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
if (!addedVerseText && state.Index + 1 < state.Tokens.Count)
{
UsfmToken nextToken = state.Tokens[state.Index + 1];
Expand All @@ -181,16 +185,24 @@ string pubNumber
}
}
start = verseRefs[i].Verse;
duplicateVerse = false;
_prevVerseRef = verseRefs[i];
}
else
{
// The duplicated verse was already written, so the range starts after it.
if (duplicateVerse)
{
start = verseRefs[i].Verse;
duplicateVerse = false;
}
_prevVerseRef = verseRefs[i];
}
}
else
{
start = verseRefs[i].Verse;
duplicateVerse = verseRefs[i].Equals(_prevVerseRef);
_prevVerseRef = verseRefs[i];
}
verseRef = verseRefs[i];
Expand All @@ -200,14 +212,17 @@ string pubNumber
{
AddTrailingTokens();
string end = start != _prevVerseRef.Verse ? "-" + _prevVerseRef.Verse : "";
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
if (!duplicateVerse)
_tokens.Add(new UsfmToken(UsfmTokenType.Verse, "v", "", "", start + end));
_skip = false;
_insertChapterIndex = -1;
_prevVerseRef = verseRef;
}
else
{
_skip = true;
// Markers that introduce a dropped verse would otherwise be flushed at the next kept verse.
_trailingVerseTokens.Clear();
}
}

Expand All @@ -232,14 +247,7 @@ private void ProcessTokens(UsfmParserState state)
while (_verseBoundary + offset < state.Index)
{
UsfmToken token = state.Tokens[_verseBoundary + offset];
if (
IsPreservedTrailingParagraphMarker(
token,
_verseBoundary + offset + 1 < state.Tokens.Count
? state.Tokens[_verseBoundary + offset + 1]
: null
)
)
if (IsPreservedTrailingParagraphMarker(state.Tokens, _verseBoundary + offset))
{
inPreservedParagraph = true;
}
Expand Down Expand Up @@ -276,9 +284,15 @@ private void AddTrailingTokens()
_trailingVerseTokens.Clear();
}

private bool IsPreservedTrailingParagraphMarker(UsfmToken token, UsfmToken nextToken)
private bool IsPreservedTrailingParagraphMarker(IReadOnlyList<UsfmToken> tokens, int index)
{
return (token.Marker == "p" && nextToken != null && nextToken.Type == UsfmTokenType.Verse)
UsfmToken token = tokens[index];
UsfmToken nextToken = index + 1 >= tokens.Count ? null : tokens[index + 1];
return (
token.Type == UsfmTokenType.Paragraph
&& nextToken != null
&& (nextToken.Type == UsfmTokenType.Verse || IsPreservedTrailingParagraphMarker(tokens, index + 1))
)
|| token.Type == UsfmTokenType.Paragraph && TrailingParagraphMarkerPatterns.IsMatch(token.Marker);
}
}
Expand Down
166 changes: 164 additions & 2 deletions tests/SIL.Machine.Tests/Corpora/ConvertUsfmVersificationHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,41 @@ public void GetUsfm_HeadingIntroducingDroppedVerse_IsDropped()
AssertUsfmEquals(target, result);
}

[Test]
public void GetUsfm_ParagraphIntroducingDroppedVerse_IsDropped()
{
// Russian Orthodox vs. Original
// DAN 3:24-90 = DAG 3:24-90
// DAN 3:91-100 = DAN 3:24-33

string usfm =
@"\id DAN - Test
\c 3
\p
\v 1-23 Text
\v 24-50 Dropped text
\q1
\v 51-90 More dropped text
\p
\v 91-100 More text
";

string target = UpdateUsfm(
usfm,
sourceVersification: ScrVers.RussianOrthodox,
targetVersification: ScrVers.Original
);
string result =
@"\id DAN - Test
\c 3
\p
\v 1-23 Text
\p
\v 24-33 More text
";
AssertUsfmEquals(target, result);
}

[Test]
public void GetUsfm_DropVerseText()
{
Expand Down Expand Up @@ -805,6 +840,119 @@ public void GetUsfm_SameSourceAndTargetVersification()
AssertUsfmEquals(target, usfm);
}

[Test]
public void GetUsfm_PreceedingHeadingsNotMoved()
{
// English vs. Original
// JOL 2:27-28 = JOL 2:27-3:1

string usfm =
@"\id JOL
\c 2
\v 27 Then you will know that I am present in Israel
\q2 and that I am the LORD your God,
\q2 and there is no other.
\q1 My people will never again
\q2 be put to shame.
\s1 I Will Pour Out My Spirit
\r (Acts 2:14–36)
\q1
\v 28 And afterward, I will pour out My Spirit on all people.
\q2 Your sons and daughters will prophesy,
\q1 your old men will dream dreams,
\q2 your young men will see visions.
";

string target = UpdateUsfm(usfm, sourceVersification: ScrVers.English, targetVersification: ScrVers.Original);
string result =
@"\id JOL
\c 2
\v 27 Then you will know that I am present in Israel
\q2 and that I am the LORD your God,
\q2 and there is no other.
\q1 My people will never again
\q2 be put to shame.
\c 3
\s1 I Will Pour Out My Spirit
\r (Acts 2:14–36)
\q1
\v 1 And afterward, I will pour out My Spirit on all people.
\q2 Your sons and daughters will prophesy,
\q1 your old men will dream dreams,
\q2 your young men will see visions.
";
AssertUsfmEquals(target, result);
}

[Test]
public void GetUsfm_MergedVerses()
{
// Original vs. English
// PSA 51:1-3 = PSA 51:0-1

string usfm =
@"\id PSA
\c 51
\s1 Create in Me a Clean Heart, O God
\r (2 Samuel 12:1–12)
\p
\v 1 For the choirmaster. A Psalm of David.
\v 2 When Nathan the prophet came to him after his adultery with Bathsheba.
\b
\q1
\v 3 Have mercy on me, O God,
\q2 according to Your loving devotion;
\q1 according to Your great compassion,
\q2 blot out my transgressions.
";

string target = UpdateUsfm(usfm, sourceVersification: ScrVers.Original, targetVersification: ScrVers.English);
string result =
@"\id PSA
\c 51
\s1 Create in Me a Clean Heart, O God
\r (2 Samuel 12:1–12)
\p
\v 0 For the choirmaster. A Psalm of David. When Nathan the prophet came to him after his adultery with Bathsheba.
\b
\q1
\v 1 Have mercy on me, O God,
\q2 according to Your loving devotion;
\q1 according to Your great compassion,
\q2 blot out my transgressions.
";
AssertUsfmEquals(target, result);
}

[Test]
public void GetUsfm_MergedVerses_RangeExtendsPastMergedVerse()
{
// Original vs. Russian Orthodox
// LEV 14:55-56 = LEV 14:55

string usfm =
@"\id LEV
\c 14
\p
\v 55 for leprosy in a garment or in a house,
\v 56-57 for a swelling, a rash, or a spot, to determine when something is clean or unclean.
";

string target = UpdateUsfm(
usfm,
sourceVersification: ScrVers.Original,
targetVersification: ScrVers.RussianOrthodox
);
string result =
@"\id LEV
\c 14
\p
\v 55 for leprosy in a garment or in a house,
\v 56 for a swelling, a rash, or a spot, to determine when something is clean or unclean.
";
AssertUsfmEquals(target, result);
}

private static string UpdateUsfm(string source, ScrVers sourceVersification, ScrVers targetVersification)
{
source = source.Trim().ReplaceLineEndings("\r\n") + "\r\n";
Expand All @@ -826,10 +974,24 @@ private static void AssertUsfmEquals(string target, string truth)
Assert.That(target, Is.Not.Null);
string[] targetLines = target.Split('\n');
string[] truthLines = truth.Split('\n');
Assert.That(targetLines.Length, Is.EqualTo(truthLines.Length));
// Assert.That(targetLines.Length, Is.EqualTo(truthLines.Length));
for (int i = 0; i < truthLines.Length; i++)
{
Assert.That(targetLines[i].Trim(), Is.EqualTo(truthLines[i].Trim()), message: $"Line {i}");
Assert.That(
targetLines[i].Trim(),
Is.EqualTo(truthLines[i].Trim()),
message: string.Join(
"\n",
[
"Expected vs. \n\tActual",
.. truthLines
.Zip(targetLines)
.Select(pair =>
$"\n{pair.First}\n\t{(pair.First.Trim() != pair.Second.Trim() ? "***" : "")}{pair.Second}"
),
]
)
);
}
}
}
Loading