Port improvements to quote denormalization

[Enkidu93]

Fixes #356

---

This change is 

[codecov-commenter]

Codecov Report

❌ Patch coverage is 99.42197% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.51%. Comparing base (6e1caae) to head (4e07d0f).

Files with missing lines	Patch %	Lines
...ine/PunctuationAnalysis/QuoteConventionAnalysis.cs	98.64%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   72.47%   72.51%   +0.04%     
==========================================
  Files         422      423       +1     
  Lines       35803    35927     +124     
  Branches     4949     4958       +9     
==========================================
+ Hits        25947    26052     +105     
- Misses       8760     8779      +19     
  Partials     1096     1096              

☔ View full report in Codecov by Sentry.
📢 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.

[ddaspit]

@ddaspit reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 83 at r1 (raw file):

            {
                ((int depth, QuotationMarkDirection direction), QuotationMarkCounts counts) = (kvp.Key, kvp.Value);
                if (!_quotationCountsByDepthAndDirection.ContainsKey((depth, direction)))

You should use TryGetValue here.

[Enkidu93]

Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 83 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You should use TryGetValue here.

Done.

[ddaspit]

@ddaspit reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 87 at r2 (raw file):

                    _quotationCountsByDepthAndDirection[(depth, direction)] = new QuotationMarkCounts();
                }
                counts.CountFrom(counts);

I don't think this is right. Maybe you should use more explicit variable names here, such as thisCounts and otherCounts.

[Enkidu93]

Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 87 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

I don't think this is right. Maybe you should use more explicit variable names here, such as thisCounts and otherCounts.

Oh my - that's pretty bad. That's what happens when you rush a change 😬. Sorry, done.

[ddaspit]

@ddaspit reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 87 at r2 (raw file):

Previously, Enkidu93 (Eli C. Lowry) wrote…

Oh my - that's pretty bad. That's what happens when you rush a change 😬. Sorry, done.

You aren't setting thisCounts when it doesn't already exist.

[Enkidu93]

Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

---

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 87 at r2 (raw file):

Previously, ddaspit (Damien Daspit) wrote…

You aren't setting thisCounts when it doesn't already exist.

I see. Done. TryGetValue always confuses me. I always think I should be able to set thisCounts = new QuotationMarkCounts() when TryGetValue fails and that'll actually update the key in the dictionary and then I end up second-guessing myself 10 times. Hopefully, I've finally gotten it. It concerns me that no tests have failed in any of these broken iterations. I'll make sure I've done this correctly elsewhere.

[Enkidu93]

Reviewable status: 11 of 12 files reviewed, 1 unresolved discussion (waiting on @ddaspit)

src/SIL.Machine/PunctuationAnalysis/QuotationMarkTabulator.cs line 87 at r2 (raw file):
Previously, ddaspit (Damien Daspit) wrote…

I see. Done. TryGetValue always confuses me. I always think I should be able to set thisCounts = new QuotationMarkCounts() when TryGetValue fails and that'll actually update the key in the dictionary and then I end up second-guessing myself 10 times. Hopefully, I've finally gotten it. It concerns me that no tests have failed in any of these broken iterations. I'll make sure I've done this correctly elsewhere.

I don't see this issue elsewhere, and I'm comforted that one of the broken iterations did in fact cause tests to fail.

[ddaspit]

@ddaspit reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)