diff --git a/src/SIL.Machine/Corpora/PlaceMarkersUsfmUpdateBlockHandler.cs b/src/SIL.Machine/Corpora/PlaceMarkersUsfmUpdateBlockHandler.cs index 24db9a277..5ee415bf1 100644 --- a/src/SIL.Machine/Corpora/PlaceMarkersUsfmUpdateBlockHandler.cs +++ b/src/SIL.Machine/Corpora/PlaceMarkersUsfmUpdateBlockHandler.cs @@ -54,17 +54,7 @@ public UsfmUpdateBlock ProcessBlock(UsfmUpdateBlock block) elements.Count == 0 || alignmentInfo.Alignment.RowCount == 0 || alignmentInfo.Alignment.ColumnCount == 0 - || !elements.Any(e => - ( - e.Type == UsfmUpdateBlockElementType.Paragraph - && alignmentInfo.ParagraphBehavior == UpdateUsfmMarkerBehavior.Preserve - && e.Tokens.Count == 1 - ) - || ( - e.Type == UsfmUpdateBlockElementType.Style - && alignmentInfo.StyleBehavior == UpdateUsfmMarkerBehavior.Preserve - ) - ) + || !elements.Any(e => e.IsPlaceable(alignmentInfo.ParagraphBehavior, alignmentInfo.StyleBehavior)) ) { return block; @@ -99,10 +89,8 @@ public UsfmUpdateBlock ProcessBlock(UsfmUpdateBlock block) else if ( !( element.Type == UsfmUpdateBlockElementType.Embed - || ( - element.Type == UsfmUpdateBlockElementType.Text - && element.Tokens[0].ToUsfm().Trim().Length == 0 - ) + || element.Type == UsfmUpdateBlockElementType.Other + || (element.Type == UsfmUpdateBlockElementType.Text && element.GetText().Trim().Length == 0) ) ) { @@ -126,7 +114,7 @@ public UsfmUpdateBlock ProcessBlock(UsfmUpdateBlock block) { if (element.MarkedForRemoval) { - string text = element.Tokens[0].ToUsfm(); + string text = element.GetText(); // Track seen tokens while ( @@ -146,7 +134,7 @@ public UsfmUpdateBlock ProcessBlock(UsfmUpdateBlock block) } else { - targetSentence += element.Tokens[0].ToUsfm(); + targetSentence += element.GetText(); } } @@ -238,6 +226,10 @@ public UsfmUpdateBlock ProcessBlock(UsfmUpdateBlock block) toInsert.Sort((p1, p2) => p1.Index.CompareTo(p2.Index)); toInsert.AddRange(embedElements.Concat(endElements).Select(e => (targetSentence.Length, e))); + // In the case of unclosed markers, toInsert might be empty + if (toInsert.Count == 0) + return block; + // Construct new text tokens to put between markers // and reincorporate headers and empty end-of-verse paragraph markers if (toInsert[0].Index > 0) diff --git a/src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs b/src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs index a88421ded..1580b07f3 100644 --- a/src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs +++ b/src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs @@ -296,7 +296,13 @@ public override void StartNote(UsfmParserState state, string marker, string call public override void EndNote(UsfmParserState state, string marker, bool closed) { if (closed) - CollectUpdatableTokens(state); + { + // Mirror StartNote: an embed in a duplicate verse is dropped, end marker included. + if (DuplicateVerse) + SkipUpdatableTokens(state); + else + CollectUpdatableTokens(state); + } base.EndNote(state, marker, closed); } @@ -334,21 +340,26 @@ public override void EndChar( bool closed ) { - // strip out char-style markers in verses that are being replaced - if (CurrentTextType == ScriptureTextType.Embed) - { - CollectUpdatableTokens(state); - } - else + // An implicitly closed character style has no end marker of its own, so the token at + // state.Index belongs to whatever closed it. Leave it for the callback that handles it. + if (closed) { - ReplaceWithNewTokens(state); - if (_styleBehavior == UpdateUsfmMarkerBehavior.Strip) + // strip out char-style markers in verses that are being replaced + if (CurrentTextType == ScriptureTextType.Embed) { - SkipUpdatableTokens(state); + CollectUpdatableTokens(state); } else { - CollectUpdatableTokens(state); + ReplaceWithNewTokens(state); + if (_styleBehavior == UpdateUsfmMarkerBehavior.Strip) + { + SkipUpdatableTokens(state); + } + else + { + CollectUpdatableTokens(state); + } } } diff --git a/src/SIL.Machine/Corpora/UsfmUpdateBlockElement.cs b/src/SIL.Machine/Corpora/UsfmUpdateBlockElement.cs index 953b03677..9956c2bfc 100644 --- a/src/SIL.Machine/Corpora/UsfmUpdateBlockElement.cs +++ b/src/SIL.Machine/Corpora/UsfmUpdateBlockElement.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; namespace SIL.Machine.Corpora { @@ -32,5 +33,21 @@ public List GetTokens() { return MarkedForRemoval ? new List() : new List(Tokens); } + + public string GetText() + { + return string.Concat(Tokens.Select(t => t.ToUsfm())); + } + + public bool IsPlaceable(UpdateUsfmMarkerBehavior paragraphBehavior, UpdateUsfmMarkerBehavior styleBehavior) + { + if (MarkedForRemoval) + return false; + if (Type == UsfmUpdateBlockElementType.Paragraph) + return paragraphBehavior == UpdateUsfmMarkerBehavior.Preserve && Tokens.Count == 1; + if (Type == UsfmUpdateBlockElementType.Style) + return styleBehavior == UpdateUsfmMarkerBehavior.Preserve; + return false; + } } } diff --git a/tests/SIL.Machine.Tests/Corpora/PlaceMarkersUsfmUpdateBlockHandlerTests.cs b/tests/SIL.Machine.Tests/Corpora/PlaceMarkersUsfmUpdateBlockHandlerTests.cs index 60c2e3a27..c3218a2e5 100644 --- a/tests/SIL.Machine.Tests/Corpora/PlaceMarkersUsfmUpdateBlockHandlerTests.cs +++ b/tests/SIL.Machine.Tests/Corpora/PlaceMarkersUsfmUpdateBlockHandlerTests.cs @@ -397,10 +397,10 @@ public void UpdateUsfm_ConsecutiveMarkers() [Test] public void UpdateUsfm_VerseRanges() { - IReadOnlyList rows = Enumerable - .Range(1, 6) - .Select(i => new UpdateUsfmRow( - [ScriptureRef.Parse($"MAT 1:{i}")], + IReadOnlyList rows = + [ + new UpdateUsfmRow( + [.. Enumerable.Range(1, 5).Select(i => ScriptureRef.Parse($"MAT 1:{i}"))], "New verse range text new paragraph 2", new Dictionary { @@ -415,8 +415,8 @@ public void UpdateUsfm_VerseRanges() ) }, } - )) - .ToList(); + ), + ]; string usfm = @"\id MAT \c 1 @@ -911,6 +911,222 @@ public void UpdateUsfm_AdjustmentOfPlacedParagraphMarker() AssertUsfmEquals(target, result); } + [Test] + public void UpdateUsfm_UnclosedStyleMarkerInNonVerseParagraph() + { + string source = "(A)"; + string pretranslation = "(A translated)"; + PlaceMarkersAlignmentInfo alignInfo = new PlaceMarkersAlignmentInfo( + sourceTokens: Tokenizer.Tokenize(source).ToList(), + translationTokens: Tokenizer.Tokenize(pretranslation).ToList(), + alignment: ToWordAlignmentMatrix("0-0 1-1 2-2"), + paragraphBehavior: UpdateUsfmMarkerBehavior.Preserve, + styleBehavior: UpdateUsfmMarkerBehavior.Strip + ); + IReadOnlyList rows = + [ + new UpdateUsfmRow( + ScrRef("PSA 119:0/1:d"), + pretranslation, + new Dictionary { { "alignment_info", alignInfo } } + ), + new UpdateUsfmRow(ScrRef("PSA 119:1"), "New verse 1"), + ]; + string usfm = + @"\id PSA +\c 119 +\d \bd (A) +\q1 +\v 1 Verse 1 +"; + + string target = UpdateUsfm(rows, usfm, usfmUpdateBlockHandlers: [new PlaceMarkersUsfmUpdateBlockHandler()]); + + string result = + @"\id PSA +\c 119 +\d (A translated) +\q1 +\v 1 New verse 1 +"; + AssertUsfmEquals(target, result); + } + + [Test] + public void UpdateUsfm_UnmatchedEndMarker() + { + string source = "Section header"; + string pretranslation = "New section header"; + PlaceMarkersAlignmentInfo alignInfo = new PlaceMarkersAlignmentInfo( + sourceTokens: Tokenizer.Tokenize(source).ToList(), + translationTokens: Tokenizer.Tokenize(pretranslation).ToList(), + alignment: ToWordAlignmentMatrix("0-1 1-2"), + paragraphBehavior: UpdateUsfmMarkerBehavior.Preserve, + styleBehavior: UpdateUsfmMarkerBehavior.Preserve + ); + IReadOnlyList rows = + [ + new UpdateUsfmRow( + ScrRef("MAT 1:0/1:s"), + pretranslation, + new Dictionary { { "alignment_info", alignInfo } } + ), + new UpdateUsfmRow(ScrRef("MAT 1:1"), "New verse 1"), + ]; + string usfm = + @"\id MAT +\c 1 +\s Section header\it* +\p +\v 1 Verse 1 +"; + + string target = UpdateUsfm( + rows, + usfm, + styleBehavior: UpdateUsfmMarkerBehavior.Preserve, + usfmUpdateBlockHandlers: [new PlaceMarkersUsfmUpdateBlockHandler()] + ); + + string result = + @"\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +"; + AssertUsfmEquals(target, result); + } + + [Test] + public void UpdateUsfm_MultipleTextRowsInVerseRangesAreUpdated() + { + string source = "This is the first part. This is the second part."; + string pretranslation = "Esta es la primera parte. Esta es la segunda parte."; + PlaceMarkersAlignmentInfo alignInfo = new PlaceMarkersAlignmentInfo( + sourceTokens: Tokenizer.Tokenize(source).ToList(), + translationTokens: Tokenizer.Tokenize(pretranslation).ToList(), + alignment: ToWordAlignmentMatrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraphBehavior: UpdateUsfmMarkerBehavior.Preserve, + styleBehavior: UpdateUsfmMarkerBehavior.Strip + ); + IReadOnlyList rows = + [ + new UpdateUsfmRow( + ScrRef("MAT 1:1"), + "Esta es la primera parte.", + new Dictionary { { "alignment_info", alignInfo } } + ), + new UpdateUsfmRow( + ScrRef("MAT 1:2"), + "Esta es la segunda parte.", + new Dictionary { { "alignment_info", alignInfo } } + ), + ]; + string usfm = + @"\id MAT +\c 1 +\v 1-2 This is the first part. +\p This is the second part. +"; + + string target = UpdateUsfm(rows, usfm, usfmUpdateBlockHandlers: [new PlaceMarkersUsfmUpdateBlockHandler()]); + + string result = + @"\id MAT +\c 1 +\v 1-2 Esta es la primera parte. +\p Esta es la segunda parte. +"; + AssertUsfmEquals(target, result); + } + + [Test] + public void UpdateUsfm_MarkerBehaviorDisagreesWithAlignmentInfo() + { + string source = "Section header"; + string pretranslation = "New section header"; + PlaceMarkersAlignmentInfo alignInfo = new PlaceMarkersAlignmentInfo( + sourceTokens: Tokenizer.Tokenize(source).ToList(), + translationTokens: Tokenizer.Tokenize(pretranslation).ToList(), + alignment: ToWordAlignmentMatrix("0-1 1-2"), + paragraphBehavior: UpdateUsfmMarkerBehavior.Preserve, + styleBehavior: UpdateUsfmMarkerBehavior.Preserve + ); + IReadOnlyList rows = + [ + new UpdateUsfmRow( + ScrRef("MAT 1:0/1:s"), + pretranslation, + new Dictionary { { "alignment_info", alignInfo } } + ), + new UpdateUsfmRow(ScrRef("MAT 1:1"), "New verse 1"), + ]; + string usfm = + @"\id MAT +\c 1 +\s Section \it header\it* +\p +\v 1 Verse 1 +"; + + string target = UpdateUsfm( + rows, + usfm, + styleBehavior: UpdateUsfmMarkerBehavior.Strip, + usfmUpdateBlockHandlers: [new PlaceMarkersUsfmUpdateBlockHandler()] + ); + + string result = + @"\id MAT +\c 1 +\s New section header +\p +\v 1 New verse 1 +"; + AssertUsfmEquals(target, result); + } + + [Test] + public void UpdateUsfm_OtherElementsDoNotAffectEmbedPlacement() + { + string source = "This is the first part. This is the second part."; + string pretranslation = "Esta es la primera parte. Esta es la segunda parte."; + string result = + @"\id MAT +\c 1 +\v 1 Esta es la primera parte. Esta es la segunda parte. \f + \ft Footnote\f* +\q1 +\q2 +"; + foreach (string milestone in new[] { "", @" \ts-s\*" }) + { + PlaceMarkersAlignmentInfo alignInfo = new PlaceMarkersAlignmentInfo( + sourceTokens: Tokenizer.Tokenize(source).ToList(), + translationTokens: Tokenizer.Tokenize(pretranslation).ToList(), + alignment: ToWordAlignmentMatrix("0-0 1-1 2-2 3-3 4-4 5-5 6-6 7-7 8-8 9-9 10-10"), + paragraphBehavior: UpdateUsfmMarkerBehavior.Preserve, + styleBehavior: UpdateUsfmMarkerBehavior.Strip + ); + IReadOnlyList rows = + [ + new UpdateUsfmRow( + ScrRef("MAT 1:1"), + pretranslation, + new Dictionary { { "alignment_info", alignInfo } } + ), + ]; + string usfm = + "\\id MAT\n\\c 1\n" + + "\\v 1 This is the first part. This is the second part.\\f + \\ft Footnote\\f*\n" + + $"\\q1{milestone}\n\\q2\n"; + + string target = UpdateUsfm(rows, usfm, usfmUpdateBlockHandlers: [new PlaceMarkersUsfmUpdateBlockHandler()]); + + AssertUsfmEquals(target, result); + } + } + [Test] public void UpdateBlock_AnusvaraTokenization() { diff --git a/tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs b/tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs index cddf0df85..f3aab5b50 100644 --- a/tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs +++ b/tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs @@ -1719,6 +1719,65 @@ public void FilterChapters_WithBadChapterReference() AssertUsfmEquals(target, result); } + [Test] + public void UpdateUsfm_UnclosedStyleMarkerDoesNotConsumeNextParagraphMarker() + { + // An unclosed character style is closed implicitly by the next paragraph marker, + // which belongs to the paragraph it starts and must survive. + IReadOnlyList rows = + [ + new UpdateUsfmRow(ScrRef("MAT 1:1"), "New verse 1"), + new UpdateUsfmRow(ScrRef("MAT 1:2"), "New verse 2"), + ]; + string usfm = + @"\id MAT +\c 1 +\q1 +\v 1 Verse 1 \bd Selah +\b +\q1 +\v 2 Verse 2 +"; + + string target = UpdateUsfm(rows, usfm); + + string result = + @"\id MAT +\c 1 +\q1 +\v 1 New verse 1 +\b +\q1 +\v 2 New verse 2 +"; + AssertUsfmEquals(target, result); + + // ...including when the unclosed style is in a non-verse paragraph + rows = + [ + new UpdateUsfmRow(ScrRef("MAT 1:0/1:d"), "New title"), + new UpdateUsfmRow(ScrRef("MAT 1:1"), "New verse 1"), + ]; + usfm = + @"\id MAT +\c 1 +\d \bd Title +\q1 +\v 1 Verse 1 +"; + + target = UpdateUsfm(rows, usfm); + + result = + @"\id MAT +\c 1 +\d New title +\q1 +\v 1 New verse 1 +"; + AssertUsfmEquals(target, result); + } + private static ScriptureRef[] ScrRef(params string[] refs) => [.. refs.Select(r => ScriptureRef.Parse(r))]; private static string UpdateUsfm(