Good question.

We cannot support every weird syntax or edge cases. Are whitespace lines common in PGN or not? I have no idea.
How tricky do you estimate the fix, in terms of complexity and added maintenance cost? If the fix is cheap I'd say let's do it, if not then we can ignore it (perhaps with a test case showing the parser purposefully behaves like that).

Sorry I can't answer your second question, I haven't thought about it. So maybe that answers my question above :)

I looked at the lila repo and found something which might be relevant:

https://github.com/lichess-org/lila/blob/f458495d5145724bf49908d65aed2a294d6ed44d/modules/study/src/main/MultiPgn.scala#L11-L11

I have not worked with scala or the lila repo before this, but based on my understanding, when importing PGNs in studies the string is split on the regex \n\n(?=\[) (Empty line followed by [ indicating a tag for the next game). Each substring is then parsed as a separate game. This can also be replicated in this repository, simplifying the implementation for parsing a multi-game PGN string.

The main drawback of this solution will be that it will break an existing test case/scenario where there are no tags at the start of a game which is not the first game in a multi-game PGN string: https://github.com/lichess-org/dartchess/blob/main/data/leading-whitespace.pgn (See fourth game). However, this will be consistent with the current lila implementation. If you are still ok with this, I can have a go at implementing it.

If the above is unacceptable, the only alternative solution I can think of will require checking the move numbers. While possible to implement, it will be more complex than the above solution: the current move number will need to be tracked and a lookahead will need to be done for the next move number whenever an empty line is found.

It is fine by me, this extra test case can be ignored, it is not an important one. It really makes sense that multi game png are separated by headers.

We should also keep this test case but with the expected result changed to make it clear we don't support this by specification.

Updated the PR and marked it Ready for review

Fix for #30, based on approach discussed above:

• Updated MultiGamePGN parser to split the PGN string on the regex \n\s+(?=\[). After the split, each individual string is parsed separately.
• Since the regex now manages splitting the PGN into multiple games, doing if (_isWhitespace(line)) return _emit(); is no longer needed.
• Uncommented crafty test case, and updated the expected result for leading whitespace test to match new behaviour.

While working on this, I also implemented a fix for #38. This fix is in the Parser but the root cause is different from #30:
To recreate: Start of comment { needs to be the last character on a line. The same line should also have an end of comment } at any point on the same line before the }. Placement of ( or ) does not make any difference.
Example:
sides, primarily for the one which had not expected this line. }) 9... Nxg4 (9... Bg6 10. Nxe5 {
The issue: When { is found before end of line, there is no call to substring and the parser continues processing the same line as a whole. The logic to find the rest of the comment string uses indexOf('}'), but since the parser is at the same line it finds the } character of the previous comment. The parser goes into an infinite loop getting stuck on the same line.
The fix: When a { is the final character on a line, change the parser state to comment and stop processing the line (move to the next line).
I also added the testcase from #38 to validate the fix. This fix is an extension of changes made in #31