Fix USFM parsing bugs

src/SIL.Machine/Corpora/FileParatextProjectTextUpdater.cs

@@ -0,0 +1,25 @@
+using System.IO;
+
+namespace SIL.Machine.Corpora
+{
+    public class FileParatextProjectTextUpdater : ParatextProjectTextUpdaterBase
+    {
+        private readonly string _projectDir;
+
+        public FileParatextProjectTextUpdater(string projectDir)
+            : base(new FileParatextProjectSettingsParser(projectDir))
+        {
+            _projectDir = projectDir;
+        }
+
+        protected override bool Exists(string fileName)
+        {
+            return File.Exists(Path.Combine(_projectDir, fileName));
+        }
+
+        protected override Stream Open(string fileName)
+        {
+            return File.OpenRead(Path.Combine(_projectDir, fileName));
+        }
+    }
+}

src/SIL.Machine/Corpora/ParatextProjectTextUpdaterBase.cs

@@ -0,0 +1,48 @@
+using System.Collections.Generic;
+using System.IO;
+
+namespace SIL.Machine.Corpora
+{
+    public abstract class ParatextProjectTextUpdaterBase
+    {
+        private readonly ParatextProjectSettingsParserBase _settingsParser;
+
+        protected ParatextProjectTextUpdaterBase(ParatextProjectSettingsParserBase settingsParser)
+        {
+            _settingsParser = settingsParser;
+        }
+
+        public string UpdateUsfm(
+            string bookId,
+            IReadOnlyList<(IReadOnlyList<ScriptureRef>, string)> rows,
+            string fullName = null,
+            bool stripAllText = false,
+            bool preferExistingText = true
+        )
+        {
+            ParatextProjectSettings settings = _settingsParser.Parse();
+
+            string fileName = settings.GetBookFileName(bookId);
+            if (!Exists(fileName))
+                return null;
+
+            string usfm;
+            using (var reader = new StreamReader(Open(fileName)))
+            {
+                usfm = reader.ReadToEnd();
+            }
+
+            var handler = new UpdateUsfmParserHandler(
+                rows,
+                fullName is null ? null : $"- {fullName}",
+                stripAllText,
+                preferExistingText: preferExistingText
+            );
+            UsfmParser.Parse(usfm, handler, settings.Stylesheet, settings.Versification);
+            return handler.GetUsfm(settings.Stylesheet);
+        }
+
+        protected abstract bool Exists(string fileName);
+        protected abstract Stream Open(string fileName);
+    }
+}

src/SIL.Machine/Corpora/ScriptureRefUsfmParserHandlerBase.cs

@@ -53,7 +53,7 @@ public override void Verse(
             string pubNumber
         )
         {
-            if (state.VerseRef.Equals(_curVerseRef))
+            if (state.VerseRef.Equals(_curVerseRef) && !_duplicateVerse)
             {
                 EndVerseText(state, CreateVerseRefs());
                 // ignore duplicate verses

src/SIL.Machine/Corpora/UsfmTextUpdater.cs → src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs

@@ -8,7 +8,7 @@ namespace SIL.Machine.Corpora
      * This is a USFM parser handler that can be used to replace the existing text in a USFM file with the specified
      * text.
      */
-    public class UsfmTextUpdater : ScriptureRefUsfmParserHandlerBase
+    public class UpdateUsfmParserHandler : ScriptureRefUsfmParserHandlerBase
     {
         private readonly IReadOnlyList<(IReadOnlyList<ScriptureRef>, string)> _rows;
         private readonly List<UsfmToken> _tokens;
@@ -20,7 +20,7 @@ public class UsfmTextUpdater : ScriptureRefUsfmParserHandlerBase
         private int _rowIndex;
         private int _tokenIndex;
 
-        public UsfmTextUpdater(
+        public UpdateUsfmParserHandler(
             IReadOnlyList<(IReadOnlyList<ScriptureRef>, string)> rows = null,
             string idText = null,
             bool stripAllText = false,
@@ -361,7 +361,7 @@ private void SkipTokens(UsfmParserState state)
         private bool ReplaceWithNewTokens(UsfmParserState state)
         {
             bool newText = _replace.Count > 0 && _replace.Peek();
-            int tokenEnd = state.Index + state.SpecialTokenCount + 1;
+            int tokenEnd = state.Index + state.SpecialTokenCount;
             bool existingText = false;
             for (int index = _tokenIndex; index <= tokenEnd; index++)
             {

src/SIL.Machine/Corpora/UsfmParser.cs

@@ -1,5 +1,7 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using System.Text.RegularExpressions;
 using SIL.Scripture;
 
@@ -41,7 +43,19 @@ public static void Parse(
                 versification,
                 preserveWhitespace
             );
-            parser.ProcessTokens();
+            try
+            {
+                parser.ProcessTokens();
+            }
+            catch (Exception ex)
+            {
+                var sb = new StringBuilder();
+                sb.Append(
+                    $"An error occurred while parsing the USFM text in Verse: {parser.State.VerseRef}, line: {parser.State.LineNumber}, "
+                );
+                sb.Append($"column: {parser.State.ColumnNumber}, error: '{ex.Message}'");
+                throw new InvalidOperationException(sb.ToString(), ex);
+            }
         }
 
         private static readonly Regex OptBreakSplitter = new Regex("(//)", RegexOptions.Compiled);

src/SIL.Machine/Corpora/ZipParatextProjectTextUpdater.cs

@@ -0,0 +1,29 @@
+using System.IO;
+using System.IO.Compression;
+
+namespace SIL.Machine.Corpora
+{
+    public class ZipParatextProjectTextUpdater : ZipParatextProjectTextUpdaterBase
+    {
+        private readonly ZipArchive _archive;
+
+        public ZipParatextProjectTextUpdater(ZipArchive archive)
+            : base(new ZipParatextProjectSettingsParser(archive))
+        {
+            _archive = archive;
+        }
+
+        protected override bool Exists(string fileName)
+        {
+            return _archive.GetEntry(fileName) != null;
+        }
+
+        protected override Stream Open(string fileName)
+        {
+            ZipArchiveEntry entry = _archive.GetEntry(fileName);
+            if (entry == null)
+                return null;
+            return entry.Open();
+        }
+    }
+}

src/SIL.Machine/Corpora/ZipParatextProjectTextUpdaterBase.cs

@@ -0,0 +1,8 @@
+namespace SIL.Machine.Corpora
+{
+    public abstract class ZipParatextProjectTextUpdaterBase : ParatextProjectTextUpdaterBase
+    {
+        protected ZipParatextProjectTextUpdaterBase(ParatextProjectSettingsParserBase settingsParser)
+            : base(settingsParser) { }
+    }
+}

tests/SIL.Machine.Tests/Corpora/UsfmTextUpdaterTests.cs → tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs

@@ -3,7 +3,7 @@
 namespace SIL.Machine.Corpora;
 
 [TestFixture]
-public class UsfmTextUpdaterTests
+public class UpdateUsfmParserHandlerTests
 {
     [Test]
     public void GetUsfm_Verse_CharStyle()
@@ -21,7 +21,7 @@ public void GetUsfm_Verse_CharStyle()
     [Test]
     public void GetUsfm_IdText()
     {
-        string target = UpdateUsfm(idText: "- Updated");
+        string target = UpdateUsfm(idText: "Updated");
         Assert.That(target, Contains.Substring("\\id MAT - Updated\r\n"));
     }
 
@@ -443,21 +443,27 @@ private static string UpdateUsfm(
     )
     {
         if (source is null)
-            source = ReadUsfm();
+        {
+            var updater = new FileParatextProjectTextUpdater(CorporaTestHelpers.UsfmTestProjectPath);
+            return updater.UpdateUsfm(
+                "MAT",
+                rows,
+                fullName: idText,
+                stripAllText: stripAllText,
+                preferExistingText: preferExistingText
+            );
+        }
         else
+        {
             source = source.Trim().ReplaceLineEndings("\r\n") + "\r\n";
-        var updater = new UsfmTextUpdater(
-            rows,
-            idText,
-            stripAllText: stripAllText,
-            preferExistingText: preferExistingText
-        );
-        UsfmParser.Parse(source, updater);
-        return updater.GetUsfm();
-    }
-
-    private static string ReadUsfm()
-    {
-        return File.ReadAllText(Path.Combine(CorporaTestHelpers.UsfmTestProjectPath, "41MATTes.SFM"));
+            var updater = new UpdateUsfmParserHandler(
+                rows,
+                idText,
+                stripAllText: stripAllText,
+                preferExistingText: preferExistingText
+            );
+            UsfmParser.Parse(source, updater);
+            return updater.GetUsfm();
+        }
     }
 }

tests/SIL.Machine.Tests/Corpora/UsfmManualTests.cs

@@ -44,7 +44,7 @@ string sfmFileName in Directory.EnumerateFiles(
             )
         )
         {
-            var updater = new UsfmTextUpdater(pretranslations, stripAllText: true, preferExistingText: false);
+            var updater = new UpdateUsfmParserHandler(pretranslations, stripAllText: true, preferExistingText: false);
             string usfm = await File.ReadAllTextAsync(sfmFileName);
             UsfmParser.Parse(usfm, updater, targetSettings.Stylesheet, targetSettings.Versification);
             string newUsfm = updater.GetUsfm(targetSettings.Stylesheet);
@@ -135,7 +135,11 @@ await Task.WhenAll(
             }
             foreach (string usfm in sfmTexts)
             {
-                var updater = new UsfmTextUpdater(pretranslations, stripAllText: true, preferExistingText: true);
+                var updater = new UpdateUsfmParserHandler(
+                    pretranslations,
+                    stripAllText: true,
+                    preferExistingText: true
+                );
                 UsfmParser.Parse(usfm, updater, settings.Stylesheet, settings.Versification);
                 string newUsfm = updater.GetUsfm(settings.Stylesheet);
                 Assert.That(newUsfm, Is.Not.Null);

tests/SIL.Machine.Tests/Corpora/UsfmMemoryTextTests.cs

@@ -25,8 +25,16 @@ public void GetRows_VerseDescriptiveTitle()
         {
             Assert.That(rows, Has.Length.EqualTo(1));
 
-            Assert.That(rows[0].Ref, Is.EqualTo(ScriptureRef.Parse("MAT 1:1")));
-            Assert.That(rows[0].Text, Is.EqualTo("Descriptive title"));
+            Assert.That(
+                rows[0].Ref,
+                Is.EqualTo(ScriptureRef.Parse("MAT 1:1")),
+                string.Join(",", rows.ToList().Select(tr => tr.Ref.ToString()))
+            );
+            Assert.That(
+                rows[0].Text,
+                Is.EqualTo("Descriptive title"),
+                string.Join(",", rows.ToList().Select(tr => tr.Text))
+            );
         });
     }
 
@@ -44,8 +52,16 @@ public void GetRows_LastSegment()
         {
             Assert.That(rows, Has.Length.EqualTo(1));
 
-            Assert.That(rows[0].Ref, Is.EqualTo(ScriptureRef.Parse("MAT 1:1")));
-            Assert.That(rows[0].Text, Is.EqualTo("Last segment"));
+            Assert.That(
+                rows[0].Ref,
+                Is.EqualTo(ScriptureRef.Parse("MAT 1:1")),
+                string.Join(",", rows.ToList().Select(tr => tr.Ref.ToString()))
+            );
+            Assert.That(
+                rows[0].Text,
+                Is.EqualTo("Last segment"),
+                string.Join(",", rows.ToList().Select(tr => tr.Text))
+            );
         });
     }
 
@@ -67,7 +83,32 @@ public void GetRows_DuplicateVerseWithTable()
             includeAllText: true
         );
 
-        Assert.That(rows, Has.Length.EqualTo(5));
+        Assert.That(rows, Has.Length.EqualTo(5), string.Join(",", rows.ToList().Select(tr => tr.Text)));
+    }
+
+    [Test]
+    public void GetRows_TriplicateVerse()
+    {
+        TextRow[] rows = GetRows(
+            @"\id MAT - Test
+\c 1
+\v 1 First verse 1
+\rem non verse 1
+\v 1 First verse 2
+\rem non verse 2
+\v 1 First verse 3
+\rem non verse 3
+\v 2 Second verse
+",
+            includeAllText: true
+        );
+        Assert.Multiple(() =>
+        {
+            Assert.That(rows, Has.Length.EqualTo(5), string.Join(",", rows.ToList().Select(tr => tr.Text)));
+            Assert.That(rows[0].Text, Is.EqualTo("First verse 1"));
+            Assert.That(rows[3].Text, Is.EqualTo("non verse 3"));
+            Assert.That(rows[4].Text, Is.EqualTo("Second verse"));
+        });
     }
 
     [Test]
@@ -88,7 +129,7 @@ public void GetRows_VersePara_BeginningNonVerseSegment()
             includeAllText: true
         );
 
-        Assert.That(rows, Has.Length.EqualTo(4));
+        Assert.That(rows, Has.Length.EqualTo(4), string.Join(",", rows.ToList().Select(tr => tr.Text)));
     }
 
     private static TextRow[] GetRows(string usfm, bool includeMarkers = false, bool includeAllText = false)