Reviewer comments

src/SIL.Machine/Corpora/ParatextProjectTextUpdaterBase.cs

@@ -24,8 +24,8 @@ public string UpdateUsfm(
             IReadOnlyList<(IReadOnlyList<ScriptureRef>, string)> rows,
             string fullName = null,
             UpdateUsfmTextBehavior textBehavior = UpdateUsfmTextBehavior.PreferExisting,
-            UpdateUsfmIntraVerseMarkerBehavior embedBehavior = UpdateUsfmIntraVerseMarkerBehavior.Preserve,
-            UpdateUsfmIntraVerseMarkerBehavior styleBehavior = UpdateUsfmIntraVerseMarkerBehavior.Strip
+            UpdateUsfmMarkerBehavior embedBehavior = UpdateUsfmMarkerBehavior.Preserve,
+            UpdateUsfmMarkerBehavior styleBehavior = UpdateUsfmMarkerBehavior.Strip
         )
         {
             string fileName = _settings.GetBookFileName(bookId);

src/SIL.Machine/Corpora/ScriptureRefUsfmParserHandlerBase.cs

@@ -22,7 +22,7 @@ public abstract class ScriptureRefUsfmParserHandlerBase : UsfmParserHandlerBase
         private bool _duplicateVerse = false;
 
         private bool _inEmbed;
-        public bool InNoteText { get; private set; }
+        protected bool InNoteText { get; private set; }
         private bool _inNestedEmbed;
 
         protected ScriptureRefUsfmParserHandlerBase()
@@ -168,12 +168,12 @@ public override void StartNote(UsfmParserState state, string marker, string call
 
         public override void EndNote(UsfmParserState state, string marker, bool closed)
         {
-            EndNoteText(state);
+            EndNoteTextWrapper(state);
             EndEmbed(state, marker, null, closed);
             _inEmbed = false;
         }
 
-        public void StartEmbed(UsfmParserState state, string marker)
+        protected void StartEmbed(UsfmParserState state, string marker)
         {
             if (_curVerseRef.IsDefault)
                 UpdateVerseRef(state.VerseRef, marker);
@@ -188,9 +188,9 @@ public void StartEmbed(UsfmParserState state, string marker)
             StartEmbed(state, CreateNonVerseRef());
         }
 
-        public virtual void StartEmbed(UsfmParserState state, ScriptureRef scriptureRef) { }
+        protected virtual void StartEmbed(UsfmParserState state, ScriptureRef scriptureRef) { }
 
-        public virtual void EndEmbed(
+        protected virtual void EndEmbed(
             UsfmParserState state,
             string marker,
             IReadOnlyList<UsfmAttribute> attributes,
@@ -250,7 +250,7 @@ bool closed
                 }
                 else
                 {
-                    EndNoteText(state);
+                    EndNoteTextWrapper(state);
                 }
             }
             if (IsEmbedStyle(marker))
@@ -268,7 +268,7 @@ protected virtual void StartNonVerseText(UsfmParserState state, ScriptureRef scr
 
         protected virtual void EndNonVerseText(UsfmParserState state, ScriptureRef scriptureRef) { }
 
-        public virtual void StartNoteTextWrapper(UsfmParserState state)
+        protected virtual void StartNoteTextWrapper(UsfmParserState state)
         {
             InNoteText = true;
             _curTextType.Push(ScriptureTextType.NoteText);
@@ -277,7 +277,7 @@ public virtual void StartNoteTextWrapper(UsfmParserState state)
 
         protected virtual void StartNoteText(UsfmParserState state) { }
 
-        public virtual void EndNoteText(UsfmParserState state)
+        protected virtual void EndNoteTextWrapper(UsfmParserState state)
         {
             if (_curTextType.Count > 0 && _curTextType.Peek() == ScriptureTextType.NoteText)
             {
@@ -381,12 +381,12 @@ private void CheckConvertVerseParaToNonVerse(UsfmParserState state)
             }
         }
 
-        public bool InEmbed(string marker)
+        protected bool IsInEmbed(string marker)
         {
             return _inEmbed || IsEmbedStyle(marker);
         }
 
-        public bool IsInNestedEmbed(string marker)
+        protected bool IsInNestedEmbed(string marker)
         {
             return _inNestedEmbed
                 || (
@@ -397,17 +397,17 @@ public bool IsInNestedEmbed(string marker)
                 );
         }
 
-        private static bool IsNoteText(string marker)
+        protected static bool IsNoteText(string marker)
         {
             return marker == "ft";
         }
 
-        public static bool IsEmbedPartStyle(string marker)
+        protected static bool IsEmbedPartStyle(string marker)
         {
             return !(marker is null) && marker.Length > 0 && marker[0].IsOneOf(EmbedPartStartCharStyles);
         }
 
-        private static bool IsEmbedStyle(string marker)
+        protected static bool IsEmbedStyle(string marker)
         {
             return !(marker is null) && marker.IsOneOf(EmbedStyles);
         }

src/SIL.Machine/Corpora/UpdateUsfmParserHandler.cs

@@ -11,7 +11,7 @@ public enum UpdateUsfmTextBehavior
         StripExisting
     }
 
-    public enum UpdateUsfmIntraVerseMarkerBehavior
+    public enum UpdateUsfmMarkerBehavior
     {
         Preserve,
         Strip,
@@ -28,8 +28,8 @@ public class UpdateUsfmParserHandler : ScriptureRefUsfmParserHandlerBase
         private readonly List<UsfmToken> _newTokens;
         private readonly string _idText;
         private readonly UpdateUsfmTextBehavior _textBehavior;
-        private readonly UpdateUsfmIntraVerseMarkerBehavior _embedBehavior;
-        private readonly UpdateUsfmIntraVerseMarkerBehavior _styleBehavior;
+        private readonly UpdateUsfmMarkerBehavior _embedBehavior;
+        private readonly UpdateUsfmMarkerBehavior _styleBehavior;
         private readonly Stack<bool> _replace;
         private int _rowIndex;
         private int _tokenIndex;
@@ -40,8 +40,8 @@ public UpdateUsfmParserHandler(
             IReadOnlyList<(IReadOnlyList<ScriptureRef>, string)> rows = null,
             string idText = null,
             UpdateUsfmTextBehavior textBehavior = UpdateUsfmTextBehavior.PreferExisting,
-            UpdateUsfmIntraVerseMarkerBehavior embedBehavior = UpdateUsfmIntraVerseMarkerBehavior.Preserve,
-            UpdateUsfmIntraVerseMarkerBehavior styleBehavior = UpdateUsfmIntraVerseMarkerBehavior.Strip
+            UpdateUsfmMarkerBehavior embedBehavior = UpdateUsfmMarkerBehavior.Preserve,
+            UpdateUsfmMarkerBehavior styleBehavior = UpdateUsfmMarkerBehavior.Strip
         )
         {
             _rows = rows ?? Array.Empty<(IReadOnlyList<ScriptureRef>, string)>();
@@ -200,7 +200,7 @@ bool closed
             base.EndChar(state, marker, attributes, closed);
         }
 
-        public override void StartEmbed(UsfmParserState state, ScriptureRef scriptureRef)
+        protected override void StartEmbed(UsfmParserState state, ScriptureRef scriptureRef)
         {
             _embedRowTexts = AdvanceRows(new[] { scriptureRef }).ToList();
             _embedUpdated = _embedRowTexts.Count > 0;
@@ -212,7 +212,7 @@ public override void StartEmbed(UsfmParserState state, ScriptureRef scriptureRef
                 CollectTokens(state);
         }
 
-        public override void EndEmbed(
+        protected override void EndEmbed(
             UsfmParserState state,
             string marker,
             IReadOnlyList<UsfmAttribute> attributes,
@@ -383,7 +383,7 @@ private bool ReplaceWithNewTokens(UsfmParserState state, bool closed = true)
 
             bool newText = _replace.Count > 0 && _replace.Peek();
             string marker = state?.Token?.Marker;
-            bool inEmbed = InEmbed(marker);
+            bool inEmbed = IsInEmbed(marker);
             bool inNestedEmbed = IsInNestedEmbed(marker);
             bool isStyleTag = marker != null && !IsEmbedPartStyle(marker);
 
@@ -395,10 +395,7 @@ private bool ReplaceWithNewTokens(UsfmParserState state, bool closed = true)
             bool useNewTokens =
                 newText
                 && (!existingText || _textBehavior == UpdateUsfmTextBehavior.PreferNew)
-                && (
-                    !inEmbed
-                    || (InNoteText && !inNestedEmbed && _embedBehavior == UpdateUsfmIntraVerseMarkerBehavior.Preserve)
-                );
+                && (!inEmbed || (InNoteText && !inNestedEmbed && _embedBehavior == UpdateUsfmMarkerBehavior.Preserve));
 
             if (useNewTokens)
                 AddNewTokens();
@@ -410,7 +407,7 @@ private bool ReplaceWithNewTokens(UsfmParserState state, bool closed = true)
             bool embedInNewVerseText = _replace.Any(r => r) && inEmbed;
             if (embedInNewVerseText || _embedUpdated)
             {
-                if (_embedBehavior == UpdateUsfmIntraVerseMarkerBehavior.Strip)
+                if (_embedBehavior == UpdateUsfmMarkerBehavior.Strip)
                 {
                     ClearNewTokens();
                     return true;
@@ -424,7 +421,7 @@ private bool ReplaceWithNewTokens(UsfmParserState state, bool closed = true)
 
             if (newText && isStyleTag)
             {
-                skipTokens = _styleBehavior == UpdateUsfmIntraVerseMarkerBehavior.Strip;
+                skipTokens = _styleBehavior == UpdateUsfmMarkerBehavior.Strip;
             }
             return skipTokens;
         }

src/SIL.Machine/Corpora/UsfmStylesheet.cs

@@ -111,7 +111,7 @@ public static bool IsCellRange(string tag, out string baseMarker, out int colSpa
             return false;
         }
 
-        private static IEnumerable<string> GetEmbedStylesheet(string fileName)
+        private static IEnumerable<string> GetEmbeddedStylesheet(string fileName)
         {
             using (
                 var reader = new StreamReader(
@@ -136,7 +136,7 @@ private void Parse(string stylesheetFileName)
             {
                 string fileName = Path.GetFileName(stylesheetFileName);
                 if (fileName == "usfm.sty" || fileName == "usfm_sb.sty")
-                    lines = GetEmbedStylesheet(fileName);
+                    lines = GetEmbeddedStylesheet(fileName);
                 else
                     throw new ArgumentException("The stylesheet does not exist.", nameof(stylesheetFileName));
             }

src/SIL.Machine/Corpora/UsfmTextBase.cs

@@ -259,7 +259,7 @@ public override void Text(UsfmParserState state, string text)
                 else if (text.Length > 0 && (CurrentTextType != ScriptureTextType.Verse || state.IsVerseText))
                 {
                     bool isEmbedOrNestedDontUpdate =
-                        InEmbed(state.Token.Marker) && (!InNoteText || IsInNestedEmbed(state.Token.Marker));
+                        IsInEmbed(state.Token.Marker) && (!InNoteText || IsInNestedEmbed(state.Token.Marker));
                     if (isEmbedOrNestedDontUpdate)
                         return;
 

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

@@ -88,7 +88,7 @@ public void GetUsfm_Verse_StripNote()
             (ScrRef("MAT 2:1"), "First verse of the second chapter.")
         };
 
-        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         Assert.That(target, Contains.Substring("\\v 1 First verse of the second chapter.\r\n"));
     }
 
@@ -245,7 +245,7 @@ public void GetUsfm_Verse_OptBreak()
             (ScrRef("MAT 2:3"), "Third verse of the second chapter.")
         };
 
-        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         Assert.That(
             target,
             Contains.Substring("\\v 2-3 Second verse of the second chapter. Third verse of the second chapter.\r\n")
@@ -391,7 +391,7 @@ public void GetUsfm_NonVerse_SkipNote()
             (ScrRef("MAT 1:0/3:ip"), "The introductory paragraph.")
         };
 
-        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        string target = UpdateUsfm(rows, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         Assert.That(target, Contains.Substring("\\ip The introductory paragraph.\r\n"));
     }
 
@@ -490,8 +490,8 @@ public void EmbedStylePreservation()
         var target = UpdateUsfm(
             rows,
             usfm,
-            embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Preserve,
-            styleBehavior: UpdateUsfmIntraVerseMarkerBehavior.Preserve
+            embedBehavior: UpdateUsfmMarkerBehavior.Preserve,
+            styleBehavior: UpdateUsfmMarkerBehavior.Preserve
         );
         var resultPp =
             @"\id MAT - Test
@@ -505,8 +505,8 @@ public void EmbedStylePreservation()
         target = UpdateUsfm(
             rows,
             usfm,
-            embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Preserve,
-            styleBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip
+            embedBehavior: UpdateUsfmMarkerBehavior.Preserve,
+            styleBehavior: UpdateUsfmMarkerBehavior.Strip
         );
         var resultPs =
             @"\id MAT - Test
@@ -520,8 +520,8 @@ public void EmbedStylePreservation()
         target = UpdateUsfm(
             rows,
             usfm,
-            embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip,
-            styleBehavior: UpdateUsfmIntraVerseMarkerBehavior.Preserve
+            embedBehavior: UpdateUsfmMarkerBehavior.Strip,
+            styleBehavior: UpdateUsfmMarkerBehavior.Preserve
         );
         var resultSp =
             @"\id MAT - Test
@@ -535,8 +535,8 @@ public void EmbedStylePreservation()
         target = UpdateUsfm(
             rows,
             usfm,
-            embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip,
-            styleBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip
+            embedBehavior: UpdateUsfmMarkerBehavior.Strip,
+            styleBehavior: UpdateUsfmMarkerBehavior.Strip
         );
         var resultSs =
             @"\id MAT - Test
@@ -626,7 +626,7 @@ public void NestedXt()
 ";
         Assess(target, result);
 
-        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         var result2 =
             @"\id MAT - Test
 \c 1
@@ -656,7 +656,7 @@ public void NonNestedXt()
 ";
         Assess(target, result);
 
-        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         var result2 =
             @"\id MAT - Test
 \c 1
@@ -686,7 +686,7 @@ public void MultipleFtOnlyUpdateFirst()
 ";
         Assess(target, result);
 
-        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmIntraVerseMarkerBehavior.Strip);
+        target = UpdateUsfm(rows, usfm, embedBehavior: UpdateUsfmMarkerBehavior.Strip);
         var result2 =
             @"\id MAT - Test
 \c 1
@@ -705,8 +705,8 @@ private static string UpdateUsfm(
         string? source = null,
         string? idText = null,
         UpdateUsfmTextBehavior textBehavior = UpdateUsfmTextBehavior.PreferNew,
-        UpdateUsfmIntraVerseMarkerBehavior embedBehavior = UpdateUsfmIntraVerseMarkerBehavior.Preserve,
-        UpdateUsfmIntraVerseMarkerBehavior styleBehavior = UpdateUsfmIntraVerseMarkerBehavior.Strip
+        UpdateUsfmMarkerBehavior embedBehavior = UpdateUsfmMarkerBehavior.Preserve,
+        UpdateUsfmMarkerBehavior styleBehavior = UpdateUsfmMarkerBehavior.Strip
     )
     {
         if (source is null)