Update PBN parser to fix comment separation #7
Conversation
Currently, when a multi-line comment starting with { and
immediately followed by content, but the content ends a few lines below,
OR a comment that is part of the data, is not parsed properly.
This commit fixes this issue and the document is parsed properly.
This allows installation directly from GitHub through npm/yarn
|
Thank you for the PR. The code looks good. Could you add a test that checks what is being fixed, take a look at this test |
|
Yes, I'll add a test after work today. |
Some comments appear inside tag sections and need to be parsed as comments while retaining the tag section and tokens.
|
I'm very sorry for the delay, but I had zero time to look at the issue. Have you thought on refactoring the transform function? |
test/pbn.js
Outdated
| @@ -367,13 +367,13 @@ describe('PBN', () => { | |||
|
|
|||
| it('should have multiple lines with braces', (done) => { | |||
| let comment = {}; | |||
| text('{\r\nline 1\r\nline 2\r\n}') | |||
| text('{\r\nline 1\r\nline 2\r\n\}') | |||
There was a problem hiding this comment.
These two lines are the same. So why should the test change?
There was a problem hiding this comment.
Oh my, I don't know how the \ got in there. I'll remove it when I make other changes
test/pbn.js
Outdated
| .pipe(pbn()) | ||
| .on('error', done) | ||
| .on('data', data => { comment = data; }) | ||
| .on('end', () => { | ||
| comment.should.have.property('type', 'comment'); | ||
| comment.should.have.property('text', 'line 1\r\nline 2\r\n'); | ||
| comment.should.have.property('text', 'line 1\r\nline 2\r\n\r\n'); |
There was a problem hiding this comment.
You have broken a test, not a good idea.
|
Let's go back basics. What PBN files are failing. We should have a test for each PBN file. Give me a link or attach them to a comment, so that I can look at them. I appreciate the work you are doing. |
|
I cannot give you the whole results file, but I can give you an excerpt of it. In the first code, one of the comments breaks the whole file when using the master branch. I've hackfixed both situations and my fork parses the file properly. I know it's not a proper solution, but I wanted to have a somewhat working example because of a rush. Thanks! And here's only 1 tag of another game: |
Refactor a bit and add tests for richardschneider#7 of richardschneider/pbn
…er-pr Refactor a bit and add tests for richardschneider#7 of richardschneider/pbn
Currently, when a multi-line comment starting with { and
immediately followed by content, but the content ends a few lines below,
OR a comment that is part of the data, is not parsed properly.