Convert USFM versification

[Enkidu93]

Fixes #421.

There are still some changes we'll need in Serval to fully address this problem besides just utilizing this functionality (see sillsdev/serval#1027 for example).

---

This change is 

[pmachapman]

I don't think code handles the common use case of a verse jumping back and forth across chapters, i.e. for Original to English:

• GEN 32:1 to GEN 31:55
• ISA 8:23 to ISA 9:1

Here are two unit test (I think these are correct???) that fail for these cases:

    [Test]
    public void GetUsfm_ConvertUsfmToUpdateRowVersification_BackOneVerseToPreviousChapter()
    {
        // English vs. Original
        // ISA 9:1 = ISA 8:23

        List<UpdateUsfmRow> rows =
        [
            new UpdateUsfmRow(ScrRef(["ISA 9:1"], ScrVers.English), "Updated verse 1"),
            new UpdateUsfmRow(ScrRef(["ISA 9:2"], ScrVers.English), "Updated verse 2"),
        ];

        string usfm =
            @"\id ISA - Test
\c 8
\v 23
\c 9
\v 1
";

        string target = UpdateUsfm(
            rows,
            usfm,
            bookId: "ISA",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.Original
        );
        string result =
            @"\id ISA - Test
\c 8
\c 9
\v 1 Updated verse 1
\v 2 Updated verse 2
";
        AssertUsfmEquals(target, result);
    }

    [Test]
    public void GetUsfm_ConvertUsfmToUpdateRowVersification_ForwardOneVerseToNextChapter()
    {
        // Original vs. English
        // ISA 8:23 = ISA 9:1

        List<UpdateUsfmRow> rows =
        [
            new UpdateUsfmRow(ScrRef(["ISA 8:23"], ScrVers.Original), "Updated verse 23"),
            new UpdateUsfmRow(ScrRef(["ISA 9:1"], ScrVers.Original), "Updated verse 1"),
        ];

        string usfm =
            @"\id ISA - Test
\c 8
\c 9
\v 1
\v 2
";

        string target = UpdateUsfm(
            rows,
            usfm,
            bookId: "ISA",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.English
        );
        string result =
            @"\id ISA - Test
\c 8
\v 23 Updated verse 23
\c 9
\v 1 Updated verse 1
";
        AssertUsfmEquals(target, result);
    }

For GetUsfm_ConvertUsfmToUpdateRowVersification_BackOneVerseToPreviousChapter(), the value of target is:

\id ISA - Test
\c 8
\v 1 Updated verse 1
\v 2
\c 9 Updated verse 2

For GetUsfm_ConvertUsfmToUpdateRowVersification_ForwardOneVerseToNextChapter(), the value of target is:

\id ISA - Test
\c 8
\c 9
\v 23 Updated verse 23
\c 9
\v 1 Updated verse 1

(apologies if my tests are wrong or if I am misunderstanding this PR)

@pmachapman reviewed 4 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on ddaspit and Enkidu93).

---

src/SIL.Machine/Corpora/UsfmToken.cs line 183 at r1 (raw file):

        }

        public UsfmToken Copy()

Optional: If you make this:

public UsfmToken Clone()

Then this class could implement ICloneable<>:

using SIL.ObjectModel;

...

public class UsfmToken: IEquatable<UsfmToken>, ICloneable<UsfmToken>

Code quote:

public UsfmToken Copy()

---

tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs line 885 at r1 (raw file):

            convertUsfmToUpdateRowVersification: false,
            versification: ScrVers.RussianOrthodox
        );

NIT: Should this have the bookId specified?

        target = UpdateUsfm(
            rows,
            usfm,
            bookId: "PSA",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.RussianOrthodox
        );

Code quote:

        string target = UpdateUsfm(
            rows,
            usfm,
            convertUsfmToUpdateRowVersification: false,
            versification: ScrVers.RussianOrthodox
        );

---

tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs line 940 at r1 (raw file):

            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.Original
        );

NIT: Should this have the bookId specified?

        string target = UpdateUsfm(
            rows,
            usfm,
            bookId: "DAN",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.Original
        );

Code quote:

        string target = UpdateUsfm(
            rows,
            usfm,
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.Original
        );

[codecov-commenter]

Codecov Report

❌ Patch coverage is 89.70588% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.35%. Comparing base (0b58830) to head (29d4f7f).

Files with missing lines	Patch %	Lines
src/SIL.Machine/Corpora/UsfmToken.cs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
+ Coverage   73.33%   73.35%   +0.02%     
==========================================
  Files         445      445              
  Lines       37317    37374      +57     
  Branches     5118     5126       +8     
==========================================
+ Hits        27367    27417      +50     
- Misses       8825     8832       +7     
  Partials     1125     1125              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Enkidu93]

Thank you for catching this, Peter. It turns out that these test failures have less to do with the specific mapping scenario and more to do with whether the mapping verses are padded with verses etc. Goes to show that writing tests is harder than you think 🤪. After reviewing the problems, I've decided the simplest thing to do is to have the verse handler add all chapter tokens rather than trying to re-use the encountered ones when possible. The only downside with this is that empty chapters (after the conversion) will be stripped. I don't think this is unreasonable behavior though, and realistically, I don't think it will make a difference.

@Enkidu93 made 4 comments and resolved 2 discussions.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on ddaspit and pmachapman).

---

src/SIL.Machine/Corpora/UsfmToken.cs line 183 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

Optional: If you make this:

public UsfmToken Clone()

Then this class could implement ICloneable<>:

using SIL.ObjectModel;

...

public class UsfmToken: IEquatable<UsfmToken>, ICloneable<UsfmToken>

Done. Thanks!

---

tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs line 885 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: Should this have the bookId specified?

        target = UpdateUsfm(
            rows,
            usfm,
            bookId: "PSA",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.RussianOrthodox
        );

Done.

---

tests/SIL.Machine.Tests/Corpora/UpdateUsfmParserHandlerTests.cs line 940 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: Should this have the bookId specified?

        string target = UpdateUsfm(
            rows,
            usfm,
            bookId: "DAN",
            convertUsfmToUpdateRowVersification: true,
            versification: ScrVers.Original
        );

Done.

[pmachapman]

from my perspective. I'm sure using it in Serval will throw all sorts of interesting edge cases we haven't thought of!

@pmachapman reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on ddaspit).