Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #333 +/- ##
=======================================
Coverage 72.22% 72.22%
=======================================
Files 414 414
Lines 35154 35169 +15
Branches 4865 4871 +6
=======================================
+ Hits 25389 25401 +12
- Misses 8674 8676 +2
- Partials 1091 1092 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- properly handle verse 0 - ignore private-use markers - fixes #329
3a4450e to
6fc66e4
Compare
Enkidu93
left a comment
There was a problem hiding this comment.
@Enkidu93 reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion
src/SIL.Machine/Corpora/ScriptureRefUsfmParserHandlerBase.cs line 71 at r2 (raw file):
if (state.VerseRef.Equals(_curVerseRef) && !_duplicateVerse) { if (state.VerseRef.VerseNum > 0)
Could we use state.IsVerseText instead?
ddaspit
left a comment
There was a problem hiding this comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @Enkidu93)
src/SIL.Machine/Corpora/ScriptureRefUsfmParserHandlerBase.cs line 71 at r2 (raw file):
Previously, Enkidu93 (Eli C. Lowry) wrote…
Could we use
state.IsVerseTextinstead?
This issue is specifically about the case where there is a \v 0 marker.
Enkidu93
left a comment
There was a problem hiding this comment.
Do you want to put in a PR for the new Machine version or should I?
@Enkidu93 reviewed 1 of 2 files at r2.
Reviewable status: 1 of 2 files reviewed, all discussions resolved
src/SIL.Machine/Corpora/ScriptureRefUsfmParserHandlerBase.cs line 71 at r2 (raw file):
Previously, ddaspit (Damien Daspit) wrote…
This issue is specifically about the case where there is a
\v 0marker.
OK, I was just thinking since the first thing IsVerseTextchecks is
if (VerseRef.VerseNum == 0)
return false;
...
we could use it, but maybe that doesn't make as much sense semantically as I thought when I left the comment 😅.
Enkidu93
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, all discussions resolved
ddaspit
left a comment
There was a problem hiding this comment.
@ddaspit reviewed 1 of 2 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ddaspit)
This change is