Repository: Bridgeconn/usfm-grammar (language: python)

Issue:
Parsing the usfm file gives the output with spaces in between
On parsing the given usfm the output json has a space before the punctuation marks.
[02-GENeng-asv.zip](https://github.com/Bridgeconn/usfm-grammar/files/4965669/02-GENeng-asv.zip)
Source of usfm file - https://ebible.org/asv/ 
Click eng-asv_usfm.zip




Discussion / hints:
I have fixed this issue in this [commit](https://github.com/Bridgeconn/usfm-grammar/commit/7628c2bc30ce56fd9f040a2fa35d2b418b980b81).

Currently the punctuations handled are only as per the following regex.
```
const punctPattern = new RegExp('^[,./;:\'"`~!@#$%^&*(){}[}|]');
```
for those punctuation marks that are outside of this list, space may be added if they occur at a marker boundary, like before.

Issue is Fixed
I'm still having this issue when using `usfm-grammar` on <https://git.door43.org/unfoldingWord/en_ult/> exporting as CSV with `relaxed`.
For comparison, I just tested unfolding Word's `usfm-js` (<https://github.com/unfoldingWord/usfm-js>) and it works great on the English texts I've been working with.
@jcuenod 
I am not able to locate where the issue is occurring in the shared usfm files.
Can you point out a small usfm snippet, for which you are getting the space issue around punctuations?
With the source file: https://git.door43.org/unfoldingWord/en_ult/src/branch/master/08-RUT.usfm

Running:

```
npx usfm-grammar -l relaxed 08-RUT.usfm -o csv > 08-RUT.csv
```

I get output like:

```
"RUT","1","8","Then Naomi said to her two daughters - in - law, “ Go, return, each woman to the house of her mother. May Yahweh act with you in covenant faithfulness as you have acted with the dead and with me."
```
I had a look at this for a while but I think the problem is actually the assumptions baked into your `punctPattern` and this:

```js
if (punctPattern.test(text)) {
    verseTextPartial = verseTextPartial.trim();
    verseTextPartial += text;
} else {
    verseTextPartial += ` ${text}`;
}
```

I was giving this a bunch of thought and realised that you can't assume something about whether a space precedes/succeeds punctuation or not. It's dependent on context.

There are a number of possibilities:
1. Periods, commas, etc. seem to be succeeded by a space and not preceded by one.
2. Single quotes and dashes have no spaces around them. E.g. "daughter - in - law" or "Peter's"
3. Double quotes are preceded by a space when they begin a quotation but are succeeded by one if they end it. This only holds conditionally, though. Punctuation styles vary. Some systems have quotation marks following the last punctuation mark; others have them preceding it. It's also conceivable a text might use single quotes for a direct quote.

I think the way [`usfm-js`](https://github.com/unfoldingWord/usfm-js) handles this is by reading the whitespace from the usfm files but, tbh, I didn't try to make too much sense of what's going on.
To be honest, I spent a while messing around with the `grammarOperations` in `usfm-grammar` so if my output doesn't match yours, it's possible I've inadvertently edited the local copy I'm running. I'm pretty sure I had these problems in Ruth, though. But if you have trouble reproducing my issue, let me know because it may well be on my end.

I am confident that the invocation I am using above is how I arrived at my initial issues. So if you can't reproduce them using that line, that would be interesting to know...
@jcuenod  
> if you have trouble reproducing my issue, let me know because it may well be on my end.

I am able to reproduce the issue too for  RUT 1:8 in the file.

> I think the problem is actually the assumptions baked into your punctPattern and this:

I agree. For one, we don't have "-" listed as a punctuation in our pattern. The assumption was that "-" would be used to indicate compound words and it was not expected to be broken up into multiple `\w`s or milestones. The same assumption was for `'` coming as in `Peter's`. The space addition/non-addition problem would arise only if it occurs at marker boundaries.

Even if we add `-` to our list of punctuations we would get `daughters- in- law` which is not exactly what we would need. As you said, there is no one rule, that would apply to all punctuations, not even for the same punctuation in different context.

> I think the way usfm-js handles this is by reading the whitespace from the usfm files but, tbh, I didn't try to make too much sense of what's going on.

Looking into usfm-js output I think it adds a `{"type":"text", "text":" "}` for newlines present in the input usfm. In the case of daughters-in-law, though they are split up into multiple markers, they are all in one line. Thus usfm-js do not add extra space text. But I don't think, having new lines between markers have any such meaning as per usfm spec(pls, correct me if I am wrong). 

Let us think through this and see how to handle this case the best. Any more suggestions from your end is also welcome.

Fix patch:
diff --git a/js/grammarOperations.js b/js/grammarOperations.js
index 20cd34e9..75815266 100644
--- a/js/grammarOperations.js
+++ b/js/grammarOperations.js
@@ -30,7 +30,9 @@ const paraMarkers = ['p', 'm', 'po', 'pr', 'cls', 'pmo', 'pm', 'pmc',
   'ph3', 'b', 'q', 'q1', 'q2', 'q3', 'qr', 'qc', 'qs', 'qa', 'qac', 'qm',
   'qm1', 'qm2', 'qm3'];
 
-const punctPattern = new RegExp('^[,./;:\'"`~!@#$%^&*(){}[}|]');
+const punctPattern1 = new RegExp('^[,.\\-—/;:!?@$%^)}\\]>”»]'); // no space before
+const punctPattern2 = new RegExp('[\\-—/`@^&({[<“«]$'); // no space after
+// both lists exculde ', ", *, &, #, ~, |, +, _, =, \
 
 function buildVerseText(elmts) {
   let verseTextPartial = '';
@@ -39,10 +41,10 @@ function buildVerseText(elmts) {
   } else if (Array.isArray(elmts)) {
     for (let i = 0; i < elmts.length; i += 1) {
       const text = buildVerseText(elmts[i]);
-      if (punctPattern.test(text)) {
-        verseTextPartial = verseTextPartial.trim();
+      if (punctPattern1.test(text) || punctPattern2.test(verseTextPartial)
+        || verseTextPartial.endsWith(' ') || verseTextPartial === '') {
         verseTextPartial += text;
-      } else {
+      } else if (text !== '') {
         verseTextPartial += ` ${text}`;
       }
     }
@@ -55,8 +57,8 @@ function buildVerseText(elmts) {
       for (let i = 0; i < elmts[key].length; i += 1) {
         const text = buildVerseText(elmts[key][i]);
         if (text !== null) {
-          if (punctPattern.test(text)) {
-            innerVerseText = innerVerseText.trim();
+          if (punctPattern1.test(text) || punctPattern2.test(innerVerseText)
+              || innerVerseText.endsWith(' ') || innerVerseText === '') {
             innerVerseText += text;
           } else {
             innerVerseText += ` ${text}`;
@@ -68,8 +70,9 @@ function buildVerseText(elmts) {
       for (let j = 0; j < elmts[key].length; j += 1) {
         const innerKey = Object.keys(elmts[key][j])[0];
         if (elmts[key][j][innerKey] !== null) {
-          if (punctPattern.test(elmts[key][j][innerKey])) {
-            verseTextPartial = verseTextPartial.trim();
+          if (punctPattern1.test(elmts[key][j][innerKey])
+            || punctPattern2.test(verseTextPartial)
+            || verseTextPartial.endsWith(' ') || verseTextPartial === '') {
             verseTextPartial += elmts[key][j][innerKey];
           } else {
             verseTextPartial += ` ${elmts[key][j][innerKey]}`;
diff --git a/package.json b/package.json
index bef1bf63..b85286d2 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
     "name": "usfm-grammar",
     "description": "An elegant [USFM](https://github.com/ubsicap/usfm) parser (or validator) that uses a [parsing expression grammar](https://en.wikipedia.org/wiki/Parsing_expression_grammar) to model USFM. The grammar is written using [ohm](https://ohmlang.github.io/). Only USFM 3.0 is supported. The parsed USFM is an intuitive and easy to manipulate JSON structure that allows for painless extraction of scripture and other content from the markup. USFM Grammar is also capable of reconverting the generated JSON back to USFM.",
-    "version": "2.0.0-beta.10",
+    "version": "2.0.0",
     "main": "./js/main.js",
     "scripts": {
         "test": "mocha --expose-gc --timeout 40000",


Test patch:
diff --git a/test/test_bugFixes.js b/test/test_bugFixes.js
index 87f0e6cd..e86755f8 100644
--- a/test/test_bugFixes.js
+++ b/test/test_bugFixes.js
@@ -40,10 +40,22 @@ describe('Test bug fixes', () => {
   it('No new space added while concatinating verseText at punctuations', () => {
     // ISSUE: Parsing the usfm file gives the output with spaces in between
     // https://github.com/Bridgeconn/usfm-grammar/issues/76
-    const inputUsfm = '\\id GEN\n\\c 1\n\\p\n\\v 1 verse one\n\\v 2 verse two\\p, which has a comma at the text break point.';
-    const usfmParser = new grammar.USFMParser(inputUsfm);
-    const jsonOutput = usfmParser.toJSON();
+    let inputUsfm = '\\id GEN\n\\c 1\n\\p\n\\v 1 verse one\n\\v 2 verse two\\p\n, which has a comma at the text break point.';
+    let usfmParser = new grammar.USFMParser(inputUsfm);
+    let jsonOutput = usfmParser.toJSON();
     assert.strictEqual(jsonOutput.chapters[0].contents[2].verseText, 'verse two, which has a comma at the text break point.');
+    inputUsfm = '\\id GEN\n\\c 1\n\\p\n\\v 1 verse one\n\\v 2 verse two,\\p which has a comma at the text break point.';
+    usfmParser = new grammar.USFMParser(inputUsfm);
+    jsonOutput = usfmParser.toJSON();
+    assert.strictEqual(jsonOutput.chapters[0].contents[2].verseText, 'verse two, which has a comma at the text break point.');
+    inputUsfm = '\\id GEN\n\\c 1\n\\p\n\\v 1 verse one\n\\v 2 verse two-\\p three which has a hyphen at the text break point.';
+    usfmParser = new grammar.USFMParser(inputUsfm);
+    jsonOutput = usfmParser.toJSON();
+    assert.strictEqual(jsonOutput.chapters[0].contents[2].verseText, 'verse two-three which has a hyphen at the text break point.');
+    inputUsfm = '\\id GEN\n\\c 1\n\\p\n\\v 1 verse one\n\\v 2 verse two\\p -three which has a hyphen at the text break point.';
+    usfmParser = new grammar.USFMParser(inputUsfm);
+    jsonOutput = usfmParser.toJSON();
+    assert.strictEqual(jsonOutput.chapters[0].contents[2].verseText, 'verse two-three which has a hyphen at the text break point.');
   });
 
   it('Files with empty lines', () => {