Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing fails on PGN with multiple comments after final move #17

Open
gtim opened this issue Jun 1, 2023 · 3 comments
Open

Parsing fails on PGN with multiple comments after final move #17

gtim opened this issue Jun 1, 2023 · 3 comments

Comments

@gtim
Copy link

gtim commented Jun 1, 2023

Thank you again for this stellar package. It seems PGNs with multiple comments after the final move cannot be parsed. The following PGN throws a syntax error on parse:

[Result "*"]

1. d4 Nf6 { comment 1 } { comment 2 } *

As far as I can tell, the PGN is according to the specification. The following parses properly:

[Result "*"]

1. d4 Nf6 { comment 1 } { comment 2 } 2. Bg5 *

If you're interested, I'd be happy to look into the grammar and write a PR.

@shaack
Copy link
Owner

shaack commented Jun 1, 2023

Hello and thank you for your thoughts. I am not sure if the PGN specification allows multiple comments after a move. It's not only about parsing, but about the data model of cm-pgn, which only expects one comment after a move.

@donkawechico
Copy link
Contributor

donkawechico commented Oct 20, 2023

@shaack FWIW, the PGN grammar definitely allows for multiple comments on a move.

See Lines 135-151:

REST_OF_LINE_COMMENT
 : ';' ~[\r\n]* -> skip
 ;

BRACE_COMMENT
 : '{' ~'}'* '}' -> skip
 ;

Because of the -> skip action, the lexer completely ignores any tokens with braces or semicolons, which means you can pile multiple occurrences of those tokens onto one and another and the lexer will simply ignore each one.

You can verify by going to http://lab.antlr.org/ and pasting the PGN grammar into the "Parser" tab. Just make sure to click on the "Lexer" tab and clear its contents first.

Repository owner deleted a comment from jackstenglein Oct 23, 2023
Repository owner deleted a comment from jackstenglein Oct 23, 2023
@shaack
Copy link
Owner

shaack commented Oct 23, 2023

Okay, in that case, if you have the time to create a PR, feel free to do so. I will merge it, if it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants